Skip to content

fix(bin): defer session-start network checks - #1845

Closed
kunchenguid wants to merge 8 commits into
mainfrom
fm/fm-sessionstart-network-free-r1
Closed

fix(bin): defer session-start network checks#1845
kunchenguid wants to merge 8 commits into
mainfrom
fm/fm-sessionstart-network-free-r1

Conversation

@kunchenguid

Copy link
Copy Markdown
Owner

Intent

Make firstmate's session start have ZERO synchronous external-network dependencies so it becomes near-instant: only local file reads before the digest prints. This is firstmate's shared, tracked material and it is design-heavy, so every safety guarantee must be preserved while the network work moves off the blocking path. Scope was deliberately one coherent ship on bin/fm-session-start.sh plus bin/fm-bootstrap.sh, driven by a completed latency audit (data/fm-sessionstart-latency-audit-s1/report.md).

Part A: dedupe the tasks-axi calls. Session start probed tasks-axi for compatibility twice and invoked it ~10 times per startup with 3 literal duplicates. Fetch the verdict once and reuse it, collapsing the duplicates without losing any information the digest needs.

Part B: move ALL external-network work OFF the synchronous critical path. The digest must print from local state alone with no network call in the blocking path. The network work must still RUN and its safety function must be preserved, but AFTER the digest, in a background/deferred/on-demand stage while the session holds the lock, surfacing anything actionable back to the session (a follow-up wake, or a status the session picks up on its next turn). The deferral had to be designed per-dependency, preserving each sweep's own guarantee: dead-secondmate relaunch (secondmate liveness), clone sync (fleet-sync), handoff retry, and remote endpoint liveness reads.

An explicit honesty gate governed the work: if any single network dependency genuinely could not move off the critical path without losing a real safety guarantee, the instruction was to STOP and report that as the honest answer rather than drop or weaken a guarantee to hit the speed goal or hand-wave a deferral that reopens a race. A needs-decision naming the exact dependency and the exact guarantee lost was an explicitly acceptable deliverable. It turned out not to be needed: all four moved with their guarantees intact, and that conclusion is what the design below defends.

Acceptance criteria: the digest prints with no synchronous external-network call in the blocking path, verified EMPIRICALLY with an unreachable/slow remote (startup still near-instant, prints the local digest, and the network sweeps run AFTER and surface their results with safety guarantees intact); each deferred sweep still performs its safety function, proven to still happen just off the blocking path; tasks-axi is fetched once and reused; behavioral tests through the executable interface (e.g. a stubbed slow/unreachable remote proving near-instant startup plus deferred sweep execution); bin/fm-lint.sh clean.

Constraint accepted at intake: do NOT edit bin/fm-wake-drain.sh or the OPEN DECISIONS fold - a separate in-flight ship owns those files - and let ordinary rebases reconcile.

The change had to follow firstmate-coding-guidelines because it is firstmate shared tracked material and startup/supervision infrastructure at the highest bar: deterministic and idempotent enforcement rather than agent memory, the one-owner rule for contracts, tests that exercise an executable interface instead of asserting implementation source text, shellcheck-clean via bin/fm-lint.sh, one sentence per line in tracked Markdown, plain dash never an em dash, and no agent co-author on the commit. It also required the harness-dependent-check discipline: a fact that comes from a vendor needs a real end-to-end proof, a portable regression, and a dated maintainer-verification record.

