Skip to content

Commit e8b6f2f

Browse files
author
jgstern-agent
committed
fix(ci): drop 3.12-broken argparse sentinel + fix retry Node.js ordering
Two independent fixes from the latest nightly-matrix re-run. 1) tests/test_cli.py::test_discuss_rejects_ack_thread_before_message started failing on Python 3.12/3.13. The 3.12 argparse refactor made positional backtracking more aggressive: `discuss WI-test --ack-thread "Reply text"` now parses as `ack_thread=True, message="Reply text"` (exit 0) where 3.10/3.11 exited 2 with "unrecognized arguments". The test was a self-documented sentinel for exactly this change — its docstring: "If a future Python or argparse change ever made the broken order work, this test would fail and we'd revisit the gate-message text." Deleted it in place with a comment block recording the history and pointing at the sibling `test_discuss_gate_message_shows_working_arg_order` which already validates the UX. No cli.py change; gate-message text still recommends the universally-working `"<msg>" --ack-thread` order. Tracker package coverage remains 100% in isolation. 2) nightly.yml test-matrix-retry and release.yml post-publish-matrix- retry both had `actions/download-artifact@v3` (a JS action that needs Node.js on PATH) running BEFORE `Install Node.js` in a fresh python:${ver}-bookworm container. download-artifact failed with `OCI runtime exec failed ... exec: "node": executable file not found` under continue-on-error, the pass marker never landed, and the skip-check incorrectly reported "Primary job failed ... retrying" for already-passing Python versions. Observed 2026-04-21: 3.10/3.11 primary passed but their retry containers still ran the full ~5-minute retry path. The primary test-matrix jobs solve this already by installing Node.js as their first step; the retry jobs just didn't copy that ordering. Moved `Install Node.js for JS- based actions` to be the first unconditional step of both retry jobs. Retry containers still spin up for every matrix value when any primary fails (platform limitation — Forgejo/GitHub Actions can't express per-matrix-value retry gating), but short-circuit in ~10s via the skip-check once the pass marker is found, down from the full 5+ minute spurious retry. Signed-off-by: jgstern-agent <josh-agent@iterabloom.com>
1 parent 08df05a commit e8b6f2f

5 files changed

Lines changed: 41 additions & 40 deletions

File tree

.ci/affected-tests.txt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
# Test selection manifest
2-
# Generated by smart-test at 2026-04-21T00:22:39-04:00
2+
# Generated by smart-test at 2026-04-21T01:17:00-04:00
33
# Mode: targeted
44
# Baseline: 7484e9d523ac1b77f43a4fe71a90c54c5aa6864d
5-
# Reason: no Python source files changed
6-
# Changed files: 6
5+
# Changed files: 7
76
# Changed source files: 0
8-
# Selected tests: 0
7+
0
8+
# Selected tests: 1
99
#
1010
# === CHANGED_SOURCE_FILES ===
1111
# === SELECTED_TESTS ===
12+
packages/hypergumbo-tracker/tests/test_cli.py

.github/workflows/nightly.yml

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,21 @@ jobs:
134134
SEGMENT_DOWNLOAD_TIMEOUT_MINS: "1"
135135

136136
steps:
137+
# Forgejo runner needs Node.js in the container to execute JS-based
138+
# actions like `actions/download-artifact@v3`. python:*-bookworm
139+
# images don't ship it. This MUST precede the download-artifact
140+
# step — if Node isn't on PATH, download-artifact fails with
141+
# `OCI runtime exec failed ... exec: "node": executable file not
142+
# found` under `continue-on-error: true`, the pass-marker file is
143+
# never written, and the skip-check then incorrectly reports
144+
# "Primary job failed ... retrying" for every Python version.
145+
# Observed 2026-04-21 on a nightly where the primary 3.10/3.11
146+
# passed but their retry ran anyway. The primary test-matrix job
147+
# solves this the same way (step "Install Node.js for JS-based
148+
# actions" at the top).
149+
- name: Install Node.js for JS-based actions
150+
run: apt-get update && apt-get install -y nodejs
151+
137152
- name: Download pass marker
138153
id: check-primary
139154
uses: actions/download-artifact@v3
@@ -153,10 +168,6 @@ jobs:
153168
echo "should_skip=false" >> "$GITHUB_OUTPUT"
154169
fi
155170
156-
- name: Install Node.js for JS-based actions
157-
if: steps.skip-check.outputs.should_skip != 'true'
158-
run: apt-get update && apt-get install -y nodejs
159-
160171
- uses: actions/checkout@v4
161172
if: steps.skip-check.outputs.should_skip != 'true'
162173

.github/workflows/release.yml

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,14 @@ jobs:
415415
SEGMENT_DOWNLOAD_TIMEOUT_MINS: "1"
416416

