Skip to content

feat: add AGENT_DEV_LAUNCHER / AGENT_REVIEW_LAUNCHER per-side overrides (INV-38)#158

Merged
zxkane merged 8 commits into
mainfrom
feat/per-side-launcher
May 26, 2026
Merged

feat: add AGENT_DEV_LAUNCHER / AGENT_REVIEW_LAUNCHER per-side overrides (INV-38)#158
zxkane merged 8 commits into
mainfrom
feat/per-side-launcher

Conversation

@zxkane
Copy link
Copy Markdown
Owner

@zxkane zxkane commented May 26, 2026

Summary

Adds two operator knobs to lib-agent.sh that let dev and review wrappers each have their own launcher prefix:

  • AGENT_DEV_LAUNCHER — defaults to ${AGENT_LAUNCHER:-}
  • AGENT_REVIEW_LAUNCHER — defaults to ${AGENT_LAUNCHER:-}

The single [INV-37] "both sides claude" launcher guard is replaced by two independent per-side guards: AGENT_DEV_LAUNCHER non-empty requires AGENT_DEV_CMD=claude; AGENT_REVIEW_LAUNCHER non-empty requires AGENT_REVIEW_CMD=claude. Strictly more permissive — every operator config that passed before still passes.

The motivating use case: a project that wants AGENT_CMD="claude" + AGENT_DEV_LAUNCHER='cc...' (Bedrock bridge for dev) + AGENT_REVIEW_CMD="kiro" (no launcher needed for review). Pre-INV-38: blocked by [INV-37]. Post-INV-38: works.

What's in the PR

  • skills/autonomous-dispatcher/scripts/lib-agent.sh — init + per-side argv tokenization (mirrors existing AGENT_LAUNCHER eval); two per-side guards replace the single guard
  • skills/autonomous-dispatcher/scripts/autonomous-dev.sh — 4-line WHY + 1-line AGENT_LAUNCHER_ARGV rebind after the existing AGENT_CMD rebind from feat: add AGENT_DEV_CMD / AGENT_REVIEW_CMD per-side overrides (INV-37) #156
  • skills/autonomous-dispatcher/scripts/autonomous-review.sh — symmetric rebind for review side
  • skills/autonomous-dispatcher/scripts/autonomous.conf.example — operator-facing comment block with worked example + caveat about the kiro --trust-all-tools stage-vs-side gotcha
  • tests/unit/test-lib-agent-per-side-launcher.sh — 14 assertions (PSL-S1..S10): defaults, fallback, per-side override, per-side guard pass/fail with RC=1, structural placement
  • tests/unit/test-lib-agent-per-side-cmd.sh — PSC-S7/S8/S11 assertion needles updated to match the new per-side error messages
  • docs/pipeline/per-side-launcher.md — spec
  • docs/pipeline/invariants.md — INV-38
  • docs/pipeline/README.md — index entry
  • docs/superpowers/plans/2026-05-26-per-side-launcher.md — implementation plan

Backwards compatibility

Existing deployments don't set the new vars. Defaults make both per-side ARGVs equal to the existing AGENT_LAUNCHER_ARGV. The wrapper rebinds copy into the same variable. Result: byte-for-byte identical pre/post.

Spec & invariant

  • docs/pipeline/per-side-launcher.md
  • INV-38: per-side AGENT_LAUNCHER precedence

Pipeline-docs gate

Touches lib-agent.sh (watched path), so docs/pipeline/ must also change. Satisfied:

  • docs/pipeline/per-side-launcher.md (new spec)
  • docs/pipeline/invariants.md (INV-38)
  • docs/pipeline/README.md (index entry)

Test plan

  • bash tests/unit/test-lib-agent-per-side-launcher.sh — 14/14 PASS
  • bash tests/unit/test-lib-agent-per-side-cmd.sh — 15/15 PASS (PSC-S7/S8/S11 needles updated)
  • All existing tests/unit/test-lib-agent-*.sh regression tests PASS
  • shellcheck -S error clean on lib-agent.sh + both wrappers + new test
  • bash -n autonomous.conf.example syntax OK
  • CI: unit-tests + shellcheck + pipeline-docs-gate green

Review process

The PR went through /pr-review-toolkit:review-pr with four specialized agents (code-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer). All Critical/High/Medium findings addressed inline:

  • H1 (comment-analyzer): dropped invalid AGENT_DEV_EXTRA_ARGS="--trust-all-tools" from the worked example (the stage-vs-side EXTRA_ARGS gap from agy-Finding-1 means it would have been forwarded to claude DEV and broken every dispatch). Added an operator caveat pointing kiro-side trust-all-tools at ~/.kiro/agents/<name>.json instead.
  • M1 (comment-analyzer): spec doc PSL-S9/S10 line-window numbers (5/6) corrected to match the actual test windows (9/9).
  • M2 (comment-analyzer): spec doc placement description for the conf-example block updated to match where the block actually lands.
  • Update README: Replace placeholder username with zxkane #3 (pr-test-analyzer): PSL-S7/S8 now assert RC=1 on guard failure, locking in the return 1 behavior so a future refactor can't silently drop it.

