Skip to content

fix: enforce authoritative Claude Vault settings - #21

Closed
rega10 wants to merge 7 commits into
mainfrom
fm/firstmate-fork-reconciliation-option-a
Closed

fix: enforce authoritative Claude Vault settings#21
rega10 wants to merge 7 commits into
mainfrom
fm/firstmate-fork-reconciliation-option-a

Conversation

@rega10

@rega10 rega10 commented Sep 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Intent

Rebuild the captain's fork's main as current upstream/main plus the re-implemented fork patch set (the approved Option A). This branch is built from current upstream/main (86ff1bf) and carries, as fresh hand-written commits (one unit per commit, each naming the fork PR/commit it replaces), everything the fork still needs; it is a strict descendant of upstream/main.

Units re-implemented against current upstream (not mechanical cherry-picks - upstream rewrote several of these files, so each was relocated/reworked by hand):

Deliberate exclusions/decisions the reviewer must not flag as mistakes:

  • Fork PR fix: keep parked tasks off the wedge escalation ladder #16 (keep parked tasks off the wedge escalation ladder) is DROPPED as SUPERSEDED: upstream's watcher independently grew the same declared-pause/captain-held long-cadence handling, so the accepted behavioral intent already holds upstream. Confirmed with the captain.
  • The Bearings DISPLAY integration of feat(bearings): add project lifecycle posture to fleet state and Bearings #18 (bin/fm-bearings-snapshot.sh, bin/fm-bearings-board.sh, the bearings skill/board template, their tests, the CI Bearings-count) is DEFERRED to a follow-up per an explicit captain decision, because upstream independently rewrote the Bearings captain-hold/gate model (deferred_marker replaced by hold_bucket/live_captain_call). Only the posture registry core lands here; docs mark the Bearings consumption as a pending follow-up.
  • The fork's source-byte-asserting tests were intentionally omitted in favor of behavioral tests, and fork changes already present or superseded upstream (e.g. the public-followup date-stability fix) were dropped.

Constraints: only the task branch is pushed; origin/main is never force-updated or touched. Because the two histories diverged (291 upstream vs 19 fork commits), merging this PR via a normal merge is impossible; landing requires the captain's SEPARATE main-replacement sign-off, and the PR description carries the updated inventory, the diff-size summary against upstream/main, and the non-destructive re-tracking command sequence for the running install. Keep the total diff small and reviewable.

What Changed

  • Make Automic Vault launches discard worker-provided --settings arguments so Claude receives one authoritative credential-scrubbing configuration while preserving unrelated worker arguments.
  • Disable Claude feedback drafts in that authoritative configuration and verify its complete authentication-neutralization contract through executable launch tests.
  • Add regression coverage for failed cmux pane listings, retain behavioral Herdr large-schema coverage, remove the source-shape assertion, and update verification documentation.

Risk Assessment

✅ Low: The changed logic is bounded, fail-closed, and the strengthened behavioral regression exercises both supported duplicate-settings forms while validating the complete authoritative settings contract.

Testing

Focused Automic Vault, cmux, and Herdr behavioral tests passed. Executable argv evidence reproduces the base commit's duplicate settings failure and shows the target emitting exactly one authoritative settings object with apiKeyHelper, every claimed credential environment key set to null, and feedbackDrafts set to off; both worker settings spellings are absent, adjacent arguments remain intact, and the worktree is clean.

Evidence: Automic Vault injected Claude argv: base failure and fixed behavior

Source: Automic Vault injected Claude argv: base failure and fixed behavior

Executable under test: bin/fm-claude-automic-vault-launch.sh --injected
Expected: one authoritative --settings; both worker-provided spellings removed.

CASE A INPUT: --settings {"worker":"separated"} --model sonnet launch-brief
<--settings>
<{"apiKeyHelper":null,"feedbackDrafts":"off","env":{"ANTHROPIC_API_KEY":null,"ANTHROPIC_AUTH_TOKEN":null,"ANTHROPIC_BASE_URL":null,"ANTHROPIC_BEDROCK_BASE_URL":null,"ANTHROPIC_VERTEX_BASE_URL":null,"ANTHROPIC_FOUNDRY_BASE_URL":null,"CLAUDE_CODE_USE_BEDROCK":null,"CLAUDE_CODE_USE_VERTEX":null,"CLAUDE_CODE_USE_FOUNDRY":null,"AWS_BEARER_TOKEN_BEDROCK":null}}>
<--permission-mode>
<bypassPermissions>
<--allowedTools>
<Bash>
<--model>
<sonnet>
<launch-brief>

