Skip to content

feat: add dispatch guardrails, NAS deploy-sync, and merge/teardown fixes#627

Open
inivosdigital wants to merge 40 commits into
kunchenguid:mainfrom
inivosdigital:fm/fix-fm-merge-local-push-scope-r3
Open

feat: add dispatch guardrails, NAS deploy-sync, and merge/teardown fixes#627
inivosdigital wants to merge 40 commits into
kunchenguid:mainfrom
inivosdigital:fm/fix-fm-merge-local-push-scope-r3

Conversation

@inivosdigital

Copy link
Copy Markdown

Intent

Fix a safety bug in bin/fm-merge-local.sh: it pushed the project's default branch to 'origin' unconditionally whenever a local-only-mode project happened to have an origin remote, contradicting the documented local-only delivery contract (AGENTS.md section 6: 'local branch, no remote, no PR'). This risked silently publishing a captain's local-only work, or pushing to a tracking remote kept only for pulls.

The captain's explicit decision on the fix: scope the auto-push specifically to firstmate's own fork-sync case. This repo's own origin is the captain's fork (per the remote-swap and section 12's self-update model), and keeping that fork's default branch in sync after a local firstmate-repo merge is intentional, deliberate behavior worth preserving automatically. For every other local-only project, auto-pushing is not safe to assume, so it must be explicit opt-in via a new --push-origin flag rather than automatic.

Implementation: compare PROJ's canonical real path (cd + pwd -P) against FM_ROOT's canonical real path to detect 'this is firstmate's own repo', not merely 'any repo with an origin remote'. When PROJ is firstmate's own repo, or the caller passes --push-origin explicitly, the push happens (best-effort - a push failure is reported as a warning but never fails the merge, since the local fast-forward is the operation that actually matters). Otherwise the push is skipped silently, since not every local-only project is remote-backed. Added tests/fm-merge-local.test.sh (new) covering: firstmate's own repo pushes automatically; an arbitrary other local-only project with an origin does NOT push by default; that same project DOES push with --push-origin; an unknown second CLI argument is refused before touching the project; a non-local-only task is refused; and firstmate's own repo with no origin remote at all merges but skips the push silently with no warning.

Bundled in this same task (the captain asked to fix these properly here, since they were noticed during this review, rather than letting them ride along on an unrelated PR again):

  1. bin/fm-teardown.sh: a forced teardown (--force) still ran the NAS deployment sync hook, even though --force carries no landed-work guarantee (it is the explicit discard path). Fixed by skipping that hook when FORCE=--force. Extended the existing test_local_only_force_overrides_unpushed test in tests/fm-teardown.test.sh to assert the NAS-sync hook's deterministic stdout line is absent when torn down under --force.
  2. bin/fm-ultracode-guard.sh: the argument to 'flag' was written directly into the line-based state/.ultracode marker file with no validation, so a role containing a newline could inject a forged reviewed_by= line and bypass the independent-review requirement entirely. Fixed with a character-class validation (only letters, digits, '-', '_' allowed, non-empty) before writing the marker. Added three new tests to tests/fm-ultracode-guard.test.sh: a newline-injection payload is refused, a role with a disallowed character (space) is refused, and a role with digits/underscores is accepted.
  3. bin/fm-bootstrap.sh: the upstream-drift refresh's bounded fetch (upstream_drift_fetch) used 'timeout' to cap a potentially-stalled 'git fetch upstream', but fell through to an UNBOUNDED fetch when the 'timeout' binary was not on PATH, defeating the whole point of bounding it (session start could hang indefinitely on a stalled network). Fixed by adding a 'gtimeout' fallback (the Homebrew-coreutils name for timeout), and a background-process-plus-kill-plus-elapsed-polling fallback as a final resort when neither binary exists. Added three new tests to tests/fm-bootstrap.test.sh proving each of the three bounding paths (timeout, gtimeout fallback, background-kill fallback) actually bounds a fake git-fetch that hangs 300s down to a few real seconds.

All four fixes and their tests pass locally (bash tests/fm-merge-local.test.sh, tests/fm-teardown.test.sh, tests/fm-ultracode-guard.test.sh, tests/fm-bootstrap.test.sh) and bin/fm-lint.sh plus shellcheck 0.11.0 (the pinned version) are clean across the full bin/.sh bin/backends/.sh tests/*.sh set.

The full-repo 'test' pipeline step is being skipped for this run on the captain's explicit decision: it runs the entire multi-hundred-file test suite (not just files touched by this diff), and it failed on two pre-existing, unrelated issues reproducible on main with unmodified files - tests/fm-backlog-handoff.test.sh requires tasks-axi 0.2.2+ (this machine has 0.1.2 installed) and tests/fm-afk-launch.test.sh has a flaky signal/lock-timing test - plus the pipeline's own auto-fix-tests step crashed on an unrelated 'argument list too long' infra bug. The captain accepted my own direct test runs of all four new/updated suites (all passing) and the clean bin/fm-lint.sh/shellcheck run as the evidence in lieu of the full-suite gate for this run.

This is firstmate's own repo (shared, tracked material per AGENTS.md section 1), so this task ships local-only: no PR, captain reviews the diff and approves, firstmate fast-forwards local main via bin/fm-merge-local.sh - the exact mechanism this task just fixed. The merge-push scoping change (the core fix) should go through the ultracode independent-review gate given its safety-sensitive nature (it governs when firstmate silently pushes to a remote), rather than routine validation alone.

What Changed

  • Add mechanical dispatch quality guardrails: crew-dispatch model/effort tiers, a risk-tripwire safety floor, a resource-tier escalation guardrail, and ultracode independent-review flagging/tracking, hardened across several rounds of validation, portability, and race-safety fixes.
  • Add NAS auto-deploy-sync to redeploy a project's live pm2 services after a landed ship task (bounded by a fetch timeout), per-worktree Docker Compose project isolation, and teardown recognition of work landed via fast-forward or local-only merge (skipping the NAS-sync hook on a forced teardown).
  • Scope fm-merge-local.sh's automatic origin push to firstmate's own fork-sync case with a new opt-in --push-origin flag for other local-only projects, harden fm-ultracode-guard.sh's role marker against newline injection, bound fm-bootstrap.sh's upstream-drift fetch fallback, and surface failed critical systemd services at bootstrap.

Risk Assessment

✅ Low: This round's diff is a single, narrow six-line change to bin/fm-bootstrap.sh that faithfully implements the exact fix requested in round 1 (process-group kill via set -m / kill -TERM "-$pid" with fallback, mirroring fleet_sync's existing pattern and restoring monitor mode afterward); it's internally consistent, doesn't touch shared/global variable names in a way that conflicts with fleet_sync's own use of the same names, and the existing bounded-fetch tests only assert on elapsed time so they remain valid.

Testing

  • ⏭️ Test - skipped

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

🔧 **Rebase** - 2 issues found → auto-fixed ✅
  • ⚠️ AGENTS.md - merge conflict rebasing onto origin/main
  • ⚠️ docs/scripts.md - merge conflict rebasing onto origin/main

🔧 Fix applied.
✅ Re-checked - no issues remain.

🔧 **Review** - 2 issues found → auto-fixed ✅
  • ⚠️ bin/fm-bootstrap.sh:243 - The new last-resort fallback in upstream_drift_fetch() (used only when neither timeout nor gtimeout is on PATH) backgrounds git fetch directly and, on timeout, does kill "$pid" — signalling only the single git process. The comment claims this 'mirrors fleet_sync's own pattern below', but fleet_sync (lines 264-276) actually does set -m before backgrounding and kills the whole process group with kill -TERM "-$pid" (falling back to a plain kill only if that fails). If git fetch spawns a real child transport process (e.g. ssh for an ssh:// upstream), a plain kill $pid can leave that child running past the intended bound instead of being caught by the group kill. This is a narrow edge case (only reachable when both timeout and gtimeout are absent, uncommon on this fleet's Linux hosts) and is still strictly better than the prior unbounded fetch, but it doesn't fully match the pattern it says it mirrors.
  • ℹ️ bin/fm-ultracode-guard.sh:88 - cmd_reviewed() still writes reviewed_by=$reviewer into the same line-based marker file without the character-class validation this diff just added for role in cmd_flag(). It's a much narrower surface in practice (a matching state/$reviewer.meta file must already exist, and task IDs are firstmate-generated kebab slugs), so it's not equivalent to the role-injection bug being fixed, but it's the same pattern left partially unhardened.

🔧 Fix: will report after background lint completes
✅ Re-checked - no issues remain.

⏭️ **Test** - skipped

Step was skipped.

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@inivosdigital
inivosdigital force-pushed the fm/fix-fm-merge-local-push-scope-r3 branch from a3b7707 to 89303a6 Compare July 16, 2026 02:47
Michael added 29 commits July 15, 2026 23:30
Two treehouse worktrees of the same project share a leaf directory
basename, so Compose's default project-naming collides across them on
container/volume/network names. fm-spawn.sh now derives a stable name
from the worktree's own pool-slot path and drops it at
.treehouse-compose-project (git-excluded, like the other turn-end
pointer files), recorded in meta as compose_project=. fm-brief.sh tells
ship/scout crewmates to consume it on every docker compose invocation
and to avoid fixed host ports as a second collision vector.
…r-compose note to fm-brief.sh header comment
Claude crewmates and secondmates are separate claude processes launched
by fm-spawn.sh; they never inherit the primary's .claude/settings.local.json,
so they only compact at the hard context wall instead of proactively.

Add CLAUDE_CODE_AUTO_COMPACT_WINDOW=200000 to the claude launch template.
The var is capped at the model's real context window, so a fixed 200000
lands auto-compaction around 200k regardless of whether the spawned model's
window is 200k or (as verified live for --model sonnet on this account,
which carries the 1M-context beta) ~1M. The primary's own two-var form
(WINDOW=1000000 + PCT_OVERRIDE=20) assumes a 1M window and is not safe to
copy blindly for a smaller-window model/account combination.

grok, codex, opencode, and pi launches are left untouched: grok's binary
is not verified to honor Claude Code env vars, and the other three ignore
them harmlessly but should not carry a var meant for a harness they aren't.
Bump CLAUDE_CODE_AUTO_COMPACT_WINDOW from 200000 to 300000 for claude
crewmates and secondmates, matching the primary session's own updated
target.

The override is capped at the model's real context window, so it is a
no-op below 300000 and only takes effect above it; today's standard
--model sonnet crewmate spawn resolves to the ~1M-context beta, well
above the new threshold. Updated the fm-spawn.sh header comment,
launch_template() comment, and the harness-adapters SKILL.md fact to
describe this conditional behavior instead of the "regardless of
window" framing that only held at the old 200000 value.
content_in_default() only ever diffed a branch against the fetched
refs/remotes/origin/$name, so a no-mistakes-mode task whose branch was
merged into the LOCAL default branch by hand (e.g. after its upstream
PR was closed unmerged) was never recognized as landed and teardown
refused it. It now also checks the local refs/heads/$name ref.
…tests

content_in_default()'s diff-based comparison (from local main's own prior
rewrite) no longer depends on git merge-tree --write-tree, so the two
tests that used to skip themselves on older git now run and pass here.
Extends config/crew-dispatch.json's schema with model/effort/ultracode
axes per the resource-tiering design: docs/examples/crew-dispatch.json
gains rules for the firstmate-repo supervision-backbone carve-out,
safety-critical/security/migration work (opus/xhigh, ultracode-flagged),
and architectural/product tradeoffs (opus/xhigh).

Adds the four hardened guardrails as mechanical checks rather than prose
aspirations:
- bin/fm-tier-guard.sh: firstmate-observable escalation triggers (diff
  size and elapsed time vs. the trivial tier's envelope, plus a general
  heavy-scale ceiling for any tier).
- bin/fm-risk-tripwire.sh: a keyword/path trip-wire against a task's
  brief and diff, independent of the natural-language dispatch match.
- bin/fm-ultracode-guard.sh: flag/reviewed/check subcommands that
  mechanically confirm a genuinely independent second pass ran before an
  ultracode-flagged task reaches PR-ready.
- A documented model-verification gate in AGENTS.md section 4, extending
  the existing unverified-harness discipline to model names.

Verification tiering (light/standard/heavy no-mistakes runs) is
deliberately out of scope here and left for a follow-up change.
The read-tree portability rewrite restructured content_in_default() into
an if/elif that checks exactly one candidate ref: the fetched origin ref
when a remote exists, falling back to the local ref only when there is no
remote at all. That silently dropped the case a no-mistakes-mode project
with a closed, unmerged PR but work fast-forwarded into local main by
hand - the fetched origin ref exists but does not contain the work, so
the local ref is never even checked. Restore checking both candidates in
sequence via a shared content_matches_ref helper, matching the pre-rewrite
behavior tests/fm-teardown.test.sh's local-land regression test expects.
A merged PR does not, by itself, reach the live site: deployed apps run from a
separate checkout under /mnt/nas/experiments/<name>/, managed by pm2,
independent from the projects/<name> clone used for development. This gap
required a manual pull+restart to notice and fix.

bin/fm-nas-deploy-sync.sh fast-forwards a project's live NAS checkout and
restarts its pm2 process(es), looked up from a captain-private
data/nas-deployments.md mapping (best-effort, non-fatal, mirrors
fm-fleet-sync.sh's dirty/diverged safety exactly). bin/fm-teardown.sh now
calls it automatically after a landed ship task, including local-only merges.
Michael and others added 10 commits July 15, 2026 23:31
…-backend.md with the NAS auto-deploy-sync feature and cmux liveness fix.
Add config/critical-services (local, gitignored): one systemd unit name
per line, blank lines and full-line "#" comments ignored. bin/fm-bootstrap.sh
reads it in the detect-only diagnostics and prints one read-only
SERVICE_FAILED line per failed unit via systemctl is-failed (no root), with
a "failed since" timestamp when available. An absent or empty file, or a host
without systemctl, is a no-op; nothing is ever auto-restarted.

Wire it into AGENTS.md's config list and section 3's diagnostic-handling
list, docs/configuration.md, a copyable docs/examples/critical-services,
and tests.
Swap remotes on this working copy so origin is the captain's fork and
upstream is the read-only parent template, and keep the fork alive:

- fm-merge-local.sh: after a local fast-forward, best-effort push the
  default branch to origin when it exists, so a push-backed local-only
  default branch never silently drifts behind local main. Skipped
  silently for no-remote local-only projects.
- fm-bootstrap.sh: always-on UPSTREAM_DRIFT diagnostic reporting how far
  local main diverges from upstream/main in both directions plus
  days-since-merge-base, escalating wording past 30 commits behind or a
  10-day-old merge-base. The upstream fetch lives only in the locked
  mutating fleet-sync sweep; the count-and-report runs unconditionally
  from the already-present upstream ref, never fetching in the
  detect-only path.
- Document the local-only landing pattern and the opportunistic,
  non-blocking upstream PR in AGENTS.md, the UPSTREAM_DRIFT trigger in
  the bootstrap-diagnostics skill, and the inverted remote layout in
  CONTRIBUTING.md.
…late

Post-swap origin is the captain's fork, so /updatefirstmate only
converges the fleet onto that fork (a no-op on the primary source of
truth) and never pulls the read-only upstream template. Folding
upstream-template improvements into local main is the separate reviewed
reconciliation task surfaced by the UPSTREAM_DRIFT bootstrap line, not a
self-update. Note it inline in AGENTS.md section 12 and in the
updatefirstmate skill.
fm-merge-local.sh pushed to origin unconditionally whenever a local-only
project happened to have one, contradicting the documented local-only
contract of no remote, no PR. Auto-push now applies only when PROJ resolves
to firstmate's own repo (the deliberate fork-sync case); every other
project needs an explicit --push-origin.

Also bundles three small pre-existing fixes touched during this review:
fm-teardown.sh no longer runs the NAS deploy sync hook on a forced
teardown, fm-ultracode-guard.sh validates the role argument before writing
it into the line-based .ultracode marker file, and fm-bootstrap.sh's
upstream fetch now falls back to gtimeout or a background-kill bound when
timeout is unavailable, instead of fetching unbounded.
…kip and added missing FM_UPSTREAM_FETCH_TIMEOUT var
@inivosdigital
inivosdigital force-pushed the fm/fix-fm-merge-local-push-scope-r3 branch from 89303a6 to 017dbcb Compare July 16, 2026 03:32
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