silent-failure-hunter found 0 critical issues (per-side guards correctly fail loud at source time; wrapper rebind is byte-identical to AGENT_LAUNCHER under default config). code-reviewer noted only an SC2034 informational warning that's consistent with existing patterns in the project.

Known limitations (out of scope, deferred)

  • AGENT_*_EXTRA_ARGS stage-vs-side mapping (agy review Finding 1, pre-existing on origin/main): run_agent reads AGENT_DEV_EXTRA_ARGS regardless of which wrapper called it, and resume_agent reads AGENT_REVIEW_EXTRA_ARGS. This means AGENT_DEV_EXTRA_ARGS=--trust-all-tools (intended for kiro review side) would be forwarded to both wrappers. Fix shape is different from this PR — likely an AGENT_SIDE parameter to run_agent/resume_agent. Tracked separately. Operators wanting --trust-all-tools on kiro review side should use ~/.kiro/agents/<name>.json for now; the conf example documents this caveat.

zxkane added 8 commits May 26, 2026 12:32
Spec for AGENT_DEV_LAUNCHER / AGENT_REVIEW_LAUNCHER operator knobs that
let dev and review wrappers each have their own launcher prefix. Pairs
with INV-37 (per-side AGENT_CMD): the launcher's claude-specific env
(Bedrock, AWS profile, ANTHROPIC_DEFAULT_*) doesn't fit non-claude
review CLIs, so the existing single AGENT_LAUNCHER blocks the natural
mixed-CLI deployment.

Per-side guard splits the [INV-37] form: each side's launcher is
checked against THAT side's AGENT_CMD. Strictly more permissive — no
existing config goes from passing to failing. Default fallback to
AGENT_LAUNCHER preserves byte-for-byte back-compat.

INV-38 (the precedence rule) is referenced in the spec but lands in
invariants.md as part of the implementation PR.
Bite-sized TDD plan, six tasks with red-green-commit cadence:
  T1 — lib-agent init (2 vars + per-side argv tokenization + guard
       rewrite) + PSL-S1..S8 + update PSC-S7/S8/S11 needles
  T2 — autonomous-dev.sh AGENT_LAUNCHER_ARGV rebind + PSL-S9
  T3 — autonomous-review.sh AGENT_LAUNCHER_ARGV rebind + PSL-S10
  T4 — autonomous.conf.example operator block
  T5 — INV-38 in invariants.md
  T6 — final regression + push + PR

Self-review confirms full spec coverage, no placeholders, type
consistency across tasks.
Adds per-side AGENT_LAUNCHER overrides. Both default to AGENT_LAUNCHER
so existing deployments are byte-for-byte unchanged. The single
'both sides claude' guard from INV-37 is replaced by two independent
per-side guards: each side's launcher is gated on THAT side's
AGENT_CMD. Strictly more permissive than the INV-37 form.

Tokenization mirrors the existing AGENT_LAUNCHER eval block (same
trust model, same WARN-on-empty-tokenize, same ERROR-on-parse-failure).

Tests:
  - new test-lib-agent-per-side-launcher.sh — PSL-S1..S8 (resolution
    + per-side guard pass/fail).
  - update test-lib-agent-per-side-cmd.sh PSC-S7/S8/S11 needles to
    match the new per-side error messages.

PSL-S9/S10 (wrapper structural placement) land in Tasks 2 and 3.
INV-38 entry lands in Task 5.
…b-agent source

Adds AGENT_LAUNCHER_ARGV=("${AGENT_DEV_LAUNCHER_ARGV[@]}") immediately
after the existing AGENT_CMD=$AGENT_DEV_CMD line so _run_with_timeout
picks up the dev-side launcher when the operator has set per-side
overrides.

Default behavior unchanged because AGENT_DEV_LAUNCHER defaults to
AGENT_LAUNCHER inside lib-agent.sh.

Test: PSL-S9 asserts the rebind line lands within 9 lines of source
lib-agent.sh (3-line WHY + AGENT_CMD rebind from INV-37 + 4-line WHY
+ new LAUNCHER rebind = 9 lines).
…ter lib-agent source