CASE B INPUT: --settings={"worker":"equals"} --model opus launch-brief
<--settings>
<{"apiKeyHelper":null,"feedbackDrafts":"off","env":{"ANTHROPIC_API_KEY":null,"ANTHROPIC_AUTH_TOKEN":null,"ANTHROPIC_BASE_URL":null,"ANTHROPIC_BEDROCK_BASE_URL":null,"ANTHROPIC_VERTEX_BASE_URL":null,"ANTHROPIC_FOUNDRY_BASE_URL":null,"CLAUDE_CODE_USE_BEDROCK":null,"CLAUDE_CODE_USE_VERTEX":null,"CLAUDE_CODE_USE_FOUNDRY":null,"AWS_BEARER_TOKEN_BEDROCK":null}}>
<--permission-mode>
<bypassPermissions>
<--allowedTools>
<Bash>
<--model>
<opus>
<launch-brief>

BASE-COMMIT REPRODUCTION (89e550a): same separated worker input
<--settings>
<{"apiKeyHelper":null,"feedbackDrafts":"off","env":{"ANTHROPIC_API_KEY":null,"ANTHROPIC_AUTH_TOKEN":null,"ANTHROPIC_BASE_URL":null,"ANTHROPIC_BEDROCK_BASE_URL":null,"ANTHROPIC_VERTEX_BASE_URL":null,"ANTHROPIC_FOUNDRY_BASE_URL":null,"CLAUDE_CODE_USE_BEDROCK":null,"CLAUDE_CODE_USE_VERTEX":null,"CLAUDE_CODE_USE_FOUNDRY":null,"AWS_BEARER_TOKEN_BEDROCK":null}}>
<--permission-mode>
<bypassPermissions>
<--allowedTools>
<Bash>
<--settings>
<{"worker":"separated"}>
<--model>
<sonnet>
<launch-brief>
Observed at base: a second <--settings> plus <{"worker":"separated"}> reaches Claude. The target cases above remove it.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

⏭️ **Rebase** - skipped

Push main to origin, or rebase your branch onto origin/main, before gating.