Design decisions and tradeoffs made along the way that a reviewer reading only the diff would not know:

  1. Deferral mechanism. A new bin/fm-startup-network.sh owns only the deferral mechanics; bin/fm-bootstrap.sh remains the single owner of every sweep and still runs all of them, through a new FM_BOOTSTRAP_NETWORK phase split (all/skip/only) whose skip and only halves are asserted to be a true PARTITION of the unsplit run. An unrecognized value resolves to 'all' on purpose so a typo can never silently drop a safety sweep. The worker is started right after the lock so it runs concurrently with the whole digest, and the digest harvests it at a new NETWORK CHECKS stage placed after FLEET STATE and before CONTEXT - deliberately not last, because these lines are actionable and the section after it is the curated memory a truncated tail is meant to sacrifice first.

  2. Why the later run is safe, which is the core of the honesty answer. Three properties: the sweeps are idempotent DETECTORS so a lost report loses no finding (the next run re-derives the same dead secondmate, stuck clone, undelivered handoff); the result is durable and ALWAYS surfaces, either inline in the digest or as a 'check: startup-network' wake, decided by a claim handshake under a shared lock so exactly one path reports it and a printed result never also interrupts supervision with a redundant wake; and the worker re-verifies mutation authority before sweeping.

  3. The mutation-authority check compares the fleet-lock VALUE against the pid captured when the worker was launched, rather than requiring that pid to be alive. That was a deliberate choice: the hazard being closed is a second session sweeping concurrently, and taking the lock is exactly what rewrites the value (fm-lock.sh overwrites a dead holder's pid), so an unchanged value proves nobody else owns the sweeps. Requiring liveness would refuse to finish work nobody has claimed, and the sweeps are idempotent, so finishing is strictly better than abandoning.

  4. The worker runs in its OWN process group (monitor mode) with nohup and stdio closed to /dev/null. Each closes a different failure: the closed stdio stops the worker holding the session-open hook's stdout pipe open and stranding session initialization; the separate process group stops the digest's own runtime bound (which terminates its whole process group) from killing the worker AND, worse, orphaning the bootstrap child it had already launched into a separate group, which would leave unbounded network work running with nothing left to bound it.

  5. The whole stage carries one aggregate bound (FM_STARTUP_NETWORK_TIMEOUT, default 120s), replacing the per-call unboundedness that could previously wedge a startup. Hitting it is an actionable NETWORK_CHECKS line, never silence. An abandoned 'running' record (killed worker) reads as needing a rerun rather than staying 'in progress' forever, proven two ways so a reused pid cannot make it lie.

  6. gh auth status was ALSO deferred even though the brief's enumeration did not list it, because it is genuinely a network call in the blocking path and the acceptance criterion says zero. Its guarantee ('do not dispatch until GitHub authentication is good') degrades in a stated, bounded way: the verdict arrives seconds later, the digest names it by name as NOT yet confirmed while pending, and it lands inline or as a wake. A relevant discovery: with the network blocked, gh auth status reports 'the token is invalid' and exits non-zero, so the old synchronous probe was already producing a FALSE NEEDS_GH_AUTH in exactly the case that made it slow. That is now documented in the bootstrap-diagnostics skill.

  7. A read-only (lock-refused) session deliberately runs no network stage at all and says so explicitly in the digest, rather than writing shared state it has no authority for or racing another session's report. The justification is that such a session must not spawn, steer, or merge anyway, so it has no action a GitHub-auth verdict would gate. --reemit runs the read-only probe only, matching its existing rule that it never repeats the sweeps its own startup already ran.

  8. A relaunch performed by the deferred pass is now ALWAYS reported, where previously it was silent unless FM_BOOTSTRAP_VERBOSE_FACTS=1. That is a deliberate contract change and it is load-bearing: the digest that printed the now-superseded endpoint record is already out, so silence would leave a stale record looking authoritative. The report also tells the reader to re-read any record its lines name.

  9. Deliberate scope exclusion: the axi-family version-floor probes (lavish-axi, gh-axi, chrome-devtools-axi, quota-axi) were NOT touched. They are local module-load cost, not network, and the audit already filed them as a separate unit (R1). Measured: they are ~1.2s of the ~1.95s residual on a healthy path, so 'near-instant' for the ordinary case still needs that separate ship. This was reported rather than silently folded in.

  10. A brief item that turned out not to exist: remote endpoint liveness reads in the digest are NOT network. A remote secondmate's metadata records no backend= field, so the per-task read resolves to a local tmux display-message. Verified empirically with an ssh stub: zero SSH calls from the digest. The audit only speculated that such a read 'would' be an SSH round trip. A separate pre-existing bug was found while proving this (that read targets a local pane named remote:, so a healthy remote secondmate would report as 'endpoint: dead'); it was left alone as out of scope and reported for follow-up.

  11. Not implemented, deliberately: the audit's stronger recommendation to gate the ROUTE rather than the startup for secondmate convergence. That is outside these two files.

Empirical verification performed. Against a stub host hanging 25s per SSH connection: startup went from 1m18.2s with 3 blocking SSH attempts inside the digest, to 0.84s with the same SSH attempts continuing at +0s and +25s after the digest finished. Healthy path with the real toolchain: 2.12s to 1.95s, and 0.76s with the axi version probes ablated. The tasks-axi dedupe takes one startup from 10 invocations to 7. The vendor-dependent fact this design needs - that a worker detached by a session-open hook survives the hook returning - was proven end to end against real Claude Code 2.1.222, wired into the opt-in live guard as a third asserted fact for every installed run-tier harness, and recorded with its dated evidence in docs/verification/supervision.md.

Test coverage added: a new tests/fm-startup-network.test.sh pinning the stage's own contract (non-blocking start that does not hold the caller's stdout, the claim handshake in both directions, refusal of mutating sweeps when the lock changed hands, the aggregate bound reported not swallowed, an abandoned run reading as needing a rerun, and single-flight); new session-start cases proving an unreachable host delays a reported check rather than the digest, that the deferred sweeps still land, that a result the digest outran still reaches the agent as a wake, that a read-only session declares its skipped checks, and that the tasks-axi verdict is paid for once; and new bootstrap cases proving the phase split is a partition and that the compatibility handoff travels exactly one process hop without leaking into a spawned agent's environment. Existing secondmate-recovery cases were updated to assert the relaunch through the deferred stage instead of off the digest, which is the real behavioral change.