Mirrors the dev-side change. Adds AGENT_LAUNCHER_ARGV=("${AGENT_REVIEW_LAUNCHER_ARGV[@]}")
immediately after the existing AGENT_CMD=$AGENT_REVIEW_CMD line so
_run_with_timeout picks up the review-side launcher when the operator
has set per-side overrides.

Test: PSL-S10 asserts the rebind line lands within 9 lines of source
lib-agent.sh (4-line WHY + AGENT_CMD rebind from INV-37 + 3-line WHY
+ new LAUNCHER rebind = 9 lines).
Adds an operator-facing comment block above the per-side AGENT_CMD
block. Includes a worked example for the 'claude-dev with cc bridge /
kiro-review without launcher' deployment pattern (the motivating use
case) and the per-side guard reminder.

Updates the AGENT_LAUNCHER hint in the AGENT_DEV_CMD block to point
operators at the new per-side launcher escape (INV-38 lifts the old
INV-37 'both sides claude' restriction).
INV-38 documents the AGENT_DEV_LAUNCHER / AGENT_REVIEW_LAUNCHER
precedence rule that pairs with INV-37 to allow mixed-CLI deployments
where only one side has a launcher (e.g. claude dev with cc bridge,
kiro review without launcher).

INV-37 cross-references list updated to include INV-38.

Anchored to docs/pipeline/per-side-launcher.md (already on this branch
with cross-reference [INV-38] resolved by this commit).
PR-review fixes:

H1 (comment-analyzer Critical-grade for operators): the worked example
in autonomous.conf.example for 'claude-dev + kiro-review' set
AGENT_DEV_EXTRA_ARGS="--trust-all-tools". But due to the pre-existing
stage-vs-side EXTRA_ARGS mapping bug (agy review Finding 1: run_agent
reads AGENT_DEV_EXTRA_ARGS regardless of caller side), the claude DEV
invocation would pick up --trust-all-tools (an invalid claude flag)
and fail at every dispatch. Drop the line; add an operator caveat
explaining that kiro-side trust-all-tools should be configured via
~/.kiro/agents/<name>.json until the side mapping bug is fixed in a
follow-up.

M1 (comment-analyzer): spec doc PSL-S9/S10 said 'within 5/6 lines';
actual test windows are +9/+9. Update spec to match.

M2 (comment-analyzer): spec doc placement description for the conf-
example block was stale (said 'before AGENT_DEV_EXTRA_ARGS'; actual
landing is between AGENT_*_EXTRA_ARGS and AGENT_DEV_CMD). Update.

#3 (pr-test-analyzer): PSL-S7/S8 didn't assert RC=1 on guard failure
— a regression that drops 'return 1' would leave the error message
intact and the test would still pass. Add 'RC=1' assertions.

Tests: PASS=14 FAIL=0 (12 + 2 RC=1 assertions). Full regression
clean. shellcheck clean. conf-syntax OK.
@zxkane
Copy link
Copy Markdown
Owner Author

zxkane commented May 26, 2026

agy review (Antigravity 2.0 v1.0.2) — CLEARED

Re-ran the local agy CLI on this PR (resumed prior conversation 56746887-b20e-4b6c-accb-c54fc3dd441b from the agy reviews of PRs #155 / #156). Verdict: CLEARED.

Verified

  • Sweep: lib-agent.sh + lib-dispatch.sh + dispatcher-tick.sh grepped for AGENT_LAUNCHER reads. No dispatcher-side coupling points other than the wrapper rebind that this PR adds — clean parallel to the per-side-CMD coupling addressed in feat: add AGENT_DEV_CMD / AGENT_REVIEW_CMD per-side overrides (INV-37) #156's followup.
  • Default fallback: \${AGENT_DEV_LAUNCHER:-\$AGENT_LAUNCHER} correctly preserves byte-for-byte back-compat.
  • Per-side guards: each side's launcher is gated against THAT side's AGENT_CMD. Strictly more permissive than the single [INV-37] guard.
  • Tests: all 14 PSL assertions + PSC-S7/S8/S11 needle updates pass.
  • shellcheck: clean. Two SC2034 warnings on the wrapper rebind lines (AGENT_LAUNCHER_ARGV "unused") are the standard shellcheck-can't-track-sourced-functions false positive, consistent with existing patterns in the project.

Out of scope (acknowledged from #156's review)

  • Finding 1 (pre-existing AGENT_*_EXTRA_ARGS stage-vs-side mapping bug) remains pre-existing on origin/main, not introduced by this PR. The conf-example caveat about --trust-all-tools documents the workaround for operators today; the underlying fix is a separate issue with a different fix shape.

@zxkane zxkane merged commit 124f4e9 into main May 26, 2026
3 checks passed
@zxkane zxkane deleted the feat/per-side-launcher branch May 26, 2026 06:57
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