fix(bin): sync upstream batch 16 — test-run refusal, captain-hold bounding, Pi provider resolution - #50
Merged
Conversation
… is set (kunchenguid#3891) * fix(bin): refuse the behavior suite in the repository primary checkout A task worker's isolated worktree placement is verified exactly once, when its task starts, and nothing re-checks it afterwards. A worker that later changes directory into the repository's primary checkout runs its Git commands, and this branch-switching suite, against the one checkout every linked worktree resolves against and every landing merges into. A run that dies mid-suite can leave that checkout on a stray branch. bin/fm-test-run.sh now refuses that case. When FM_TASK_ID marks a task worker and the runner resolves to the primary checkout, every executing mode exits non-zero before selecting a suite, with one line naming the primary path and pointing at the assigned task worktree. The predicate is the one bin/fm-spawn.sh already uses for launch placement: the working tree's own git dir is the repository's common git dir, which separates the primary from every linked worktree even when their top levels differ. A run with no FM_TASK_ID set is unchanged, and so are the inspection modes, which execute nothing. When git resolves neither directory - a non-repository fixture, a detached copy - nothing proves this is the primary, so the run proceeds. bin/fm-spawn.sh sets the marker: ship and scout launches export FM_TASK_ID into the pane shell on the same pre-launch channel as GOTMPDIR, and the name joins the sanitized launch environment allowlist so an isolated launch keeps it. * no-mistakes(review): clear inherited task marker in test lib; name resolved ROOT * no-mistakes(document): docs: record FM_TASK_ID marker and runner placement refusal --------- Co-authored-by: Talon Stark <talonstark@gmail.com>
) * fix(bin): bound a stale alarm with the backlog hold, not only the status line A legitimate wait has two records and the stale alarm reads only one. `status_is_paused_or_captain_held` takes a status line, so it sees a wait the worker declared. It cannot see the wait firstmate records when it hands work to the captain: `bin/fm-captain-hold.sh hold` writes that into the backlog and leaves the status log alone, so a delivered task keeps `done: PR ...` as its last line for the whole time the captain is deciding. Both stale branches were blind to it, and each churned a new pane hash back into its own alarm: a `done:` line is captain-relevant and reaches the terminal-stale branch, while a held task whose last line is `working:` reaches `surface_nonterminal_stale` and fails its declared-wait test. Consult that second record where the watcher is about to alarm, through `bin/fm-captain-hold.sh open`, which already owns the predicate's semantics, and bound the alarm on the shared `.paused-resurfaced-<key>` marker and `PAUSE_RESURFACE_SECS` window the declared-wait absorb already uses. The first sight still alarms, the window's end alarms once more, and a held crew that goes genuinely silent still escalates through the wedge timer. Only an established open captain call bounds anything: an unreadable backlog, an absent or incompatible tasks-axi, a row this home does not carry, and every task with no hold keep alarming exactly as before. The backlog hold is deliberately not recorded as a declared pause, because the loop-top reconciliation and `pause_state_class` both read the status line and would clear a flag that line does not support. Extends the fix in kunchenguid#3443, which closed the forms of this loop that the status line itself can express. * fix(bin): identify the captain call a stale alarm is bounded by Three gaps in the bound added by the previous commit, all in how the throttle is scoped and where the backlog is consulted. The scope carried only the status-log signature. A task can be held, answered with `--release`, and re-held as a genuinely different captain call without any status append, so the second call inherited the first one's marker and its first sight was absorbed - the one thing this bound must never do. The task id is not the call: `bin/fm-captain-hold.sh open` gains `--identity`, which reports the call's own lifecycle - its hold-set stamp and the number of recorded answers - on an exit 0 and only then, leaving the silent predicate every existing caller reads unchanged. The throttle scope now carries that identity. The terminal path recorded the throttle before publishing the durable wake. A failed append exits the watcher with nothing queued, and the next sighting then read that fresh marker and absorbed the retry, turning a delayed alarm into a lost one. Recording moves behind the append, as the non-terminal path already had it, and the comment claiming the marker could not outlive its wake is gone because it was false. The backlog was consulted only on a new terminal pane hash. A captain call can open after a hash was absorbed as provably working, changing neither the pane nor the status log, so nothing re-read the backlog and the wedge timer kept firing possible-wedge alarms through a legitimate wait. That timer now consults the call at its own alarm boundary and takes the same bounded cadence - and only at that boundary, so an ordinary repeat poll under the bound stays the local-only read it was. Regression coverage for each, all driving churn through one watcher process rather than relaunching per pane change: relaunch cost dominated the earlier shape, and an absorbing watcher stays in its poll loop across churn in production anyway. An unheld task still alarms on every new hash, and an elapsed wedge timer with no open captain call still escalates as a possible wedge. * fix(review): Compose stale throttles with captain-call lifecycle identity * fix(review): Preserve bounded same-hash captain-call resurfacing * revert(bin): narrow the captain-hold stale bound to its observed defect Lifts the lifecycle-identity and cadence-ownership work back out, leaving the change at the shape that matches the defect actually observed: the stale alarm did not consult the backlog captain hold, on either stale branch. Reviewing the wider version surfaced a series of adjacent gaps in the watcher's alarm state machine - a call opening after the first alarm, marker invalidation at the hold lifecycle boundary, and which deadline a terminal timer represents. They are real, but fixing them turns a small extension into a state-machine change to the alarm path, which is a different review on a subsystem that is being actively reworked. They are named as known limitations rather than carried here, and none of them is load-bearing for what remains: the bound does strictly less than the reverted version, leaves the wedge path escalating on STALE_ESCALATE_SECS exactly as before, and introduces no silence that the existing terminal-alarm path did not already have. Kept from the reverted work is the record-after-append ordering, because that is a defect in the code being shipped rather than an adjacent one: recording the cadence marker before publishing the durable wake let a failed append lose an alarm outright instead of delaying it. History is preserved: the earlier commits stay on the branch and this removal sits on top of them. * fix(review): Document secondmate captain-hold scope boundary * fix(document): Document captain-hold stale alarm scope * fix(bin): bind the stale throttle to the captain call, not the status log The throttle this change introduces was scoped to the task's status-log signature. Answering a call with `--release` and holding the task again creates a genuinely different captain call without necessarily appending to that log, so the second call inherited the first one's marker and its first sight was absorbed. That is the one alarm this bound must never swallow. A delivery announced twice is noise; a decision waiting on the captain that is never surfaced is invisible, because nobody asks for what they do not know to ask for. Measured rather than assumed, on the same fixture - a delivered task held for the captain, released, and re-held with no status append, driven through bin/fm-watch.sh: base c499f84 call-1 first=ALARM call-1 churn=ALARM new call first sight=ALARM before this fix call-1 first=ALARM call-1 churn=absorbed new call first sight=absorbed after call-1 first=ALARM call-1 churn=absorbed new call first sight=ALARM Base never suppresses the new call, so the suppression came from this change and closing it completes the fix rather than widening it. `bin/fm-captain-hold.sh open` gains `--identity`, printing the call's lifecycle - its hold-set stamp and count of recorded answers - on an exit 0 and only then, so the silent predicate bin/fm-teardown.sh reads is untouched. The throttle scope carries that identity beside the status signature. The sibling case was measured too and is NOT included: on the status-declared path, where the last line is `captain-held:`, base already absorbs a re-held call's first sight. That behaviour predates this change and stays documented as a known limitation rather than repaired here. * fix(document): Document captain-call throttle lifecycle scope * fix(ci): isolate the Herdr restart fixtures from a claimed worktree The Herdr behaviour test intermittently reused a local worktree still claimed by an earlier fixture after a restart. The restart scenarios now use an isolated Treehouse project. The full Herdr test passes on Herdr 0.8.2; bash -n and git diff --check pass as well.
…ranch (kunchenguid#3871) * Let the supervision branch resolve extension-registered providers The isolated branch ModelRuntime cannot see providers an extension registered into main's runtime at run time, so a pin on pi-devin-auth's devin/swe-1-7 (or an unpinned branch following a main session on devin) failed with "unavailable to the isolated branch runtime". Capture main's ModelRegistry alongside mainModel and copy each extension-registered provider config into the branch runtime at model-resolution time. The config carries the provider's own streamSimple and oauth wiring by reference, so the custom gRPC transport reaches the branch unchanged instead of being reimplemented. The /supervision-model picker uses the same copy so those models are offered. Update configuration.md and pi-supervision-branch.md, which previously stated extension-registered providers were not offered. * no-mistakes(document): docs: own devin provider carve-out in branch architecture doc * no-mistakes(ci): Fixed the Greptile P1 finding: the /supervision-model picker copied extension-registered providers into the branch ModelRuntime but checked hasConfiguredAuth without refreshing them, so providers with provisional post-registration auth were omitted from the picker while the pin-resolution path (which did refresh) accepted them. Root-cause fix in .pi/extensions/fm-branch-supervision.ts: moved the `refresh({ providers, allowNetwork: false })` call into `copyExtensionProviders` (now async, refreshing every provider it copied) and removed the duplicate per-provider refresh from `resolveBranchModel`. Both the picker and the resolution path now share one copy-and-refresh step, so hasConfiguredAuth is real in both. Regression coverage in tests/fm-pi-branch-extension.test.sh: the stubbed ModelRuntime now mirrors the real runtime by leaving a registered provider's auth pending until `refresh()` runs for it. With that stub, the existing extension-registered-provider case fails against the pre-fix extension (picker offers only anthropic/main-model) and passes with the fix. Verification: tests/fm-pi-branch-extension.test.sh passes (42 ok, no failures); tests/fm-branch-supervision.test.sh passes; tests/fm-pi-primary-types.test.sh skips locally because tsc is not installed (the refresh signature reused is the one the existing code already called). Intent constraints preserved: isolation flags untouched, carve-out still scoped to provider registration, graceful fallthrough when no providers are registered * ci: retrigger flaky Herdr/serial-1 lanes * no-mistakes(document): docs already cover branch extension-provider copy
… bounded captain-hold stale alarms, Pi extension providers Upstream range 5592cb6..0b9f518, three commits, brought in with one merge commit so the waypoint stays in ancestry: 6d396da fix(bin): refuse test runs in the primary checkout when a task marker is set (kunchenguid#3891) d4eb228 fix(bin): bound stale alarms for backlog captain holds (kunchenguid#3842) 0b9f518 feat(pi): resolve extension-registered providers in the supervision branch (kunchenguid#3871) Two files conflicted. 1. bin/fm-watch.sh, one hunk, resolved to UPSTREAM. This is the same problem the fork's PR 18 (5e1e655, "stop false stale wakes on healthy paused crew lanes") and upstream kunchenguid#3842 (d4eb228) both fixed with different approaches: the fork anchored a one-shot on the status-file mtime (paused_gate_needs_surface), upstream bounds the alarm on an open backlog captain hold (task_captain_call_open / stale_wait_declaration / captain_call_declaration / stale_wait_throttled / stale_wait_record / captain_call_stale_bound). The captain's standing conflict rule is to take upstream where both sides fixed the same thing differently, so upstream's block is kept whole and paused_gate_needs_surface is deleted. git auto-merged the fork's call site beside upstream's, leaving a half-spliced hybrid: the fork function was gone from the conflict resolution but still had a caller. Four fork-side remnants of PR 18's approach were therefore resolved to upstream's shape as well, so the main loop is upstream's at every line: - the non-terminal-stale paused) branch: the fork's `if paused_gate_needs_surface ... surface_nonterminal_stale ... else handle_paused_stale` restored to upstream's plain `handle_paused_stale`. - pause_state_class's header comment: the fork's PR-18 expansion (which pointed at the deleted function) restored to upstream's four-line text. - the secondmate stale case: the fork's three-way `paused) / working) clear_pause_tracking / *) rm -f "$ssf" "$ewf"` restored to upstream's `paused) / *) clear_pause_tracking`. - the busy-pane and hash-change pause-bookkeeping clears: the fork's removal of `[ "$n" -ge 2 ] ||` and its added `afk_present ||` both restored. Both existed only to protect PR 18's declaration one-shot from being cleared. After the resolution, bin/fm-watch.sh differs from upstream/main in 9 hunks, each belonging to a fork watcher fix upstream never touched: - sourcing bin/fm-liveness-lib.sh; liveness_defers_wedge; the wedge_timer_check deferral and its comment; the fm_task_script_snapshot_* naming (PR 1, f9a59bb, declared-liveness wedge anchoring). - WATCHER_CLEANUP_LOCK_TICKS and watcher_cleanup's bounded lock wait (PR 7, 2386360, deadlock-safe signal handling). - the Bash 3.2 guarded array expansions in signal_turnend_panes_churned; watcher_close_has_nothing_to_recover and the release-lock-quiet transition (PR 47, 9e69eed, silent watcher deaths and empty recovery wakes). No surviving hunk belongs to PR 18. All 132 non-empty lines upstream added to bin/fm-watch.sh in this range are present in the merged file. grep -c 'the guard is progress, not depth' bin/fm-wake-lib.sh = 1 (PR 47 intact). 2. docs/scripts.md, one hunk, adjacency only. Upstream reworded the fm-test-run.sh row for kunchenguid#3891 while the fork had added a fm-test-env-lib.sh row (a fork-only script from PR #41) directly beneath it. Resolved to upstream's row text plus the fork's additive row. Test fallout. tests/fm-watch-triage.test.sh auto-merged and carried both sides (98 base + 5 fork-only + 4 upstream-new = 107 cases). One fork case was deleted: - test_declared_pause_survives_benign_pane_repaint (from PR 18): its phase C asserts that a one-poll busy blip must not clear a parked lane's pause bookkeeping, which is exactly the behaviour upstream's `[ "$n" -ge 2 ]` clear restores. Phases A and B pass unchanged; only the dropped approach's assertion fails. No other case was touched. The two remaining PR-18-era cases (test_live_declared_pause_gate_surfaces_once_per_declaration, test_live_pause_flag_absorbs_when_authoritative_state_falls_back) pass on upstream's content-keyed throttle and were kept, as were PR 47's two cases. One stale comment naming the deleted function was corrected to describe the throttle that actually governs that fixture. Merged suite: 106 cases, 0 failures. Silent-splice sweep: every non-empty line upstream added since 5592cb6 is present in the merged tree for all 14 other touched files (fm-spawn.sh 17, fm-test-run.sh 40, fm-teardown.sh 3, tests/lib.sh 5, fm-test-run.test.sh 57, fm-kimi-harness.test.sh 2, fm-pi-branch-extension.test.sh 106, fm-backend-herdr-presentation-e2e.test.sh 16, fm-branch-supervision.ts 54, architecture.md 8, configuration.md 5, scripts.md 1, pi-supervision-branch.md 2, verification/trace-context.md 1; 0 missing each). kunchenguid#3891's marker lands whole: FM_TASK_ID is exported to ship and scout panes by bin/fm-spawn.sh and refused against the primary checkout by bin/fm-test-run.sh. bin/fm-captain-hold.sh auto-merged clean and carries upstream's new `open <task> --identity` predicate; its only difference from upstream is the fork's additive precondition-reminder feature. bin/fm-test-run.sh keeps the fork's family registrations: --check-coverage reports total=201 with every tests/*.test.sh in a family. .github/workflows/ is byte-identical to upstream.
…eckout fixture Upstream's kunchenguid#3891 test `test_task_marker_refuses_the_primary_checkout` builds a synthetic repo plus linked worktree and copies `bin/fm-test-run.sh` into each with a bare `cp`. Upstream's runner starts from that alone; the fork's does not. `bin/fm-test-env-lib.sh` is fork-only (PR #41, the owner of the fleet-home overrides a test must never inherit) and `bin/fm-test-run.sh` sources it, refusing with "missing bin/fm-test-env-lib.sh beside this runner; a fixture that copies the runner must copy it too" when it is absent. So the fixture's linked worktree could not start the runner at all, and the case failed on its second phase ("the runner must still run in a linked task worktree") while its first phase, the refusal this upstream commit adds, passed correctly. This file already owns the fix: `install_runner()` was added for exactly this, with the comment "A fixture that copies only the runner leaves it unable to start." The new fixture now uses it instead of the bare `cp`. No runner behaviour changes, and the case exercises upstream's contract as written. Verified: reproduced the failure standalone against the merged tree (the linked worktree exited 2 with the missing-library diagnostic and never wrote its `ran` marker), then re-ran tests/fm-test-run.test.sh, which now passes end to end.
…n-provider test coverage
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
4 participants
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
Captain's standing policy for this fork (2026-09-05, verbatim): "the main goal for firstmate is to become sync with upstream. once we reach that state, we avoid introducing more changes and stay aligned with the upstream. until then you can keep adding work that helps aligning with the upstream easier and reaches the intended state quickly."
Captain's conflict rule (2026-09-05, verbatim): "if there is something upstream and our code both worked on and fixed but approaches differ, take the upstream change, however, if our change is better than upstream, file a PR. for other items, take the upstream changes."
Captain's decision on 2026-09-07 (verbatim): "park whatever diverges from upstream. our goal was to sync with upstream for firstmate. if thats achieved and new updates can easily be pulled from upstream, thats it. no further work on firstmate." Sync batches are the only firstmate work; no fork-side improvements, no new upstream PR candidates unless a measured defect forces one.
The ask this task serves: upstream sync batch 16 - bring the fork's origin/main (now bb429c7: batch 15 landed as #49, waypoint 5592cb6) up to upstream's current tip, waypoint 0b9f518 ("feat(pi): resolve extension-registered providers in the supervision branch (kunchenguid#3871)"). Three upstream commits in range 5592cb6..0b9f518, 17 files, +783/-48:
open <task> --identityread-only predicate), bin/fm-teardown.sh (+3), tests/fm-watch-triage.test.sh (+272), tests/fm-backend-herdr-presentation-e2e.test.sh, docs/architecture.md, docs/configuration.md.What Changed
bin/fm-spawn.shnow exportsFM_TASK_IDto ship/scout panes over the same channel asGOTMPDIR, andbin/fm-test-run.shrefuses to run any executing test mode when that marker is set and the runner resolves to the repository's primary checkout (comparing git-dir against git-common-dir), while inspection-only modes remain unaffected;tests/lib.shgained the runner's library needed by the newtests/fm-test-run.test.shcoverage.bin/fm-captain-hold.sh opengained an--identityflag that prints a call's lifecycle identity (hold-set timestamp + resolution-record count) on an open captain hold, andbin/fm-watch.shuses it to bound repeated stale-wait alarms per distinct identity/status hash within the resurface window, covering both plain stale waits and captain-held waits;bin/fm-teardown.shand doc references were updated to match, andtests/fm-watch-triage.test.sh/tests/fm-backend-herdr-presentation-e2e.test.shwere expanded and isolated accordingly..pi/extensions/fm-branch-supervision.tscaptures main'sModelRegistryalongside its model, and copies extension-registered providers (e.g. a runtime-registered "devin" provider) into the isolated branchModelRuntimewhen a model can't otherwise be resolved, refreshing auth for the copied providers so branch model resolution and availability checks see them; covered by the newtests/fm-pi-branch-extension.test.sh.docs/architecture.md,docs/configuration.md,docs/scripts.md,docs/verification/trace-context.md, anddocs/pi-supervision-branch.mdto describe the new task-marker refusal, captain-hold identity/bounding behavior, and extension provider resolution.Risk Assessment
✅ Low: Clean upstream sync; the one real merge conflict in bin/fm-watch.sh was independently traced and does not reintroduce the original bug it appears to remove protection for.
Testing
Two of the three targeted suites for batch 16 (primary-checkout refusal and Pi extension provider resolution) ran to completion with all cases passing, directly demonstrating the fixed/added behavior end-to-end via real fixture repos and worktrees; the third suite (stale-alarm bounding for captain holds) was still executing — with all observed assertions passing — when this report was required, so its final pass/fail status is not yet confirmed.
Evidence: fm-test-run.test.sh full transcript (primary-checkout refusal, 34/34 passed)
Source: fm-test-run.test.sh full transcript (primary-checkout refusal, 34/34 passed)
Evidence: fm-pi-branch-extension.test.sh full transcript (extension provider resolution, 42/42 passed)
Source: fm-pi-branch-extension.test.sh full transcript (extension provider resolution, 42/42 passed)
Evidence: fm-watch-triage.test.sh partial live transcript (stale-alarm bounding, in progress at report time)
Source: fm-watch-triage.test.sh partial live transcript (stale-alarm bounding, in progress at report time)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
Step was skipped.
✅ **Review** - passed
✅ No issues found.
tests/fm-watch-triage.test.sh- The targeted test suite for the batch's stale-alarm bounding change (d4eb228, exercised by tests/fm-watch-triage.test.sh, +272 lines) did not finish running before this report had to be produced. It builds real fixture repos and appears to use wall-clock timers (multiple minutes elapsed with minimal CPU), similar to the other two suites which each took ~5 minutes. Live partial output (captured to evidence) showed dozens of 'ok' assertions covering stale-classification, wedge-escalation, and captain-hold absorb logic with zero failures observed up to that point, but the suite had not reached its FM_TEST_END/exit status. The other two targeted suites (tests/fm-test-run.test.sh for 6d396da + the d6190d3 fixture fix, and tests/fm-pi-branch-extension.test.sh for 0b9f518) both completed with full passes. Recommend re-running./bin/fm-test-run.sh tests/fm-watch-triage.test.shto completion and confirming its final exit=0 before treating batch 16 as fully validated../bin/fm-test-run.sh tests/fm-test-run.test.sh --json /tmp/fm-test-run-result.json— 34/34 cases passed (exit=0), including 'a task marker refuses execution in the primary checkout and leaves worktrees and inspection alone' (the case d6190d3 fixed by using install_runner in the linked-worktree fixture)./bin/fm-test-run.sh tests/fm-pi-branch-extension.test.sh --json /tmp/fm-pi-branch-result.json— 42/42 cases passed (exit=0), including 'an extension-registered provider resolves in the isolated branch runtime' (the 0b9f5186 feature)./bin/fm-test-run.sh tests/fm-watch-triage.test.sh --json /tmp/fm-watch-triage-result.json— started but did not complete before this report; live partial transcript showed dozens of passing stale-alarm/wedge-escalation/captain-hold assertions with zero observed failuresgit status --short— confirmed the worktree is clean, no transient test artifacts left behind✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.