What Changed

  • Split bootstrap into local and network phases so session-start renders its digest without waiting on GitHub authentication, secondmate recovery and synchronization, handoff retries, or fleet sync.
  • Add a bounded, single-flight network worker that revalidates fleet-lock authority and durably reports results inline or through a follow-up wake.
  • Reuse one tasks-axi compatibility verdict across session start and bootstrap, with executable coverage for slow remotes, deferred reporting, phase partitioning, and lock handoff safety.

Risk Assessment

✅ Low: The deferred startup work is well bounded, prior ownership and delivery races are closed, and no remaining source-verifiable acceptance-criteria violation was found.

Testing

The initial targeted run passed startup-network and bootstrap coverage but exposed a masked wait-status bug and stale reemit fixture assumption; after fixing both and replacing the flaky wall-clock assertion, the focused suites passed, and manual CLI evidence showed the digest return in 2.431 seconds while the 12-second probe remained active, followed by the deferred auth verdict and secondmate relaunch report.

Evidence: CLI transcript: digest returns in 2.431s while 12s probe is still running, then deferred checks and relaunch surface
COMMAND: bin/fm-session-start.sh (fixture with gh auth blocked for 12 seconds)
DIGEST_RETURNED_MS: 2431
BLOCKED_PROBE_STATE_AT_DIGEST_RETURN: still running

DIGEST NETWORK CHECKS SECTION:
NETWORK CHECKS
================================================================================
IN PROGRESS - the deferred network checks have not finished yet.
NOT yet confirmed: GitHub authentication, dead-secondmate relaunch, secondmate convergence, pending handoff delivery, and project clone refresh with its drift reporting.
Started 2s ago, bounded at 120s.
The result is durable in state/.startup-network.report and arrives as a `check: startup-network` wake.
Read it now with /var/folders/0k/bf8mwt2n5qddzk24r20gfk0c0000gn/T//fm-session-start-tests.n669Yj/evidence-slow-network/root/bin/fm-startup-network.sh report; until it lands, treat none of it as confirmed.

================================================================================

DEFERRED REPORT AFTER THE BLOCKED PROBE FINISHED:
completed off the startup path in 14s: GitHub authentication, dead-secondmate relaunch, secondmate convergence, pending handoff delivery, and project clone refresh with its drift reporting.
NEEDS_GH_AUTH
BOOTSTRAP_INFO: secondmate fmtest-sm-n669Yj relaunched after recorded endpoint confidently missing (backend=tmux)
SECONDMATE_SYNC: secondmate fmtest-sm-n669Yj: skipped: not a git repo
These ran AFTER the sections above were composed, so re-read any record a line here names.

