Skip to content

ci: extract the shell-suite runner and guard that CI keeps calling it - #119

Closed
laird wants to merge 26 commits into
masterfrom
ci/extract-shell-suite-runner
Closed

ci: extract the shell-suite runner and guard that CI keeps calling it#119
laird wants to merge 26 commits into
masterfrom
ci/extract-shell-suite-runner

Conversation

@laird

@laird laird commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Ports scripts/run-shell-suites.sh and tests/test_ci_runs_shell_suites.py
from the stale #77 onto current master.

Scope — read this before the diff

This is not a fix for issue #32. I initially thought it was. #32 — CI
collected only pytest tests/, so every tests/*.sh suite went unexecuted —
was already fixed on master by inlining a loop into the shell job of
.github/workflows/test.yml. The suites run today.

What the inline fix left open:

  1. The logic was reachable only by pushing. There was no way to run locally
    what CI runs, so a suite could pass under one invocation and fail under the
    other with nothing to reconcile them.
  2. Nothing asserted CI still ran the suites. The fix lived entirely in YAML.
    Deleting the shell job would restore the original silence without failing a
    single test.

This PR closes those two. It is a refactor plus a regression guard, not a bug
fix — judge it on that basis.

Changes

  • scripts/run-shell-suites.sh — the inline loop, extracted. CI now calls
    it, so the workflow and bash scripts/run-shell-suites.sh execute the same
    code. Keeps the original's design notes: globs tests/*.sh but not
    tests/fixtures/*.sh (those are setup|teardown helpers; note a bash glob
    does not cross / but git ls-files 'tests/*.sh' does), lives in scripts/
    so it cannot match its own glob, and accumulates failures rather than
    set -e-ing out on the first so one broken suite cannot mask five others.
  • .github/workflows/test.yml — the shell job's 20-line inline script
    becomes run: bash scripts/run-shell-suites.sh.
  • tests/test_ci_runs_shell_suites.py — 7 tests. They are pytest tests on
    purpose: a shell test guarding the shell runner could itself go unexecuted,
    whereas pytest is what CI is already guaranteed to run.

Two corrections to the ported originals

Both came from checking the originals' claims rather than trusting them.

The CI guard could not fail. test_ci_invokes_the_shell_runner
substring-searched the raw YAML for run-shell-suites.sh. test.yml's comments
name the runner while explaining why it is there, so the assertion stayed green
after the step that runs it was deleted. Both CI assertions now match a run:
step against comment-stripped YAML. Verified by deleting each step in turn:

mutation before after
shell runner step removed 7 passed ❌ test_ci_invokes_the_shell_runner fails ✅
pytest step removed 7 passed ❌ test_ci_still_runs_pytest fails ✅

The env-scrub rationale was stale. The runner's header claimed the per-suite
scrub of ISSUE_SOURCE / ISSUE_BACKEND / ISSUE_DIR_PATH was load-bearing —
"Without this, three suites fail locally and pass in CI." Not true on current
master: #19 made .autocoder.json win over a stale exported ISSUE_SOURCE, and
all suites pass with those three set to junk values (I tried ISSUE_SOURCE=github,
=jira, ISSUE_BACKEND=issues-gh.sh, ISSUE_DIR_PATH=/nonexistent, and
combinations — zero failures). The scrub is kept as cheap insurance against a
future suite that resolves config itself, and the comment now says that instead
of overclaiming.

Verification

  • 7/7 new tests pass, and go red under both negative mutations above
  • 112 pytest tests pass
  • bash scripts/run-shell-suites.sh → 17 passed, 0 failed, 0 skipped
  • bash -n clean on the runner

🤖 Generated with Claude Code

laird and others added 26 commits August 7, 2026 15:36
…g to N-clean; scope + exercise preflight; swarm cadence)
Design spec for adding a human-gated planning capability to the manager
session (brainstorm → spec → critical review → decompose to stories →
existing fleet implements), plus renaming /fix→/dev and /fix-loop→/dev-loop
with aliases. Reuses existing skills and the decomposed/subtask convention;
adds two backlog-aware behaviors to the manager loop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Critical-design-review round 1 approved the planning-pipeline spec as-is:
0 literal-wrongness findings, 0 forced decisions. Empirically confirmed the
two load-bearing hand-off facts (subtask is claimable; decomposed parents are
excluded from claiming). Recorded the CDR approval in the spec status and
carried the three CIR-deferred items forward as notes B8/B9 (+ existing B5).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eview-1 to autocoder-planning-pipeline-implementation-plan
Tracks the 2026-07-28 handoff documenting the /fix→/dev rename and
peters-toolkit:bugfix integration, plus the still-unaddressed critical
review of the planning-pipeline implementation plan.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… reimplemented on master + platform drift repair
… exclusion, drift repair), marketplace to 3.32.0
…ound/improved/learned per round, verdict chapter)
…e v2 removal

Atlassian removed POST /rest/api/2/search from Jira Cloud (HTTP 410,
CHANGE-2046). list and any-claimable now use POST /rest/api/3/search/jql
with an explicit fields list, nextPageToken pagination (no startAt), and
existence-of-first-page instead of the removed total for any-claimable.
Issue lifecycle endpoints stay on v2 (not removed; plain-string bodies).
The fake serves the new contract, enforces a settable page cap to exercise
the token loop, and returns 410 on v2 search so regressions cannot pass.

Live verification against real Jira Cloud pending by coordinator.
…text

Live smoke showed /rest/api/3/search/jql returns description as an ADF
document object, not v2's plain string. The list reshape now walks ADF
content nodes collecting text (paragraphs joined with newlines; tolerates
null, plain strings, malformed nodes), so body stays a plain string for
all consumers. v2 CRUD read paths are untouched (still plain strings).
The fake now serves ADF descriptions from v3 search to pin the contract.
…-smoke-test.sh; live validation pending an ADO sandbox)
…gents

The SKILL.md had no model guidance. Top-level coordinator (scope/grade/fix
decisions) uses the deep model; subagents and swarm workers use the balanced
model. Mirrors the autocoder manager/worker tier pattern.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… model selection (#110)

* feat(autocoder): add model-config startup confirmation and env-var-driven model selection

Autocoder agents now resolve model tiers from env vars → .autocoder.json →
built-in defaults (claude-opus-5 / claude-sonnet-5 / claude-haiku-4-5 on
Claude Code; gemini-2.5-pro / gemini-2.0-flash on Antigravity).

On first run with no model config found, the agent presents the defaults and
asks the user to confirm or override before proceeding.  The confirmed models
are exported as MANAGER_MODEL / WORKER_MODEL / FAST_MODEL and referenced in
every Task tool call in the workflow so agents actually use them.

Also adds skills/autocoder/references/model-config.md documenting the tier
system and how to persist overrides via .autocoder.json.

Parallel maintenance: plugins/autocoder/commands/fix.md ↔ .agent/workflows/fix.md

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore: bump autocoder to 4.11.0, marketplace to 3.33.0

Reflects model-config startup confirmation and env-var-driven model
selection added in the previous commit.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
#112)

Switch fix.md and model-config.md from full API model IDs (claude-opus-5)
to platform tier shorthand (opus/sonnet/haiku for Claude Code, pro/flash
for Gemini). Agents inherit credentials from the running session — no
separate API keys needed.

Rename skills/harden → skills/improve to reflect the general improvement
and validation loop, not just security hardening.

Bump autocoder 4.11.0 → 4.12.0, marketplace 3.33.0 → 3.34.0.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
- test_issues_gh_search.sh (11 failures): BASH_ENV=/home/laird/.bashenv
  prepends ~/bin to PATH on every bash invocation, putting the real gh
  before the test stub. Fix: pass BASH_ENV="" when invoking the backend.

- test_issue_source_lib.sh (1 failure): bash reverts VAR=val assignments
  on shell-function calls after the function returns, so the previously-
  exported ISSUE_DIR_PATH from the CLI-dir test step was restored instead
  of the function's new value. Fix: export explicitly before the call.

- test_worker_launch_lib.sh (1 failure): Gemini workers should use the
  /fix-loop interactive command (matching .agent/workflows/fix-loop.md),
  not a non-existent gemini-fix-loop.sh shell script. Fix: align
  worker-launch-lib.sh with the test expectation.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
#96)

restart-worker.sh killed the hung worker but never relaunched it, leaving the
pane at a bare shell while still printing "✅ Worker restarted".

Root cause: callers derive AGENTS_REPO_ROOT as SCRIPT_DIR/../../.., which is
only correct for a repo checkout. An installed plugin lives at
cache/plugin-marketplace/autocoder/<version>/scripts, so that walk lands on the
cache root; appending plugins/autocoder/scripts/claude-worker-loop.sh yields the
hybrid path from the report — real in neither the cache nor marketplace layout.

Changes:
- worker-launch-lib.sh: resolve the loop script from the lib's own directory
  (WORKER_LAUNCH_LIB_DIR); both ship in the same scripts/ dir. Fail loudly with
  a non-zero return if it is missing instead of emitting an unusable path.
- restart-worker.sh: after sending WORKER_CMD, capture the pane and exit
  non-zero if it reports "No such file or directory"/"command not found", so a
  killed-but-not-restarted worker can no longer be reported as success.
- test_worker_launch_lib.sh: assert the emitted loop path EXISTS on disk when
  repo_root does not follow the checkout layout, and that it is not derived from
  repo_root. Verified red before the fix with exactly the reported path shape.

Verification: 17/17 shell unit tests pass; bash -n + py_compile clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Issue #32 — CI collected only `pytest tests/`, so every tests/*.sh suite
went unexecuted — was already fixed by inlining a loop into the `shell`
job. This does not re-fix that. It closes the two gaps the inline fix
left:

  1. The logic was reachable only by pushing. There was no way to run
     locally what CI runs, so a suite could pass under one invocation and
     fail under the other with nothing to reconcile them.
  2. Nothing asserted CI still ran the suites. The fix lived entirely in
     YAML; deleting the job would restore the original silence without
     failing a single test.

scripts/run-shell-suites.sh is that loop, extracted; the workflow now
calls it, so CI and `bash scripts/run-shell-suites.sh` execute the same
code. tests/test_ci_runs_shell_suites.py is the guard — 7 pytest tests
(pytest, because a shell test guarding the shell runner could itself go
unexecuted) asserting the runner exists and parses, that CI invokes it,
that pytest is still run alongside rather than instead, that the glob is
not narrowed and any SKIP entry carries a reason, that fixtures stay
excluded, that failures accumulate rather than aborting on the first,
and — end-to-end — that injecting a failing suite makes the runner exit
non-zero.

Two corrections to the ported originals, both from checking their claims
rather than trusting them:

  - test_ci_invokes_the_shell_runner substring-searched the raw YAML for
    "run-shell-suites.sh". test.yml's comments name the runner, so the
    assertion stayed green after the step running it was deleted — a
    guard that could not fail. Both CI assertions now match a `run:`
    step against comment-stripped YAML. Verified by deleting each step
    in turn and watching the matching test go red.
  - The runner's header claimed the per-suite scrub of ISSUE_SOURCE /
    ISSUE_BACKEND / ISSUE_DIR_PATH was load-bearing ("three suites fail
    locally and pass in CI"). It is not, on current master: #19 made
    .autocoder.json win over a stale exported ISSUE_SOURCE, and all
    suites pass with those vars set to junk. The scrub is kept as cheap
    insurance and the comment now says so.

Verified: 7/7 new tests pass and go red under both negative mutations;
112 pytest tests pass; runner reports 17/17 shell suites passing;
bash -n clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
laird added a commit that referenced this pull request Aug 17, 2026
…#127)

ci: extract the shell-suite runner and guard that CI keeps calling it. Supersedes #119. Unrelated pre-existing shell CI failure tracked as #125 (23 passed, 1 failed -- matches local exactly).
@laird

laird commented Aug 17, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #127.

This branch was 26 commits behind master (CONFLICTING), mostly carrying already-landed history. The one real commit is reapplied cleanly on current master in #127, with one correction: master gained tests/run-shell-suite.sh (a meta-runner) after this PR was opened, and this branch's tests/.sh glob would have picked it up as a leaf suite, running everything a second time nested inside it on every CI run. #127 narrows the glob to tests/test_.sh, which excludes it structurally, with a test pinning the exclusion.

Independently verified the CI guard's core claim (that it actually fails when the workflow step is removed) before merging -- confirmed.

@laird laird closed this Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant