Skip to content

fix(autonomy,lanes): read the lane-stop gate's enable flag from trusted channels only - #1865

Merged
kyle-sexton merged 4 commits into
mainfrom
fix/1784-channel-f-enable-flag
Aug 3, 2026
Merged

fix(autonomy,lanes): read the lane-stop gate's enable flag from trusted channels only#1865
kyle-sexton merged 4 commits into
mainfrom
fix/1784-channel-f-enable-flag

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the remaining P1 of #1784. lane-stop-gate.sh read
CLAUDE_PLUGIN_OPTION_LANE_STOP_GATE_ENABLED straight off the environment — channel B of
docs/conventions/hook-config-delivery, whose rule 3 requires channel F for a safety-critical
optional-with-default toggle, and whose fact 4 records that an unconfigured key lets a watched
repository's own .claude/settings.json env block populate CLAUDE_PLUGIN_OPTION_* freely. A
gate whose enablement the watched repository controls is not a gate.

Per-key resolution is now, in precedence order: managed settings (fixed root-owned paths plus
managed-settings.d/ drop-ins) ▷ the per-session arm record ▷ the user settings.json located
only from the hook's own plugins/cache install anchor ▷ the in-script defaults. No path any of
these reads is env-derived; the managed-settings platform comes from uname -s, not the
repo-settable $OSTYPE, and the resolved primary is asserted absolute so it can never become a
cwd-relative (repo-plantable) path. The env mirrors are never read as values — presence alone only
decides whether to evaluate and whether to surface a visible once-per-session notice.