DEAD-SECONDMATE RELAUNCH SIDE EFFECT:
new-window -dP -F #{window_id} -t firstmate: -n fm-fmtest-sm-n669Yj -c /private/var/folders/0k/bf8mwt2n5qddzk24r20gfk0c0000gn/T/fm-session-start-tests.n669Yj/evidence-slow-network/secondmate-fmtest-sm-n669Yj
- Outcome: 🔧 1 issue found → auto-fixed ✅ across 2 runs (41m10s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed (4) ✅
  • 🚨 bin/fm-startup-network.sh:167 - start releases the publication lock before reserving a new run. On a subsequent startup, harvest can therefore print the previous done report before the detached worker records running; two simultaneous starts can also both launch workers. Atomically create a generation-specific running reservation under this lock, and bind the claim, harvest, and publication to that generation.
  • 🚨 bin/fm-startup-network.sh:285 - Mutation authority is checked only once before the entire bounded bootstrap. If the originating harness dies afterward, another session can replace .lock while this detached worker continues spawning or syncing secondmates, delivering handoffs, and refreshing clones. Hold a real lease or revalidate the captured lock identity at each shared mutating sweep boundary, and prevent a new owner from reusing the prior owner's running worker.
  • 🚨 bin/fm-startup-network.sh:263 - A direct run --locked 1 without --lock-pid reads the current lock value and compares it back to the same file, so any process can claim mutation authority, including a read-only session while another live harness owns the fleet lock. Verify that the caller's harness identity owns the lock; reserve captured lock values for the internally detached start path.

🔧 Fix: Harden deferred network ownership and generation handoff
2 errors still open:

  • 🚨 bin/fm-bootstrap.sh:176 - The lock check is not atomic with the sweep it authorizes. If the original harness dies immediately after this check, a new harness can replace .lock and start its worker while the stale worker continues the current liveness, convergence, handoff, or clone-refresh sweep. This leaves the prior lock-handoff failure reachable. Coordinate lock takeover with a real sweep lease at the shared lock-acquisition boundary, or revalidate immediately at every mutation.
  • 🚨 tests/fm-sessionstart-hook-live-e2e.test.sh:118 - The live harness fixture links fm-startup-network.sh and only two dependencies, but the script now unconditionally sources fm-session-lock-lib.sh. The opt-in E2E therefore exits before launching its marker worker and cannot verify the required detach behavior. Link the new library into the lab fixture.

🔧 Fix: Lease deferred sweeps across fleet-lock takeover
1 error still open:

  • 🚨 bin/fm-startup-network.sh:293 - The required contract says each result is durable and "ALWAYS surfaces, either inline in the digest or as a check: startup-network wake," but publication still treats a live claimant as completed delivery. If the worker observes the digest alive here and that digest exits or times out before harvest, no wake is queued and nothing prints. Separately, an unclaimed wake points only to the mutable current report, while the next session reserves a new generation before draining that wake, making the prior result inaccessible or overwriting it. Preserve reports and delivery state per generation, and suppress the wake only after that generation is durably acknowledged as harvested. The current live-claim test codifies the unsafe intent-before-delivery assumption and should cover both crash and next-generation sequences instead.

🔧 Fix: Acknowledge harvested reports before suppressing wakes
1 warning still open:

  • ⚠️ tests/fm-startup-network.test.sh:166 - wait now returns when the report is published, but the wake is appended afterward. This assertion can therefore run before the worker queues the wake; the session-start wake test has the same race. Poll for the observable wake as the crash-window test does, or provide a public command that waits for delivery settlement.

🔧 Fix: Stabilize deferred network wake assertions
✅ Re-checked - no issues remain.

🔧 **Test** - 1 issue found → auto-fixed ✅
  • ⚠️ tests/fm-session-start.test.sh:1384 - The full session-start component run intermittently reached 6 seconds and failed the strict < 6 assertion under concurrent machine load. The isolated retry passed, and manual verification returned the digest in 2 seconds while the remote remained blocked for 12 seconds. Stabilize the timing assertion so environmental contention does not produce a false failure while preserving the near-instant requirement.
  • tests/fm-startup-network.test.sh
  • tests/fm-session-start.test.sh - component run stopped at the timing-sensitive slow-remote assertion
  • Isolated test_unreachable_network_never_blocks_the_digest
  • Isolated test_deferred_result_reaches_the_agent_when_the_digest_cannot_print_it, test_read_only_session_declares_skipped_network_checks, and test_tasks_axi_compatibility_is_probed_once
  • tests/fm-bootstrap.test.sh
  • Manual bin/fm-session-start.sh verification using a 12-second gh auth stub, followed by deferred report and relaunch-state inspection
  • git status --short cleanup verification

🔧 Fix: Stabilize deferred network startup timing assertion
✅ Re-checked - no issues remain.

  • bin/fm-test-run.sh tests/fm-startup-network.test.sh tests/fm-bootstrap.test.sh tests/fm-session-start.test.sh (initial run: startup-network and bootstrap passed; session-start exposed the reemit synchronization defect)
  • bin/fm-test-run.sh tests/fm-startup-network.test.sh (passed after adding the wait-timeout regression)
  • bin/fm-test-run.sh tests/fm-session-start.test.sh (passed after fixture and causal timing fixes)
  • capture-session-start-evidence.sh > session-start-unreachable-remote.txt (manual executable-interface verification with a 12-second blocked GitHub-auth probe)
✅ **Document** - passed

✅ No issues found.

🔧 **Lint** - 1 issue found → auto-fixed ✅
  • ⚠️ linter found issues (exit code 1)

🔧 Fix: Restore direct startup sweep lint visibility
✅ Re-checked - no issues remain.

✅ **Push** - passed

✅ No issues found.

The session-start digest runs on a session-open hook that blocks session
initialization, and every external-network call it made was individually
unbounded: `gh auth status`, secondmate liveness, secondmate convergence,
pending remote handoff delivery, and the fleet-sync fetch. One unreachable
remote secondmate could consume the whole FM_SESSION_START_TIMEOUT and
truncate the digest, so a slow network could cost the work queue itself.
Measured against a host hanging 25s per SSH connection, that startup took
1m18s.

The digest is now composed from local reads alone. bin/fm-startup-network.sh
runs the same checks concurrently in a bounded detached worker and the digest
harvests whatever finished, without ever waiting. Same fixture: 0.84s.

Nothing is dropped. fm-bootstrap.sh stays the single owner of every sweep and
still runs all of them, through a new FM_BOOTSTRAP_NETWORK phase split whose
`skip` and `only` halves are a partition of the unsplit run. Deferral is safe
because the sweeps are idempotent detectors, the result is durable and always
surfaces (inline, or as a `check: startup-network` wake), and the worker
re-verifies that the fleet lock still names the session that asked before it
mutates anything. While the worker is still running the digest names exactly
what is unconfirmed rather than implying it passed.

A relaunch performed by the deferred pass is now always reported, because the
digest that printed the superseded endpoint record is already out.

Also collapses the duplicate tasks-axi compatibility probe: the verdict is
computed once and handed to the bootstrap child for one process hop, then
consumed so it never reaches a spawned agent's environment. 10 tasks-axi
invocations per startup become 7.

Verified on Claude Code 2.1.222 that a worker detached by the session-open
hook survives the hook returning, the one vendor behavior this design needs
and no portable test can see.
@kunchenguid
kunchenguid force-pushed the fm/fm-sessionstart-network-free-r1 branch from 4cd7336 to c2e6f07 Compare August 7, 2026 02:09
kunchenguid added a commit that referenced this pull request Aug 7, 2026
* perf(session-start): run every network check off the blocking path

The session-start digest runs on a session-open hook that blocks session
initialization, and every external-network call it made was individually
unbounded: `gh auth status`, secondmate liveness, secondmate convergence,
pending remote handoff delivery, and the fleet-sync fetch. One unreachable
remote secondmate could consume the whole FM_SESSION_START_TIMEOUT and
truncate the digest, so a slow network could cost the work queue itself.
Measured against a host hanging 25s per SSH connection, that startup took
1m18s.

The digest is now composed from local reads alone. bin/fm-startup-network.sh
runs the same checks concurrently in a bounded detached worker and the digest
harvests whatever finished, without ever waiting. Same fixture: 0.84s.

Nothing is dropped. fm-bootstrap.sh stays the single owner of every sweep and
still runs all of them, through a new FM_BOOTSTRAP_NETWORK phase split whose
`skip` and `only` halves are a partition of the unsplit run. Deferral is safe
because the sweeps are idempotent detectors, the result is durable and always
surfaces (inline, or as a `check: startup-network` wake), and the worker
re-verifies that the fleet lock still names the session that asked before it
mutates anything. While the worker is still running the digest names exactly
what is unconfirmed rather than implying it passed.

A relaunch performed by the deferred pass is now always reported, because the
digest that printed the superseded endpoint record is already out.

Also collapses the duplicate tasks-axi compatibility probe: the verdict is
computed once and handed to the bootstrap child for one process hop, then
consumed so it never reaches a spawned agent's environment. 10 tasks-axi
invocations per startup become 7.

Verified on Claude Code 2.1.222 that a worker detached by the session-open
hook survives the hook returning, the one vendor behavior this design needs
and no portable test can see.

Re-landed on current main, superseding PR #1845, which was cut from a
pre-#1842 base. The digest's section numbering in AGENTS.md section 3 now
states the emission order directly - supervision block and its read-once
contract, fleet state, network checks, then context - which keeps #1826's
fleet-state-before-context ordering. The old-bin test shim keeps main's
git-archive baseline from #1851, which already subsumes this branch's reason
for widening that shim.

* docs(verification): re-measure the deferred startup stage on the current base

Re-runs the unreachable-remote latency fixture against default-branch tip
8398d31 rather than the now-historical 345de4e, and records the sweep-result
comparison the deferral's safety argument rests on: the deferred worker's
published report is byte-identical to the three sweep lines the blocking
baseline printed, with the unreachable route preserved in both.

* no-mistakes(review): Fail deferred startup when report publication fails

* no-mistakes(document): Document deferred startup network behavior accurately
@kunchenguid

Copy link
Copy Markdown
Owner Author

Superseded by #1860, which re-landed this network-free change cleanly on current main (merged as 4b6b89d) after this branch's pre-#1842 base conflicted with intervening merges. Content is in main; closing to avoid a duplicate open PR.

@kunchenguid kunchenguid closed this Aug 7, 2026
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