fix(bin): defer session-start network checks - #1845
Closed
kunchenguid wants to merge 8 commits into
Closed
Conversation
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
force-pushed
the
fm/fm-sessionstart-network-free-r1
branch
from
August 7, 2026 02:09
4cd7336 to
c2e6f07
Compare
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
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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
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
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-startreleases the publication lock before reserving a new run. On a subsequent startup,harvestcan therefore print the previousdonereport before the detached worker recordsrunning; 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.lockwhile 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 directrun --locked 1without--lock-pidreads 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 detachedstartpath.🔧 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.lockand 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 linksfm-startup-network.shand only two dependencies, but the script now unconditionally sourcesfm-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 acheck: startup-networkwake," 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 beforeharvest, 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-waitnow 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< 6assertion 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.shtests/fm-session-start.test.sh- component run stopped at the timing-sensitive slow-remote assertionIsolatedtest_unreachable_network_never_blocks_the_digestIsolatedtest_deferred_result_reaches_the_agent_when_the_digest_cannot_print_it,test_read_only_session_declares_skipped_network_checks, andtest_tasks_axi_compatibility_is_probed_oncetests/fm-bootstrap.test.shManualbin/fm-session-start.shverification using a 12-secondgh authstub, followed by deferred report and relaunch-state inspectiongit status --shortcleanup 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 ✅
🔧 Fix: Restore direct startup sweep lint visibility
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.