417417
steps:
418+
# Must run before `actions/download-artifact@v3` (a JS action) —
419+
# see the identical note on the test-matrix-retry job in
420+
# nightly.yml. Without this, download-artifact fails silently
421+
# under continue-on-error, the pass marker never lands, and the
422+
# skip-check incorrectly retries every Python version.
423+
- name: Install Node.js for JS-based actions
424+
run: apt-get update && apt-get install -y nodejs
425+
418426
- name: Download pass marker
419427
id: check-primary
420428
uses: actions/download-artifact@v3
@@ -434,10 +442,6 @@ jobs:
434442
echo "should_skip=false" >> "$GITHUB_OUTPUT"
435443
fi
436444
437-
- name: Install Node.js for JS-based actions
438-
if: steps.skip-check.outputs.should_skip != 'true'
439-
run: apt-get update && apt-get install -y nodejs
440-
441445
- uses: actions/checkout@v4
442446
if: steps.skip-check.outputs.should_skip != 'true'
443447

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ This changelog tracks the **tool version** (package releases). The **schema vers
1212

1313
### Fixed
1414

15+
- **Argparse sentinel test dropped + nightly/release retry Node.js chicken-and-egg fixed:** two unrelated nightly-run artifacts from the re-run after the previous CI PR. (1) `packages/hypergumbo-tracker/tests/test_cli.py::TestWriteCommands::test_discuss_rejects_ack_thread_before_message` started failing on the Python 3.12/3.13 slices of the test-matrix. The test was a self-documented sentinel for the argparse behavior change described in CPython's 3.12 argparse refactor: `discuss WI-test --ack-thread "Reply text"` used to hit `exit 2 unrecognized arguments` on 3.10/3.11 but on 3.12+ argparse's more-aggressive positional backtracking consumes `"Reply text"` into the `nargs="?"` positional `message`, producing `ack_thread=True, message="Reply text"` and exit 0. The test's own docstring predicted exactly this: "If a future Python or argparse change ever made the broken order work, this test would fail and we'd revisit the gate-message text." Deleted the test with an in-place comment block documenting the history and pointing at the sibling `test_discuss_gate_message_shows_working_arg_order` which still validates the actual UX. No code change needed — the gate-message text already recommends the universally-working `"<msg>" --ack-thread` order. Coverage stays at 100% (verified via `./scripts/check-package-coverage tracker`). (2) `test-matrix-retry` in `.github/workflows/nightly.yml` was firing the full retry for every Python version when ANY primary matrix slice failed, even the versions whose primary passed. Root cause: `actions/download-artifact@v3` is a JS action that needs Node.js, but the retry job runs in a fresh `python:<ver>-bookworm` container and had `Install Node.js` AFTER `Download pass marker` — so download-artifact failed with `OCI runtime exec failed ... exec: "node": executable file not found` under its `continue-on-error: true` wrapper, the pass marker file never landed, the subsequent `Skip if primary passed` check incorrectly printed `Primary job failed for Python 3.10 — retrying` and ran the full retry. Observed 2026-04-21 — 3.10/3.11 primary passed but their retry containers still spun up and ran tests. The primary `test-matrix` job already solves this correctly by installing Node.js as its first step; the retry job just didn't copy that ordering. Moved `Install Node.js for JS-based actions` to be the first unconditional step in the retry. Same bug present in `.github/workflows/release.yml`'s post-publish-matrix-retry (download-artifact@v3 before Install Node.js) — fixed identically. After the fix the retry containers still spin up when ANY version fails primary (GitHub/Forgejo Actions can't express per-matrix-value retry gating natively — it's a platform constraint), but short-circuit in ~10 seconds once the skip-check sees the pass marker instead of running the full 5+ minute retry. Refactoring out of the matrix pattern into per-version jobs would fix the spin-up overhead entirely at the cost of ~2× the YAML duplication; deferred since the post-fix overhead is well under the nightly's noise floor.
16+
1517
- **`hypergumbo-lang-rust-analyzer` wired into nightly.yml / release.yml / full-suite.yml:** only `ci.yml` (per-PR) had the new package in its install lines. Observed failure: `nightly.yml`'s `test-matrix` job collection-errored on every rust-analyzer test file (`ModuleNotFoundError: No module named 'hypergumbo_lang_rust_analyzer'`) because the install block installed five sibling `hypergumbo-lang-*` packages but not this one, while `pytest packages/*/tests/` still tried to collect its tests. Three coordinated edits: (1) `nightly.yml` — added `-e packages/hypergumbo-lang-rust-analyzer` to all three install blocks (lines 49, 168, 245) and `--cov=packages/hypergumbo-lang-rust-analyzer/src` to both COV_PATHS lines (75, 197). (2) `release.yml` — same two kinds of additions at the four install blocks (91, 327, 448, 525) and both COV_PATHS lines (353, 477). Would have failed the same way at the next tag-push; preemptive. (3) `full-suite.yml` — added a dedicated `test-rust-analyzer` job (no retry variant; matches the `test-common` / `test-extended` "small-package" pattern rather than the `test-core` / `test-mainstream` / `test-tracker` retry-eligible pattern since rust-analyzer has ~25 tests across 5 files), and extended the `aggregate` job to include it in the `needs` list, the per-job echo, the success-gate, the coverage roll-up (divisor 5 → 6), the `record-run-result` JSON, and the `all-100%` badge gate. Full-suite was silently not exercising rust-analyzer tests at all before this.
1618