Because a hook cannot observe --settings (channel F's documented residual), the per-session
opt-in the launcher previously shipped that way needed a trusted replacement rather than deletion.
New channel G: hooks/lane-stop-gate-arm.sh writes a per-session record under the plugin's own
install-derived data directory, and the session carries only a random record id. The id is a
capability pointer, never authority — shape-validated before any path use, looked up only in the
install-anchored store, claimed by the first presenting session so a replay is refused, and
TTL-bounded. lane-launcher.sh arms a gate-requesting lane at launch and fails closed: a lane that
cannot be armed is skipped with an error rather than launched silently ungated.

The two sibling P2s of #1784 landed separately in #1851 and are untouched here; this branch was
rebased onto them and both fixes were confirmed intact (lane_json_field's has($k) check and the
marker-consumption ledger).

Fixes from independent review

A fresh-context audit of this branch, run with the rationale withheld, found four defects in the
work; all four are fixed here:

  • Keying every settings read on the marketplace-qualified id left the managed scope contributing
    no verdict without a plugins/cache anchor
    — silently disabling the org-mandate path on the
    one install class (--plugin-dir) for which it is the only enable path, and on whose
    availability the arm helper's refusal to arm there is premised. An unanchored install now matches
    on the manifest name beside the hook; anchored installs keep their exact-id match.
  • The launcher accepted a partial arm: one helper succeeding marked the lane armed, though each
    install writes into its own store and the launcher cannot tell which one the session loads. Every
    discovered helper must now arm.
  • The presence preflight read discovery as find_gate_arm_scripts | grep -q ., which under
    pipefail takes SIGPIPE on the producer's second write
    — so a machine carrying two autonomy
    installs read as "no helper found" and was refused a gate-requesting launch outright.
  • An empty configured sentinel silenced the token channel while the block reason still
    instructed the agent to emit an empty token on its own line. It now falls back to the default.

The audit also found a defect that is not fixed here and is filed as #1883: every hook built
on hook::buffer_stdin can be disengaged by a repo env block setting
CLAUDE_PLUGIN_OPTION_STDIN_READ_TIMEOUT below the read's practical resolution
(lib/hook-utils.sh rejects exact zero but accepts 0.000001). It is pre-existing on main, is
not the enable flag #1784 names, and lives in a shared library materialized into 16 plugin copies —
a fleet-wide change that deserves its own review rather than riding this PR.

Test plan

Full gate battery run against the shipped tree:

  • plugins/autonomy/hooks/lane-stop-gate.test.sh — PASS=72 FAIL=0
  • plugins/claude-ops/skills/lanes/scripts/lane-launcher.test.sh — PASS, 169 cases
  • shellcheck --rcfile .shellcheckrc on all six changed shell files — clean
  • scripts/check-changelog-parity.sh --check, --check-order, --check-bump origin/main
  • scripts/check-changed-skills.sh origin/main
  • scripts/check-shell-portability.sh origin/main — no unexcused GNU-only constructs
  • markdownlint-cli2 on the changed docs — 0 errors

Regression evidence, observed rather than asserted:

  • Substituting origin/main's lane-stop-gate.sh under the new suite yields PASS=54 FAIL=14,
    with the pinned attack case reporting env-only ENABLED=true wrongly engaged the gate (channel-B authority).
  • The four assertions added for the review fixes yield FAIL=4 against this branch's own pre-fix
    commit, including the empty-token nudge reproduced verbatim.
  • The multi-install launcher case fails against the pre-fix launcher, which is how the SIGPIPE
    defect above was found.

Related

Closes #1784
Refs #1883

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 6m 40s —— View job


Security review — PR #1865

  • Gather context (diff, changed files)
  • Review lane-stop-gate.sh / lane-stop-gate-lib.sh / lane-stop-gate-arm.sh (trusted-channel config derivation)
  • Review lane-launcher.sh changes
  • Review test files for security-relevant assertions
  • Review docs/config changes for contract issues
  • Post findings

Scope: the trusted-config redesign of the lane-stop gate (lane-stop-gate.sh, lane-stop-gate-lib.sh, lane-stop-gate-arm.sh) and the launcher's new arming step (lane-launcher.sh). No GitHub Actions workflows changed in this PR.

The core fix is sound: the gate no longer trusts the bare CLAUDE_PLUGIN_OPTION_LANE_STOP_GATE_* environment for authority (only managed settings, an install-anchored arm record, or install-anchored user settings.json can enable it), the arm-id is shape-validated before any path use, and the untrusted-enable case now gets a visible notice instead of a silent bypass. I found no way for a watched repository's own .claude/settings.json env block to re-enable the gate or forge/redirect an arm record — the test suite's Case 24–26 pins line up with the code.

Three narrower issues surfaced from the trust/data-flow analysis:


1. IMPORTANT (confidence: medium) — Launcher blindly executes every installed "autonomy"-named plugin's arm helper, not just the marketplace(s) referenced in the lane's own settings.

find_gate_arm_scripts() (lane-launcher.sh#L558-L571) globs <config>/plugins/cache/*/autonomy/*/hooks/lane-stop-gate-arm.sh across every installed marketplace, and arm_stop_gate() (lines 595-599) runs bash "$script" on every match found, unconditionally — regardless of which specific autonomy@<marketplace> key the lane's own settings actually requested. The same arm id, cwd, sentinel and marker are handed to each one (lines 609-612).

Failure scenario: an operator adds a second, less-trusted marketplace for an unrelated plugin, but that marketplace also happens to ship (or a compromised/typosquatting update later ships) a plugin literally named autonomy with a hooks/lane-stop-gate-arm.sh at that path. The launcher will execute that script — with the operator's real lane cwd/sentinel/marker as arguments — even though the lane's settings never referenced that marketplace's plugin at all. The design comment ("arming all of them is idempotent") treats this as harmless because a legitimate arm helper is idempotent, but it doesn't defend against an arm helper that isn't the legitimate one.

Scoping execution to only the marketplace-qualified key(s) actually present in settings.pluginConfigs (parsed from the same JSON arm_stop_gate already has) would close this without behavior change for the single-marketplace case.


2. SUGGESTION (confidence: low-medium) — Arm id travels via --settings argv for the life of the background claude --bg process.

The arm id is injected into --settings and passed as a claude CLI argument (lane-launcher.sh#L681), which stays visible in that process's argv (ps//proc/<pid>/cmdline) for as long as the background lane runs. docs/conventions/hook-config-delivery's own decision rule 5 says a sensitive value should "never [ride] argv (visible in process listings)" (README.md), yet channel G's own id is exactly that. The design already reasons the id "carries no authority" and is claimed by the first Stop event (typically fast), which meaningfully limits this — but any other process co-resident as the same OS user (a different, less-trusted project/session) can read the id from argv and race to present it to its own Stop hook before the legitimate lane's first stop, potentially preempting the legitimate session's claim and knocking its gate back to disabled (fail-open, so the practical effect is "gate silently doesn't engage for the targeted lane," not privilege escalation). Worth a one-line residual note in the channel-G table entry if not already covered by the "no (plaintext record in data dir)" secrecy column.


3. SUGGESTION (confidence: low) — Arm-record claim can be skipped entirely when session_id is empty, leaving the record permanently unclaimed.

In gate_load_arm_record() (lane-stop-gate.sh#L158-L172): if the hook payload's session_id is empty/missing on an unclaimed record, neither the [[ -n "$claimed" ]] branch nor the elif [[ -n "$SESSION_ID" ]] claim branch fires, so GATE_ARM_JSON is set (gate treated as valid/enabled) without ever writing a session_id claim. The record stays unclaimed indefinitely and any later presenter — including a different session that later reuses/replays the same id — would also load it successfully, defeating the "first presenter claims, a replayed id is refused" guarantee documented in the header. session_id is populated by the Claude Code harness itself (not attacker-controlled repo content), so this is a low-likelihood edge case rather than a demonstrated attacker path, but it isn't covered by the test suite (no case exercises an empty session_id with a valid arm record). Consider treating empty SESSION_ID as "cannot claim" and only honoring the arm record when a session id is present to bind to.


No issues found in: managed-settings path resolution (correctly uname -s-based, not $OSTYPE; absolute-path-only), arm-id shape validation before path construction (no traversal), sentinel regex escaping, marker consumption ledger, or shell-injection surfaces (all CLI args passed via bash arrays, no eval/string-built commands).

kyle-sexton and others added 3 commits August 3, 2026 01:14
…s only; launcher arms lanes

The gate read CLAUDE_PLUGIN_OPTION_LANE_STOP_GATE_* off the bare environment
(hook-config-delivery channel B), whose unset case a watched repository's own
.claude/settings.json env block populates freely: the repo could decide whether
the gate runs, weaken the sentinel, or point the marker at its own file. Config
now resolves managed settings > per-session arm record > user settings (located
from the hook's own plugins/cache install anchor) > in-script defaults; the env
mirrors are never authority, and an env-only enablement claim surfaces a
visible once-per-session notice instead of a silent disengage.

Because a hook cannot observe --settings (channel F's residual), the launcher's
per-session opt-in moves to an operator-side arm record: lane-launcher.sh runs
autonomy's new hooks/lane-stop-gate-arm.sh at launch and injects a random
lane_stop_gate_arm_id; the gate treats the env-delivered id as a capability
pointer only (shape-validated, anchored-store lookup, first-session claim, TTL,
terminal consumption). A gate-requesting lane that cannot be armed is skipped —
fail closed at launch, fail open at stop time.

Convention doc gains channel G (operator-side arm record), the F+G pairing in
decision rule 3, and the autonomy adopters row.

Fixes #1784

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Independent security review of the diff found five issues; the two substantive
ones are fixed here:

P1 — managed-settings path selection branched on $OSTYPE, a bash variable a repo
env block can set: the highest-precedence (managed) scope could be suppressed
(unknown OSTYPE → empty) or, on POSIX, relocated to a cwd-relative repo-plantable
path. Selection now reads `uname -s` (process-intrinsic, matching the Python
exemplar's sys.platform), and the resolved primary is asserted absolute.

P3 — the arm record was consumed on the first completion-signaled or post-nudge
stop, silently disarming the gate for every later cycle of a one-session /loop
lane, and the next stop emitted a false "env channel only" notice. The record is
no longer consumed on a stop: it lives for the claiming session (bound by the
session-id claim), retired by TTL plus the launcher's relaunch sweep. The
not-honored notice now distinguishes a spent/expired arm id (points at re-arming)
from an env-only enablement claim (the attack).

Also: corrected the CLAUDE_PLUGIN_DATA ledger-fallback comment/CHANGELOG (its
failure direction is granted, not withheld — harmless because the fallback
reaches only the marker channel, an agent-writable declaration; enablement and
the arm record use the install-anchored store); moved the launcher's
missing-helper check into validate_launch_inputs so restart preflights it before
stopping a healthy lane; strengthened the traversal test to assert the id-shape
guard at the path boundary (was vacuous); added a unit case pinning that managed
selection ignores hostile $OSTYPE and never yields a relative path; added the
multi-cycle coverage case (a later stop of an armed session stays gated).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… wrapped clause

The typos gate rejects "unparseable" in favor of "unparsable". Reflow the
channel-G paragraph's mid-sentence break in the same pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton force-pushed the fix/1784-channel-f-enable-flag branch from b0e19fd to 6447bf8 Compare August 3, 2026 05:28
@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 19s —— View job


Security review — PR #1865

  • Gather context (diff since last review, changed files)
  • Review lane-stop-gate.sh / lane-stop-gate-lib.sh / lane-stop-gate-arm.sh
  • Review lane-launcher.sh changes (arming step)
  • Check whether prior findings were addressed
  • Review test files for security-relevant assertions
  • Post findings

Scope: same trusted-config redesign as the prior review on this PR (lane-stop-gate.sh, lane-stop-gate-lib.sh, lane-stop-gate-arm.sh, lane-launcher.sh's arming step). The only commit added since that review (6447bf8) is a comment-only typo fix (unparseableunparsable) — no security-relevant code changed. All three previously-reported findings are still present, unaddressed in the current HEAD (6447bf8c):


1. IMPORTANT (confidence: medium) — Launcher still executes every installed "autonomy"-named plugin's arm helper across all marketplaces, not just the one the lane's own settings requested.

find_gate_arm_scripts() (lane-launcher.sh#L558-L576) still globs <config>/plugins/cache/*/autonomy/*/hooks/lane-stop-gate-arm.sh unscoped by marketplace, and arm_stop_gate() (lines 604-609) still runs bash "$script" on every match unconditionally, regardless of which specific autonomy@<marketplace> key the lane's settings actually referenced. The header comment's rationale is unchanged ("arming all of them is idempotent"), which addresses a legitimate second install but not a malicious/typosquatted one shipped under a second, less-trusted marketplace. No test was added exercising the multi-marketplace case (lane-launcher.test.sh's new coverage only adds single-marketplace autonomy@test-marketplace fixtures).

Scoping execution to only the marketplace-qualified key(s) present in the lane's settings.pluginConfigs (already parsed by lane_requests_stop_gate/gate_option_from_settings) would close this without behavior change for the single-marketplace case.


2. SUGGESTION (confidence: low-medium) — Arm id still travels via --settings argv for the life of the background claude --bg process.

cmd+=(--settings "$settings") (lane-launcher.sh#L681) is unchanged; the arm id remains visible in ps//proc/<pid>/cmdline for the lane's whole run, which is the exact pattern docs/conventions/hook-config-delivery's decision rule 5 says to avoid for a sensitive value. The design's own reasoning (id "carries no authority," claimed by the first Stop event) still meaningfully limits impact to "gate silently doesn't engage for the targeted lane," not privilege escalation — this remains a suggestion, not a blocker.


3. SUGGESTION (confidence: low) — Arm-record claim in gate_load_arm_record() can still be skipped when session_id is empty, leaving the record permanently unclaimed.

lane-stop-gate.sh#L158-L172: when claimed and SESSION_ID are both empty, neither the [[ -n "$claimed" ]] branch nor the elif [[ -n "$SESSION_ID" ]] branch fires, so GATE_ARM_JSON is still set without ever writing a claim — the record stays unclaimed and a later replay would also load successfully. session_id is harness-populated (not attacker-controlled), so likelihood remains low, and the test suite still has no case exercising an empty session_id against a valid arm record.


No new issues found in the delta since the last review (there is none of substance — only the typo-fix commit). No issues found in: managed-settings path resolution, arm-id shape validation before path construction, sentinel regex escaping, marker consumption ledger, or shell-injection surfaces — consistent with the prior pass.

@kyle-sexton kyle-sexton changed the title wip(autonomy): lane-stop gate reads its enable flag from trusted channels only fix(autonomy,lanes): read the lane-stop gate's enable flag from trusted channels only Aug 3, 2026
… arm to succeed

Independent audit of the trusted-channel work surfaced four defects in it.

Keying every settings read on the marketplace-qualified id left the managed
scope contributing no verdict without a plugins/cache anchor, so an org mandate
silently did not apply to --plugin-dir installs — the one class for which
managed settings are the only enable path, and on whose availability the arm
helper's refusal to arm there is premised. An unanchored install now matches on
the manifest name beside the hook; anchored installs keep their exact-id match.

An empty configured sentinel silenced the token channel while the block reason
still told the agent to emit an empty token on its own line. It now falls back
to the default.

The launcher accepted a partial arm: one helper succeeding marked the lane
armed even though each install writes into its own store and the launcher
cannot tell which one the session loads. Every discovered helper must now arm.

The preflight read discovery as `find_gate_arm_scripts | grep -q .`, which
under pipefail takes SIGPIPE on the producer's second write — so a machine with
two autonomy installs read as "no helper found" and was refused a
gate-requesting launch outright.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 17s —— View job


Security review — PR #1865

  • Gather context (diff since last review, changed files)
  • Determine what changed since the last review (6447bf812e97d3)
  • Re-check the three previously-reported findings against new HEAD
  • Review the new commit's own changes for fresh security issues
  • Post findings

Scope: the delta since the prior review — one new commit, 12e97d3 ("reach managed settings unanchored; require every arm to succeed"), touching lane-stop-gate-lib.sh, lane-stop-gate.sh, and lane-launcher.sh's arming step.

The new commit's own changes are sound and introduce no new issues. gate_resolve_install()/gate_resolve_plugin_name() (lane-stop-gate-lib.sh#L68-L89) read the manifest from the same BASH_SOURCE-derived trust anchor as everything else (not the watched repo), so a --plugin-dir install's name-based fallback in gate_settings_option() (lines 178-192) can only ever be reached against managed settings (already root-owned/trusted) — gate_user_settings_file() and gate_trusted_data_dir() still hard-fail unanchored, so this doesn't open a new path to the user-settings or arm-record scopes. The anchored case is unchanged (still an exact marketplace-qualified match). The --gate-arm-script all-must-succeed change and the grep -q .→command-substitution fix in lane-launcher.sh are correctness/robustness fixes with no adverse trust-boundary effect I could find.

All three previously-reported findings remain unaddressed at 12e97d3 — none of the changed lines touch them:

1. IMPORTANT (confidence: medium) — Launcher still executes every installed "autonomy"-named plugin's arm helper across all marketplaces, not just the one the lane's settings requested. find_gate_arm_scripts() (lane-launcher.sh#L558-L570) is still unscoped by marketplace, and arm_stop_gate() (lines 593-609) still runs bash "$script" on every match, with the real lane cwd/sentinel/marker, regardless of which autonomy@<marketplace> key the lane's own settings referenced. Worth noting: this commit's new "every helper must arm or the lane is skipped" rule (added specifically to close a different gap — a stale/inactive install's helper silently reporting success) is a reasonable fail-closed choice on its own, but it slightly amplifies this finding's footprint: a second, less-trusted marketplace's typosquatted autonomy plugin can now also deny gated launches outright (by exiting non-zero) in addition to the original concern of running with the real lane's arguments. New test case ("arm: discovery finds every installed marketplace's helper", lane-launcher.test.sh) confirms and documents the unscoped-discovery behavior rather than fixing it. Scoping execution to the marketplace-qualified key(s) present in settings.pluginConfigs (already parsed by gate_option_from_settings) would close this.

2. SUGGESTION (confidence: low-medium) — Arm id still travels via --settings argv (lane-launcher.sh#L691) for the life of the background claude --bg process, contrary to docs/conventions/hook-config-delivery decision rule 5. Unchanged since the last review; impact remains bounded to "gate silently doesn't engage for the targeted lane," not privilege escalation.

3. SUGGESTION (confidence: low)gate_load_arm_record() (lane-stop-gate.sh#L145-L172) can still skip the claim when SESSION_ID is empty, leaving the record permanently unclaimed and re-loadable by a later replay. Unchanged since the last review; session_id is harness-populated, so likelihood remains low, and there's still no test exercising this edge case.

No new issues found. No issues in: managed-settings path resolution, arm-id shape validation, sentinel regex escaping, marker-consumption ledger, or shell-injection surfaces.

@kyle-sexton
kyle-sexton marked this pull request as ready for review August 3, 2026 14:05
@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 27s —— View job


Code review — PR #1865

  • Gather context (diff, changed files)
  • Review lane-stop-gate-lib.sh (new trust/resolution logic)
  • Review lane-stop-gate.sh changes
  • Review lane-stop-gate-arm.sh (new)
  • Review lane-launcher.sh arming changes
  • Review test coverage
  • Review docs/config/changelog changes
  • Post findings

Scope: this is the code-review lane — correctness, design, error handling, tests, and documentation, per REVIEW.md. Trust-boundary/authorization findings (unscoped marketplace arm-helper execution, arm-id-in-argv exposure, empty-session_id claim skip) are the security lane's scope and are covered there already; I didn't re-report them.

Overall this is a well-structured trust redesign: the precedence chain (managed ▷ arm record ▷ user settings ▷ default) is implemented consistently across lane-stop-gate.sh/lane-stop-gate-lib.sh/lane-stop-gate-arm.sh, the fail-open (hook) vs. fail-closed (arm helper/launcher) split is applied correctly at every exit path I traced, and the 40-case lane-stop-gate.test.sh + the launcher's new arm cases pin the documented behavior (including two good regression-shaped cases: SIGPIPE-safe sentinel matching on a 120KB message, and the same-second/same-size marker-recreation boundary). Docs (hook-config-delivery README/CHANGELOG, plugin CHANGELOGs, config.md) are consistent with the code and with each other, and the version bumps (autonomy 0.12.0, claude-ops 0.26.0, contract 1.2.0) match the stated bump rules.

One finding from the design/error-handling pass:


SUGGESTIONarm_stop_gate() (lane-launcher.sh#L593-L619) leaves orphaned arm records behind when arming a lane across multiple discovered installs partially fails.

When two (or more) autonomy installs are discovered, the loop at lines 601-605 runs every helper unconditionally; if helper A succeeds (writing a real arm record under its own install-derived lane-arms/ directory) and helper B then fails, arm_stop_gate returns 1 and the lane is skipped — but install A's already-written record is never cleaned up. REVIEW.md's error-handling check calls for "an atomicity mechanism... spanning" writes to two or more related state locations, or a compensation step, so an interruption doesn't leave state no code path expects; here there's neither a spanning transaction nor a compensating delete for the partially-succeeded write.

Practical impact is low — the record can only ever be loaded by a session presenting its exact random id, and the lane launch that would have carried that id was aborted, so the record is inert, not a live grant. But it is unbounded litter: it sits in <config>/plugins/data/<id>/lane-arms/ past its 7-day TTL until some other future arm call to that same install's store happens to run the find -mtime +7 -delete sweep (lane-stop-gate-arm.sh#L121-L123) — which may never happen for an install that stops being the "winning" one. It's also untested: the multi-install arm case in lane-launcher.test.sh (lines 774-801) uses logging stubs that never write a real record file, so there's no coverage either way for whether a partially-armed install's on-disk record is cleaned up or left behind.

A cheap fix: track which helper invocations succeeded (the id + which install's arm dir), and on a later failure in the same arm_stop_gate call, best-effort rm -f the records already written before returning 1. Not blocking given the inertness of the leftover record, but worth closing for hygiene, especially since the design explicitly reasons about compensating/self-cleaning behavior everywhere else in this PR (TTL sweeps, ledger cleanup, best-effort claim writes).


@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 27s —— View job


Security review — PR #1865

  • Check for new commits since the last review
  • Confirm HEAD SHA against last-reviewed SHA

HEAD is still 12e97d38b88da4e6eb524301319150b1eddc9872 — the exact commit already covered by the most recent review on this PR. No new commits have landed since then, so there is no new diff to review.

All prior findings stand as last reported at that SHA:

  • IMPORTANT (confidence: medium) — launcher still executes every installed "autonomy"-named plugin's arm helper across all marketplaces, unscoped by the lane's own settings.pluginConfigs key (lane-launcher.sh#L558-L570, #L593-L609).
  • SUGGESTION (confidence: low-medium) — arm id still travels via --settings argv for the life of the background lane process (lane-launcher.sh#L691).
  • SUGGESTION (confidence: low)gate_load_arm_record() can still skip the claim when SESSION_ID is empty, leaving the record permanently unclaimed (lane-stop-gate.sh#L145-L172).

Nothing further to add — re-request review once new commits land, or address finding #1 (the only IMPORTANT-severity item) to close this out.

@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 3, 2026 14:06
@kyle-sexton
kyle-sexton merged commit 3a9c53c into main Aug 3, 2026
37 checks passed
@kyle-sexton
kyle-sexton deleted the fix/1784-channel-f-enable-flag branch August 3, 2026 14:06
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 12e97d38b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/autonomy/hooks/lane-stop-gate.sh
Comment thread plugins/claude-ops/skills/lanes/scripts/lane-launcher.sh
Comment thread plugins/claude-ops/skills/lanes/scripts/lane-launcher.sh
kyle-sexton added a commit that referenced this pull request Aug 3, 2026
Reconciles two out-of-session merges, #1848 and #1865. Conflicts were
confined to claude-config's plugin.json and CHANGELOG.md; the
audit-instructions surfaces this branch owns merged clean because neither
incoming PR touched them.

plugin.json takes 0.21.0. Main's 0.20.1 is a patch beneath this branch's
minor, so the branch version already subsumes it and no re-bump is owed.
CHANGELOG keeps both blocks with [0.21.0] above [0.20.1], preserving
newest-first order.

Both version literals were asserted by hand after the merge rather than
trusted to the resolution: identical strings on either side auto-merge
clean and wrong, and criteria.md's frontmatter is exactly that shape — it
carries 1.8.0 here against 1.7.0 on main and was never conflicted, so
nothing would have reported a silent revert. Confirmed 1.8.0, with both new
rows, the target-keyed high fence, and the catalog-read --opinion clause
intact.

Neither incoming PR interacts semantically with I21 or I22. #1848's
claude-config changes are confined to audit-pass (its evals and run
contract) and name no I-row, no OPINION enablement, and no criteria surface.
#1865 edited the lanes config row this branch's standing gate cites, but
only the settings row governing lane-stop-gate arming; the model and effort
rows and the sample lane block are unchanged at the same lines, so every
standing-gate adjudication still holds and the new row prescribes no effort
level or model lane to become a candidate of its own.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 5, 2026
…ed settings exclusion (#1934)

## Summary

Roster row 152 (server-managed settings) close-out. The lane-stop gate's
org-veto layer
(`plugins/autonomy/hooks/lane-stop-gate-lib.sh`,
`gate_managed_settings_files`) reads only endpoint
managed-settings paths plus `managed-settings.d/` drop-ins.
Server-managed settings

([code.claude.com/docs/en/server-managed-settings](https://code.claude.com/docs/en/server-managed-settings))
are a second admin-controlled managed source at the same top precedence
tier — and when the server
delivers any keys, endpoint sources are ignored, not merged — so an org
managing exclusively via the
server channel cannot veto lanes.

The exclusion is correct, not a gap: the veto's trust rationale is
root-owned paths a repo cannot
forge, and the server channel's only on-disk artifact is the
user-writable cache
`~/.claude/remote-settings.json`, which fails that test. The live page
itself calls the channel
"a client-side control, not a security boundary" (verified 2026-08-04).
This PR makes the exclusion
documented and deliberate rather than silent:

- Comment at the exclusion site in `lane-stop-gate-lib.sh` stating the
trust constraint.
- README precedence-list note with the same constraint, directing orgs
on the server channel to also
  deliver an endpoint `managed-settings.json` to veto lanes.
- autonomy 0.12.1 + changelog entry. No behavior change.

No linked issue

## Related

- Roster row 152, doc-alignment campaign (batch B6 finding, orchestrator
ruling: deliberate exclusion, documented)
- #1784 / #1865 (trusted-channel config resolution that established the
veto's trust rationale)

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 9, 2026
…h an atomic claim (#2065)

## What

`plugins/autonomy/hooks/lane-stop-gate.sh` bound an arm record to its
first
presenting session with a read-then-write, and honored the record from
the
value read *before* that write. Two Stop invocations presenting the same
fresh
arm id both read it unclaimed, both wrote, and the last `mv -f` won — so
**both
honored the arm for that event**, while the loser, possibly the
legitimate
lane, was refused on every later stop and **ran ungated from then on**.

`mv -f` made the rename atomic; the read → modify → write around it was
not,
and `GATE_ARM_JSON` was assigned the pre-claim JSON unconditionally, so
the
record was honored without ever re-reading what actually persisted.

## How

- The claim is an **exclusive create** — `set -o noclobber` on a `>`
redirection, i.e. `O_CREAT|O_EXCL` — of a `<record>.claim` sidecar
holding the
owning session id. Same primitive `statusline-tee.sh` already uses, and
its
recorded reason for avoiding `flock` (absent on macOS) applies here too.
Both
the `umask` and the option are scoped inside a subshell; verified
empirically
  that neither leaks to the caller.
- `GATE_ARM_JSON` is assigned **only past the ownership verdict**. An
unowned
record contributes no config at all, which is what stops a replaying
session
  from being honored.
- A claim file exists only because some process won that create, so an
**empty**
one is that winner caught between its create and its write — not an
ownerless
  record. Reading it in that instant would hand one fresh arm to every
concurrent presenter and reopen the race a few microseconds wide, so the
owner
  read is retried over a bounded budget.
- **Fail direction preserved in both directions.** An unwritable store
leaves no
claim, and a claim whose owner never lands exhausts the budget; both
**honor**
the arm — the gate stays ON. A legitimate lane silently losing its gate
is the
harm, never an extra gated stop. Refusing a durably ownerless claim
would make
the record permanently unclaimable, which is the original harm in a new
shape.
- **Compatibility.** A record claimed before the sidecar existed carries
its
owner in the record itself and has no sidecar; that field stays
authoritative,
so an upgrade cannot let a second session claim a record already bound
to a
  live lane. Nothing writes it any more.
- `lane-stop-gate-arm.sh` clears the sidecar **before** it (re)writes a
record,
so a re-armed id starts unclaimed rather than staying bound to a dead
session.
The clear precedes the write deliberately: a crash between the two then
leaves
the old record with no claim (an extra gated stop) instead of a fresh
record
beside a stale claim, which would refuse the new lane on every stop
until the
record aged out. The record it writes is built fresh by `jq -n` and
never
carries an owner, so no owner can be orphaned. The gate's TTL sweep
drops
  record and sidecar together.
- The claim path carries **the record path's own `[[ -f ]]` asymmetry**.
It is a
second predictable name in the same store, and without a type guard a
planted
FIFO took the write with no reader and hung the entire Stop event —
which the
harness resolves by allowing the stop, i.e. an ungated lane on every
attempt.
Anything at that path this hook did not write now decides nothing and
the arm
  is honored.
- One spelling of the sidecar path (`gate_arm_claim_path` in
`lane-stop-gate-lib.sh`) so the gate and the arm helper cannot diverge.

## Verification

`bash plugins/autonomy/hooks/lane-stop-gate.test.sh` — **83 pass, 0
fail**,
including six new cases:

| Case | Asserts |
| --- | --- |
| 41 | a pre-claimed record is refused for a non-owner and honored for
its persisted owner |
| 42 | re-arming clears the previous lane's claim, and the relaunched
lane can claim it |
| 43 | a record claimed before the sidecar existed stays bound to its
legacy owner |
| 44 | six presenters racing one fresh arm id — exactly one is honored,
and the persisted claim names that session |
| 45 | an ownerless claim still honors the arm (fail direction: gate ON)
|
| 46 | a non-regular file at the claim path neither hangs the hook nor
loses the gate |

Every claim-ownership case pins the **exit code** as well as the
decision; a
stdout-only assertion would pass a hook that started exiting 127 in
silence,
which is the regression shape this effort has already hit once.

Nothing here was reasoned about where it could be measured:

- An **independent fresh-context verifier**, given the finding and the
branch
but not the author's rationale and told to refute, ran the *new* test
file
against `origin/main`'s hooks: **76 pass / 6 fail**, among them `6 of 6
concurrent presenters were honored for one fresh arm id`. The race
reproduces
on main and the new cases are genuine regression tests, not decoration.
It
also ran 150 × 6 function-level and 25 × 6 end-to-end concurrency trials
  against the fix: zero multi-winner, zero mismatch.
- The **empty-claim window** was demonstrated, not inferred: with a
pause
  injected between the exclusive create and the owner write, six of six
concurrent presenters were honored. With the bounded re-read, that same
  injected pause yields one of six across repeated runs, and a pause
deliberately exceeding the budget over-gates (four of four honored)
rather
  than leaving any lane ungated.
- The **FIFO hang** was demonstrated the same way: with the type guard
removed
from a scratch copy the hook never returns; with it, the hook completes
and
  the gate stays on.

Also green from the worktree root: changelog parity (`--check`,
`--check-order`,
`--check-bump` vs `origin/main`), shell-portability lint,
`check-silent-skips.sh`,
`check-changed-skills.sh origin/main`,
`check-contract-clause-coverage.py`,
`validate-plugins.sh`, ShellCheck, shfmt, markdownlint.

## Known and deliberate

- Exactly one session ends up with the arm, so a **leaked arm id still
strands
the loser** — that is the first-presenter-wins contract, not the race,
and it
is unchanged by this PR. What changes is that the binding is now decided
once,
  atomically, instead of by whichever writer happened to land last.
- A **mid-lane downgrade** to a version that predates the sidecar would
ignore a
live claim, since 0.13.1 records the owner only in the sidecar. The
upgrade
  direction is handled and pinned by case 43.

## Related

- Review finding `PRRT_kwDOTCGFQM6WACHN` (#1865), P1, verdict REAL.
- `PRRT_kwDOTCGFQM6T8HlP` (#1694) is already fixed and deliberately
**not**
folded in: its one open sub-ask (a missing C3 changelog line) harms no
install
  and is parked for the operator.

No linked issue

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Aug 9, 2026
…gate arming to the installs that asked (#2050)

Three bot-filed P2 defects in the `claude-ops` lanes launcher, all
verified to reproduce at `origin/main` and to stop reproducing here.
Every fix is covered by a new assertion that FAILS against the pre-fix
launcher and passes against this one.

## 1. The launch-commit marker key was digested in the wrong repository
(#1383)

`git hash-object` uses the object format of whatever repository it
resolves. The launcher called it **unscoped**, so it keyed on the
*caller's* format while taking the toplevel from the repository `--repo`
names. Reached from a SHA-1 working directory, a SHA-256 target produced
a 40-character key, while `skills/lanes/context/refresh.md`'s probe runs
inside that checkout and computed the 64-character one — the launcher
wrote its marker to a directory the probe never reads and staleness
detection was silently off.

The comment above the key asserted the two sides agree because both call
`git rev-parse --show-toplevel`. That settles the *path* and says
nothing about the *digest*, so the invariant it claimed did not hold.

Both digests are now taken with `-C "$REPO"`. The anchor is `$REPO`
(guaranteed by `resolve_repo` to be an existing directory) rather than
the hashed `$top` (a string git handed back) — anchoring on a path that
may not exist would fail the digest into the `unkeyed` fallback and
collapse every such repo onto one key.

**Scope is broader than the filed report:** `restart-consumer.sh`
derived its ledger key the same unscoped way and is fixed with it. The
hand-recompute snippets in the README, the changelog, and `refresh.md`
already run inside the target repository and were correct as written;
they are untouched.

## 2. An explicitly empty stop-gate marker was read as an absent one
(#1865)

`gate_option_from_settings` ended `select(type == "string") ] | last //
empty`, which prints nothing for an explicit `""` and nothing for an
absent key. `[[ -n "$marker" ]]` then dropped `--marker` for both, the
arm record carried no marker key at all, and `lane-stop-gate.sh`'s
precedence (managed ▷ arm record ▷ user settings ▷ default) walked past
it to the user-level marker — where a marker file left over from another
lane can authorize a stop this lane never signaled.

A `v:` prefix now carries "the lane set this" through the shell, so an
explicit empty value reaches the helper as `--marker ""`,
`lane-stop-gate-arm.sh` records `{"marker": ""}`, and the gate's `[[ -n
"$MARKER" ]]` guard leaves the marker channel off instead of falling
through.

**The sentinel is deliberately not symmetric.** `lane-stop-gate.sh`
substitutes the default token for an empty sentinel, so emptiness is not
a configured value there; recording one would buy no behavior change
while shadowing the user-level sentinel. An empty sentinel is therefore
still treated as absent, and a fixture pins that asymmetry.

## 3. The stop-gate arm id reached installs that never asked for it
(#1865)

Arming keyed off an any-quantifier over the `autonomy` / `autonomy@*`
namespace, then injected `lane_stop_gate_arm_id` into **every** entry in
it, and option extraction took its last match from any entry rather than
a requesting one.

The gate never treats this channel as a trusted verdict in either
direction, so an id landing on an entry set to `false` was not
overriding that `false`. What it did do is mark installs the lane never
asked to arm — leaving the settings handed to `claude` an inaccurate
record of what was requested, and letting a non-requesting entry's
marker reach the arm call. One shared filter now defines "an entry that
requested the gate", and detection, option extraction, and injection all
use it.

Arming every discovered helper script is unchanged and deliberate.

## Tests

`lane-launcher.test.sh` grows a SHA-256 cross-format marker fixture
(skipped where git cannot create a SHA-256 repository) and four
gate-arming fixtures. Against the pre-fix launcher with this test file,
six assertions fail:

- `marker: written under the TARGET repo's object-format key`
- `marker: nothing is written under the caller-format key`
- `arm: an explicitly empty marker still reaches the helper`
- `arm: options come from the requesting entry`
- `arm: a disabled sibling's marker never reaches the helper`
- `arm: the explicitly-disabled entry receives no arm id`

All six pass here; the suite is 193 assertions, 0 failures.

## Verification (independent re-run)

The pre-fix control was reproduced by copying the scripts directory to a
scratch path, replacing
`lane-launcher.sh` with `origin/main`'s, and running this branch's
**unchanged** test file against
it: `lane-launcher.test: FAIL — 6 case(s) failed` there, `PASS — 193
cases` here. The six failures
are exactly the list above, so no fixture is passing on both trees.

The SHA-256 block **executed** rather than skipping — `git version
2.54.0.windows.1` creates
`--object-format=sha256` repositories, and cases 158-160 report PASS.
The skip guard remains because
the format is not universally compiled in.

Beyond the arm stub: `lane-stop-gate-arm.sh` invoked directly with
`--marker ""` writes
`"marker": ""` into the arm record, while omitting the flag writes no
`marker` key at all. The
launcher's explicit-empty distinction therefore survives to the gate,
whose `gate_option` returns the
empty string (via the same `v:` idiom) rather than falling through to
user settings.

Folding the key test and the value test into one `select` adds no type
fragility: jq's `and`
short-circuits, so a non-autonomy scalar entry is never indexed, and an
*autonomy* entry whose value
is a scalar errors identically under the old and new filters.

Gates from the worktree root, all green: `check-changelog-parity.sh`
`--check` / `--check-bump
origin/main` / `--check-order`, `check-shell-portability.sh`,
`check-skill-portability.sh`,
`check-silent-skips.sh`, `check-plugin-manifest-presence.sh`,
`check-changed-skills.sh origin/main`
(0 errors; one pre-existing SKILL.md-length warning),
`validate-plugins.sh`, `markdownlint-cli2` on
the changelog, and `shellcheck -x` on all three changed scripts.

Version renumbered to `0.27.6` — `main` published `0.27.4` and then
`0.27.5` while this branch was in
flight.

## Related

No linked issue

The three findings were filed as review threads on merged PRs #1383 and
#1865, not as issues. Those PRs are referenced for provenance only —
this PR closes nothing.

A fourth thread on #1383, `PRRT_kwDOTCGFQM6TzlNw`, needs no change here:
the vacuous-traversal escape
it describes was already closed by `353baf64` (#1851), and a control at
`353baf64^` reproduces it.
Its adjacent defence-in-depth observation — the three preflight `jq`
substitutions in
`lane-launcher.sh` that ignore exit status — is deliberately left for a
separate change. Lines 1-406
of that file are byte-identical to `main` and all seven `$(jq …)`
command substitutions in it are
unchanged; the only edit above the marker-key block is line 407, where
the property list's own count
went from "Two" to "Three".

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

autonomy: lane-stop gate reads its enable flag off channel B, plus two stranded defects from #969

1 participant