Skip to content

fix(herdr-lab): place --session before the child-argv delimiter and refuse ambiguous delimiter shapes - #1

Merged
Amplify-Logic merged 4 commits into
mainfrom
fm/firstmate-herdr-lab-agent-session-safety-u2
Jul 18, 2026
Merged

fix(herdr-lab): place --session before the child-argv delimiter and refuse ambiguous delimiter shapes#1
Amplify-Logic merged 4 commits into
mainfrom
fm/firstmate-herdr-lab-agent-session-safety-u2

Conversation

@Amplify-Logic

Copy link
Copy Markdown
Owner

Intent

Fix bin/fm-herdr-lab.sh so 'run agent start ... -- ' places --session before the child-argv delimiter (Herdr consumes it instead of the child), reject ambiguous/unsafe delimiter shapes before invoking Herdr, and update the brief/doc wording that claimed the selector is always trailing.

What Changed

  • bin/fm-herdr-lab.sh now splits run arguments on the first literal --: for agent start ... -- <child argv> it injects --session <name> before the delimiter so Herdr consumes it instead of the child process, while ordinary calls still get it appended; everything after the first -- (including further -- tokens) passes through to the child untouched.
  • The run command refuses ambiguous or unsafe delimiter shapes before invoking Herdr: a delimiter on any command other than agent start, an option immediately before the delimiter (a value-taking option could swallow the injected --session), agent start with a delimiter but no child command, agent start without any delimiter, and a caller-supplied --session anywhere before the delimiter. tests/fm-herdr-lab.test.sh pins the accepted and refused shapes.
  • bin/fm-brief.sh and docs/herdr-backend.md no longer claim the session selector is always trailing; they now describe the helper as placing --session on every call (before the child-argv delimiter for agent start), with the brief test updated to match.

Risk Assessment

✅ Low: The follow-up commit cleanly implements the requested first-delimiter-only contract with an exact regression test, removes no safety guard that pre-delimiter checks don't already cover, and leaves the header, docs, and tests consistent with the actual behavior.

Testing

Completed 1 recorded test check.

  • Outcome: ⏭️ skipped across 1 run (23m22s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed ✅
  • ℹ️ bin/fm-herdr-lab.sh:181 - The multiple-delimiter refusal counts literal -- tokens inside the child argv, so a legitimate child command that itself contains -- (e.g. run &lt;s&gt; agent start probe -- npm run build -- --flag) is refused. This contradicts the header comment's claim that 'the opaque child argv itself passes through untouched' (line 26), and the error rationale ('could shift a lifecycle operation past the guard') does not apply to a -- after the first delimiter, since splitting and all guards key only off the first --. If the restriction is intentional fail-closed behavior, consider tightening the header wording; if not, count only pre-child delimiters.
  • ℹ️ bin/fm-herdr-lab.sh:159 - The option-immediately-before-delimiter guard (refusing e.g. agent start --some-flag -- child) also rejects boolean flags that could never swallow the injected --session, and only inspects the single argument adjacent to the delimiter, so a hypothetical greedy multi-value Herdr option two positions back could still absorb the injected selector. As a shell-side heuristic this is a reasonable fail-closed best effort and is documented and tested as such; noting the boundary of the guarantee, no action needed.

🔧 Fix: only treat first -- as delimiter; child argv untouched
✅ Re-checked - no issues remain.

⏭️ **Test** - skipped
  • 🚨 tests failed with exit code 1
  • command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

`agent start ... -- <child argv...>` always got a trailing --session
appended after --, so Herdr never consumed it and the child process
received it instead - meaning Herdr could fall through to the live
default session for agent-start calls. fm_herdr_lab_raw now places
exactly one --session immediately before -- for that shape and still
appends it for every ordinary call. fm_herdr_lab_cli rejects multiple
delimiters, a delimiter on any command other than agent start, and
agent start without a delimiter plus non-empty child command, before
any Herdr call is made. Updated the brief scaffold and backend doc
wording that claimed the selector is always trailing.
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