1719
- **Nightly coverage gaps closed (scip_pb2 omit + tree-sitter-c-sharp 0.23.5 grammar shift):** two unrelated holes surfaced by the `--cov-config=.coveragerc.no-embeddings` invocation of nightly.yml when `sentence-transformers` is unavailable (the default in the Python-matrix containers). (1) `packages/hypergumbo-core/src/hypergumbo_core/scip/_generated/scip_pb2.py` was reporting 18% coverage because the `*/scip/_generated/scip_pb2.py` omit lived only in `pyproject.toml` under `[tool.coverage.run]`; passing `--cov-config=` makes coverage.py read ONLY that config file, so the pyproject omit list is dropped. Duplicated the entry in `.coveragerc.no-embeddings` so both config paths exclude the auto-generated protobuf binding. (2) `packages/hypergumbo-lang-mainstream/src/hypergumbo_lang_mainstream/csharp.py` was reporting 98% with lines 112-131 (the named-attribute-argument `kwargs` branch) uncovered. Root cause: `tree-sitter-c-sharp~=0.23.1` in mainstream's pyproject.toml resolves to the latest-satisfying 0.23.5, and 0.23.5 flattened the named-argument grammar — `[Route("api", Name="Get")]` now emits `attribute_argument → [identifier, =, string_literal]` directly instead of wrapping those children in an `assignment_expression` node. The code's `find_child_by_type(arg, "assignment_expression")` returned None, the named branch was dead, and the test that should have covered it (`test_annotation_with_named_argument`) started accumulating `"Name"` into the positional `args` list — confirmed by running it under 0.23.5 (`assert ['api/test', 'Name'] == ['api/test']`). Fix: bumped the pin to `tree-sitter-c-sharp~=0.23.5` (matches what pip already resolves) and rewrote the named-arg detection to look for a direct `=` child on `attribute_argument`, then extract the first `identifier` and first `string_literal` children into `kwargs[name] = value`. All 87 `test_csharp.py` cases pass under 0.23.5; full smart-test run: 5201 passed, 0 failed, `csharp.py` at 100% coverage.

packages/hypergumbo-tracker/tests/test_cli.py

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,34 +1237,17 @@ def test_discuss_gate_message_shows_working_arg_order(
12371237
# Anti-regression: must NOT show the broken order
12381238
assert "--ack-thread \"<your reply>\"" not in out
12391239

1240-
def test_discuss_rejects_ack_thread_before_message(
1241-
self, tmp_path: Path, capsys: pytest.CaptureFixture,
1242-
mock_agent_uid: None,
1243-
) -> None:
1244-
"""WI-foril: regression check that argparse really does reject the
1245-
broken order. If a future Python or argparse change ever made the
1246-
broken order work, this test would fail and we'd revisit the
1247-
gate-message text. Until then, the fix in the gate-message is the
1248-
right shape (steer users away from a still-broken pattern).
1249-
"""
1250-
tracker_root = _setup_tracker(tmp_path)
1251-
ops_dir = tracker_root / "tracker-workspace" / ".ops"
1252-
_add_item_with_discussion(ops_dir, "WI-test", [
1253-
{"at": "2026-01-01T01:00:00Z", "by": "agent",
1254-
"actor": "test_agent", "message": "Initial note"},
1255-
{"at": "2026-01-02T01:00:00Z", "by": "human",
1256-
"actor": "jgstern", "message": "Please investigate this"},
1257-
])
1258-
with pytest.raises(SystemExit) as exc:
1259-
main([
1260-
"--tracker-root", str(tracker_root),
1261-
"discuss", "WI-test",
1262-
"--ack-thread", "Reply text",
1263-
])
1264-
# argparse exits 2 (SystemExit) on unrecognized arguments
1265-
assert exc.value.code == 2
1266-
err = capsys.readouterr().err
1267-
assert "unrecognized arguments" in err
1240+
# WI-foril: the sentinel test that asserted argparse rejects
1241+
# `--ack-thread "<msg>"` (flag-before-positional) was removed when
1242+
# Python 3.12's argparse refactor made the backtracking over
1243+
# `nargs="?"` positionals more aggressive and the "broken" order
1244+
# started parsing as `ack_thread=True, message="<msg>"`. Its own
1245+
# docstring anticipated this: "If a future Python or argparse
1246+
# change ever made the broken order work, this test would fail and
1247+
# we'd revisit the gate-message text." The gate-message
1248+
# (tests/test_cli.py::test_discuss_gate_message_shows_working_arg_order)
1249+
# still recommends the universally-working order (`"<msg>"` then
1250+
# `--ack-thread`), so no text change is needed.
12681251

12691252
def test_discuss_no_warning_when_last_is_agent(
12701253
self, tmp_path: Path, capsys: pytest.CaptureFixture,

0 commit comments

Comments
 (0)