🔧 **Review** - 2 issues found → auto-fixed ✅

  • ℹ️ bin/fm-project-posture.sh:367 - In bin/fm-project-posture.sh the clear subcommand (lines 363-371) goes straight to write_registry &#34;$project&#34; active without running the line_posture multi-lifecycle-token guard that get (line 323) and set (line 344) both enforce. write_registry's find_lifecycle removes only the FIRST lifecycle token, so on a corrupt line like - foo [parked archived] - desc (added 2026-01-01) clear foo succeeds and rewrites to - foo [archived] - desc ..., leaving a residual token and masking the corruption, whereas get foo/set foo active correctly refuse. Only reachable via a manually hand-edited registry (the posture writer never emits multi-token lines), hence low impact. Remedy: run the same line_posture refusal in the clear path (or make write_registry strip every lifecycle token). Corrects existing internal-tool behavior mechanically; does not extend scope.

  • ℹ️ bin/fm-bitwarden-ceremony.sh:504 - In bin/fm-bitwarden-ceremony.sh line 504, stamp=$(today) is an unguarded command-substitution assignment under set -eu. If today() (date -u +%Y-%m-%d) fails, the shell exits immediately with bare code 1 and never reaches the intended diagnostic on line 505 (is_date &#34;$stamp&#34; || die &#39;...&#39;). The two other today() call sites guard it explicitly (cmd_init at line 385 if ! stamp=$(today); then die ..., and parse_record), so cmd_mark is the sole inconsistent site. Impact is limited: it fails safe (no record is written or corrupted); the only defect is a message-less exit-1 instead of the intended refusal diagnostic. Remedy: mirror the guarded if ! stamp=$(today); then die ... form used at the other two sites.

  • ⚠️ tests/fm-claude-automic-vault.test.sh:596 - This regression check is vacuous for the duplicate-settings failure: the exercised worker template supplies no second --settings, so the test still passes if the new stripping loop is removed. It also validates only ANTHROPIC_API_KEY, not the complete credential-key nulling contract claimed in the verification document. Exercise the injected launch with worker --settings &lt;json&gt; and --settings=&lt;json&gt; inputs, assert only the authoritative settings reach fake Claude, and validate every credential key.

🔧 Fix: Strengthen Automic Vault settings filtering regression
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-claude-automic-vault.test.sh — all 6 behavioral suites passed, including test_enabled_disabled_and_non_claude_launches, which exercises the spawn→launch path and asserts the effective injected exec argv carries exactly one --settings nulling the credential keys and setting feedbackDrafts off, with no worker-template --settings surviving
  • Manual end-to-end exercise of the real bin/fm-claude-automic-vault-launch.sh --injected exec-composition path with a fake claude recording its argv: confirmed exactly one authoritative --settings reaches the worker and the template's own --settings (space form and inline --settings= form) plus --dangerously-skip-permissions are stripped

✅ No issues found.

  • Inspected git diff 89e550aa38c9f6fab217eeb1649aac65b9b2bbbe..718b452516e8d8f2d84c0c1ecf055b0a782dfad4 to identify the affected behavioral surfaces.
  • bash tests/fm-claude-automic-vault.test.sh
  • bash tests/fm-backend-cmux.test.sh
  • bash tests/fm-backend-herdr.test.sh
  • Executed bin/fm-claude-automic-vault-launch.sh --injected with separated --settings &lt;json&gt; and inline --settings=&lt;json&gt; worker inputs and captured the fake Claude argv.
  • Executed the base-commit launcher from 89e550a with the same separated settings input to reproduce the duplicate-settings failure.
  • Verified git status --short was empty after removing transient test files.
✅ **Document** - passed

✅ No issues found.

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

✅ No issues found.

Rene Garza Jr. added 7 commits September 5, 2026 11:13
Rene Garza Jr. Rene Garza Jr.
fix(cmux): treat a failed list-panes call as an unconfirmed surface
0ccb7d7 
Re-implements fork commit 5bb9217 against current upstream code.

fm_backend_cmux_surface_exists piped list-panes straight into jq without
checking the CLI succeeded or returned output. With pipefail off, a
list-panes that exited nonzero but still printed a matching pane was read
as a confirmed live surface (a false positive on liveness). Capture the
CLI output first, fail on a nonzero exit or empty output, then feed jq.

Adds a behavioral regression to tests/fm-backend-cmux.test.sh driving the
surface_exists interface with a stub that prints a matching pane and exits
nonzero; it fails against the old pipe and passes with the guard.

Claude-Session: https://claude.ai/code/session_0174SML661zGCCJueePCzpY5
Rene Garza Jr. Rene Garza Jr.
fix(herdr): match the events schema in-process to avoid broken-pipe n…
147ebe2 
…oise

Re-implements the still-needed core of fork PR #6 (7a2b1c4) against current
upstream code. The bundled fm-pr-lib.sh GitLab-MR regex tweak is dropped:
that pattern is already byte-identical in upstream.

fm_backend_herdr_events_capable piped the ~220KB `herdr api schema --json`
payload into `grep -Fq`. grep exits on the first match and closes the pipe
while printf is still writing, so every TTY-attached watcher probe printed
"printf: write error: Broken pipe" twice. The gate still returned capable,
so the noise trained operators to ignore real errors on that stream. Match
the two event needles with in-process `case` substring matching instead: no
pipe, no early-exit consumer, no SIGPIPE.

Adds test_events_capable_* to tests/fm-backend-herdr.test.sh, which drive
the gate against a large synthetic schema (needles mid-body) under a PTY and
fail on any broken-pipe write error. The fork's source-byte-shape lock test
is intentionally omitted per the repo's no-source-assertion test rule; the
PTY cases prove the behavior instead. Documents the rationale in
docs/herdr-backend.md.

Claude-Session: https://claude.ai/code/session_0174SML661zGCCJueePCzpY5
Rene Garza Jr. Rene Garza Jr.
no-mistakes(review): make AV credential-scrubbing --settings the sole…
02543b3 
… injected claude settings
Rene Garza Jr. Rene Garza Jr.
no-mistakes(document): document AV sole-settings credential-scrub saf…
ba262e8 
…ety invariant
Rene Garza Jr. Rene Garza Jr.
no-mistakes(document): Document AV sole-settings verification
8c0925f
Rene Garza Jr. Rene Garza Jr.
no-mistakes(review): Strengthen Automic Vault settings filtering regr…
718b452 
…ession
Rene Garza Jr. Rene Garza Jr.
no-mistakes(document): Consolidate Herdr and Vault verification docum…
b2a87a8 
…entation
@rega10
rega10 force-pushed the fm/firstmate-fork-reconciliation-option-a branch from aa8edae to b2a87a8 Compare September 5, 2026 16:31
@rega10 rega10 changed the title feat: rebuild fleet orchestration on current upstream fix: enforce authoritative Claude Vault settings Sep 5, 2026
@rega10

rega10 commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #22. The no-mistakes pipeline's CI step automatically rebased this branch onto the old origin/main when it detected the (expected) mergeability conflict, which discarded the intended upstream/main base and defeated the Option A reconstruction. The corruption was only ever on this remote branch; the intact upstream-based head was preserved locally, the pipeline's fix commits were cleanly cherry-picked onto it, and it was re-pushed as a fresh branch with a new PR (#22) opened via direct-PR (not through no-mistakes, so the CI step cannot rebase it again). Closing this in favor of #22.

@rega10 rega10 closed this Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@rega10