feat(bin): sync upstream to #2850 with GitLab merge support - #40
Merged
Merged
Conversation
…guid#2758) * fix(lint): name the installer when ShellCheck or actionlint is missing A missing actionlint exited 127 like a bare command-not-found. Fail with exit 1 and point at the pinned installer, matching the missing-ShellCheck path, without weakening the version pin. * test: isolate kimi and muse detection from inherited Cursor markers Harness detection checks CURSOR_AGENT before ancestry, so these markerless-adapter cases failed when the suite itself ran under Cursor. Clear the verified markers the same way the secondmate harness tests already do. * no-mistakes(document): Document Muse Cursor marker cleanup
…lled but inert (kunchenguid#2684) * feat(checks): report tool updates that are available or installed but inert Firstmate had no way to notice that tooling this home depends on needs an update, and no way at all to notice the worse case: an update that installed correctly and then did nothing. That second case is why this exists. A tool that self-installs into ~/.local/bin while a version manager keeps its own older copy earlier on PATH looks completely up to date to anything that asks only "is a newer version published". On 2026-08-20 a Herdr update landed at 0.8.2 while an older 0.8.0 copy stayed earlier on PATH, so every Herdr command failed on a protocol mismatch and firstmate could not read its own fleet. bin/fm-tool-update-check.sh reports the two conditions separately: <tool> update available a newer version exists at the update source. <tool> update not in effect a newer copy is installed on this host, but PATH still resolves an older one. PATH skew is measured, never inferred. Every executable copy of a watched command on PATH is asked for its own version and those answers are compared, so one lookup cannot hide the skew, and a directory name is never read as a version because a version manager's "latest" directory can hold an older build. A copy that will not report a version is a check failure, not a pass. The watched tools live in local, gitignored config/watched-tools.json, so adding a tool is a config edit rather than a code change, and the file is never propagated to another home. Update sources cover both shapes: a local clone's commit distance from its remote branch, and a command's own version and update announcement, including a tool like no-mistakes that prints its version on one command and announces a new release on another. The check prints one line when something needs attention and prints nothing otherwise, so it rides the existing watcher state-check contract with its trust binding instead of introducing a schedule of its own, and state/.tool-updates keeps the same pending update from being reported on every poll. The check only reports. It never installs, updates, reorders PATH, touches a version manager, or fetches into a watched repository; every git probe is read-only. Tests cover the skew case as a regression, and it was verified by mutation: removing the skew report, or stopping after the first PATH hit as a single lookup would, each make that test fail. * no-mistakes(review): fix tool update check probe reporting, budget, and shim write * no-mistakes(review): keep sweeps alive on broken patterns and oversized budgets * no-mistakes(review): roll back failed arm, widen budget clamp, bound repo probe * no-mistakes(review): guard git probes at the budget, record uncut findings * no-mistakes(document): fix stale watched-tool report-record wording in docs and header * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes The behavior shard's watch-triage suite failed on the new worktree-write wedge tests. Those five tests are the only ones in the file that do not use its standard waits. They give a fixed 3 second liveness budget to the one poll that now spawns the bounded worktree walk, and 4 seconds to an escalating watcher where every other test in the file gives 10. On a loaded runner that poll outlives the fixed budget, so the round is reaped before the deferral it asserts on is recorded, and the test reports a lost deferral instead of the deferral under test. Wait for a completed poll cycle through the file's own wait_poll_cycle, which is what its header documents this hazard for, and use the file's standard 100 tick exit budget. Verified against a load that reproduces the failure: 11 of 12 runs failed before, 8 of 8 pass after. Verified by mutation too, so the waits still prove the behavior: removing the write deferral, and keeping a finished deferral chain across an idle-timer repair, each still fail their test.
* fix: treat yolo as merge authority only, not ask-user finding authority Yolo on/off was documented as also deciding no-mistakes ask-user findings, which hid firstmate's duty to judge unambiguous-toward-design findings itself. Keep every safety boundary; this is a contract clarification, not a relaxation. * no-mistakes(document): Clarify yolo documentation ownership and merge posture
… work over (kunchenguid#2767) * feat(voice): spoken round trip on Nova Sonic 2 with a measured relay cost Step one of the spoken interface: the laptop captures and plays audio, this desktop holds the model session, and no AWS credential leaves the desktop. Measured, amazon.nova-2-sonic-v1:0 in eu-north-1, end of speech to first byte of reply audio, 6 runs each, all answered, on a question that forces a records read: relay path 1.229 1.379 1.428 1.447 1.481 1.516 median 1.438 direct 1.147 1.179 1.203 1.237 1.244 1.317 median 1.220 The relay costs about 0.22s of the median. The direct figure reproduces the earlier survey, which is what makes it a usable control. Excluded: the captain's own ssh round trip, microphone capture, and speaker output. This desktop has no microphone and no speaker, so every run used audio files. Three pieces: bin/fm-voice-relay.py holds the conversation on this host bin/fm_voice_records.py what a spoken answer may read, and the handover bin/fm-voice-client.py the laptop end; audio devices UNVERIFIED bin/fm_voice_frame.py the wire format both machines share Real work is handed to the existing bin/fm-inbox.sh rather than a second queueing surface, and the agent says it is handing over rather than answering as firstmate. Read scope: Done history and free-form note bodies are never assembled at any scope, so the wide default cannot reach the places commercial detail accumulates. config/voice-read-scope narrows it to counts only, and config/voice-read-deny excludes a named item in one line. The boundary is an executable test that widening the reader fails. Push to talk is the default because it is cheaper and the choice is still open; --listen open-mic is the single flip. Two traps worth knowing: a clip with no trailing silence is never answered, and the end of a reply is contentEnd with stopReason END_TURN, not completionEnd. A second user turn in one session is treated as barge-in unconditionally, and an interrupted turn that calls a tool is lost, so the session reconnects per turn and gives up conversational memory. That is the concrete thing step three has to solve. * no-mistakes(review): fix voice relay credential reuse, frame validation and record parsing * no-mistakes(review): test uplink header guard, bound unknown expiry, align state dir * no-mistakes(review): decide deny per item, guard turn failures, bound ambient credentials * no-mistakes(review): read account config from home, harden deny and turn failures * no-mistakes(review): close status verb set, fix inbox help, pair data override * no-mistakes(review): keep profile-free relay alive, unblock loop, fix dead assertion * no-mistakes(review): hide finished pull requests, refuse open mic, keep suite offline * no-mistakes(review): survive reader failures, release devices, fix claims A failure while handling a model event, or while sending a tool result, left the reader task dead with ended and turn_done clear, and close() re-raised the stored failure on every await. One dropped stream became a relay that could never build another session. The reader now reports the session over in a finally whatever killed it, and close() absorbs the task the same way it already absorbed its sends. The laptop client releases what it already started when a later startup step refuses, SystemExit from the handshake wait included, and names a device refusal instead of leaking a raw PortAudio error. Whether it releases correctly against a real device is still unverified here. The records docstring claimed every reading was filtered to open ids. Only the pull request count and list are; the worker count and the state histogram cover every live runtime record, finished ids included, because a meta file still on disk still needs tearing down. The finished-work deny half of the suite asserted things that held with the deny list absent. It is replaced by a deny on an open title, which removes the row and says so while the count stays honest. * no-mistakes(review): name reader failures, split file and device refusals A failure inside the model reader released the waiting turn and told nobody. The session was not marked spent, no notice reached the client, and the client waits for a reply end or a notice, so the captain got their whole timeout of silence and then a record saying the turn went unanswered with nothing about why. Both ends of the relay now name a failed turn through one function, once per turn, and --self-test carries the cause in relay_error the way the client's own record does. Two things that are not failures stay that way. A stream that simply ends is the end of a session, which serve still reads on its own terms. A stream that goes away because close() asked it to is an ordinary renew, and announcing it would have put a failure notice in front of the captain on every turn. On the laptop end, the refusal that became a device error covered the file-backed playback and capture too, so a mistyped --in-file was reported as an audio device failure and the advice named the flag that had just failed. The file ends now report the path and the flag that chose it and stay an OSError; the device ends keep the device advice and name the flag for that end. The device paths remain unrun here, so only the file halves are covered by a test. * no-mistakes(test): survive model session end, order client turn frames * no-mistakes(document): sync voice relay docs with reviewed relay behavior * no-mistakes(document): re-measure relay latency and correct its cause * no-mistakes(document): correct measurement date and name the unmeasured SSH hop * no-mistakes(document): describe the unpublished control measurement, fix list formatting * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes
…unchenguid#2763) * fix: keep Relay public loops open until retire Delivering a promised-final reply was deleting the only record that tied a public thread to later work, so a follow-on ship silently owed no closing reply. Retain the registration after delivery, rechain follow-on work onto the same thread, and make retire --reason the only close. * no-mistakes(review): Propagate public follow-up registration removal failures * no-mistakes(review): Persist retire receipts and align parent resolution * no-mistakes(review): Make rechain resumable after partial obligation creation * no-mistakes(review): Repair follow-up state, briefs, and expiry escalation * no-mistakes(review): Serialize follow-up delivery stamps with retirement * no-mistakes(review): Serialize rechain claims and protect registration terminal states * no-mistakes(review): Avoid reporting retired delivery loops as open * no-mistakes(document): Refresh public-loop documentation and verification evidence * no-mistakes: apply CI fixes * no-mistakes(review): Preserve delivered follow-up bindings during registration replay * no-mistakes(review): Harden public follow-up retirement and rechain races * no-mistakes(review): Fail closed on unresolved secondmate retirement * no-mistakes(review): Bind secondmate cleanup to its recorded canonical home * no-mistakes(review): Fix rechain command output and expiry validation * no-mistakes(review): Validate brief keys and warn on remote promotion * no-mistakes(document): Document retained public follow-up loops * no-mistakes(lint): Remove unused bounded-wait loop variable
…ath (kunchenguid#2779) * feat(bin): merge GitLab merge requests through the guarded PR merge path bin/fm-pr-lib.sh already parses a GitLab merge request URL for the watcher, but bin/fm-pr-merge.sh refused every non-github provider, so a merge request had to be merged by hand and got none of the recording, guards, or audit trail a pull request gets. The merge path now dispatches on the parsed provider. A GitHub URL keeps its exact previous behavior. A GitLab URL is addressed through glab by the project URL rebuilt from the parsed host and path, so a merge request on any instance resolves and no host is hardcoded, and no merge-method flag is added because the project's own merge method is what should apply. A GitLab merge happens only after one live read of the merge request confirms it is open, detailed_merge_status is mergeable, has_conflicts is false, blocking_discussions_resolved is true, and the head pipeline succeeded at the exact current head. Every failing condition is reported, not just the first. The verified head is bound to the merge with glab's --sha, so a push landing between the read and the merge fails the merge instead of landing commits nothing verified. Recorded metadata is never the authority for any of this: a rebase moves the head and leaves a recorded value stale, so a recorded head that disagrees with the live one is reported rather than trusted, and the recorded value is read before the recording step because that step drops a GitLab head it cannot resolve. * no-mistakes(review): reject bundled -R clusters and make tool-absence cases host-independent * no-mistakes(test): state authorised GitHub narrowing of bundled -R guard This branch NARROWS GitHub behaviour. The narrowing was authorised deliberately rather than slipping in by accident, and it applies to both providers, GitHub and GitLab alike, because a script that guards one provider and not the other is a trap for the next reader. What bin/fm-pr-merge.sh now refuses is extra merge arguments containing a bundled short-option cluster that includes R, for example "-dR other/repo". The forge CLIs expand such a cluster one character at a time, so it carries "--repo other/repo", and that later value wins over the repository the URL named. Before this change, "fm-pr-merge.sh <task> <github-url> -- -dR other/repo" reached "gh-axi pr merge 12 --repo example/repo --squash -dR other/repo" and exited 0 with pr= recorded and the merge poll armed. It now exits 1 with "extra merge arguments must not override the repository", records nothing, and invokes no forge merge command. Every other GitHub invocation is byte-identical to the base commit. Closing that hole honours the existing rule rather than departing from it. The file header already forbids --repo and -R because the repository must come only from the URL, so a bundled cluster carrying a repository override was never legitimate behaviour to preserve: it was that guard being evaded. Redirecting a merge to a repository the URL does not name is exactly what the guard exists to prevent. The refusal is already pinned on both paths by the existing case test_bundled_repo_override_args_refuse_before_recording in tests/fm-pr-merge.test.sh. On GitHub ("-dR wrong/repo") and on GitLab ("-yR https://other.example/g/p") it asserts exit 1, the refusal wording, no pr= in the task meta, no armed merge poll, and no forge merge command invoked, with a control case proving a cluster that carries no repository override still reaches the forge. No duplicate assertion was added. Both assertions were confirmed to have teeth by narrowing the guard back to a bare -R and watching each path fail. This commit carries no file change: the guard and its coverage landed in 614853d, and this message exists so the pull request description states the narrowing. * no-mistakes(document): fix README pointer for GitLab watch and merge doc * no-mistakes: apply CI fixes
kunchenguid#2788) * no-mistakes: apply CI fixes * fix(bin): drop a private record citation and narrow the review rule Three corrections to the spoken interface that landed in kunchenguid#2767, plus one fix carried over from that branch after its pull request had already been merged. The confidentiality fix. The module docstring of bin/fm-voice-relay.py cited a private, gitignored fleet record by exact path and section number. That widens what this public repository points at, and it cannot resolve for any reader here, because the path has never been in the repository. Both traps it pointed at are already described in full in the list immediately below it, and docs/voice-relay.md carries the same two for operators with no citation at all, so the pointer is removed and no claim is weakened by losing it. Two comments that referred to "the survey" as though it were something a reader could open are reworded the same way. Neither exposed a path, so that half is comprehensibility rather than confidentiality. The review rule. .greptile/rules.md is kept, because its conditions are right and deleting it would leave the next reviewer to re-litigate a decision already argued out. What was wrong with it is narrower than its existence: it read as settled repository policy, when whether VISION.md itself should be reconciled is an open question belonging to the captain. One sentence now says so, and says that the conditions listed below it are what the interpretation depends on. That narrows the claim rather than widening it. The carried-over fix. The first commit on this branch is 7f98e79 from fm/voice-relay-build-v4, taken verbatim rather than rewritten. It closes the window where a transport failure was recorded and then erased, so a run could be emitted as answered false with relay_error null. That matters more than it looks: relay_error is the field that keeps an infrastructure failure from being averaged into a latency figure, so the failure mode is a dead connection wearing the costume of a slow reply. It landed fifteen minutes after kunchenguid#2767 merged and so never reached the default branch. * no-mistakes(review): name a reason on every unanswered-turn close path * no-mistakes(review): guard the downlink body and pin frames to their turn * no-mistakes(review): attribute reply audio to its own turn and tell endings apart * no-mistakes(review): tell a cut-short reply from an unanswered turn * no-mistakes(review): discard reply audio arriving after the output closes * no-mistakes(review): count discarded reply audio on the speaker path too * no-mistakes(review): keep a reason off a turn already answered in full * no-mistakes(review): say a reset cut a reply short, not that none arrived * no-mistakes(review): read one turn's audio count once, and hush a tidy exit * no-mistakes(document): fix stale session-end relay_error claim in voice-relay guide
…unchenguid#2811) A pi worker parked on an interactive prompt - a permission dialog, a question menu, a trust dialog - reports agent_status=blocked, because it is waiting on a human keystroke. Pi draws that menu above its separator pair, so the composer region between the rules is blank and structure alone looks like a free composer. _fm_composer_pi_verdict admitted blocked alongside idle and done, so the shared classifier reported an affirmatively empty composer for exactly the pane where typing is unsafe. Every "is it safe to type here?" consumer reads that verdict and proceeds only on an affirmative empty, so both are told yes on a parked prompt: the away-mode injection guard in bin/fm-supervise-daemon.sh, and fm-send's pre-type refusal. The keys then answer the menu instead of composing a message - the highlighted default is selected, the text is discarded, and the record attributes a decision to a human who never made it. blocked now defers to unknown, which every consumer already treats as fail-closed. idle and done still prove an empty composer, so ordinary steering is unchanged, and Cursor is unaffected because its always-blocked panes never reach this pi-only branch. Regression coverage lands first at both levels: the verdict owner (a blocked pi defers) and the herdr adapter (a parked pi prompt is not an empty composer).
…2849) * fix(bin): require a clone root before fleet-sync touches a project Git repository discovery walks upward, so `git -C projects/<dir>` on a plain directory nested under projects/ resolves to the enclosing repository - in a firstmate home, the firstmate checkout itself. fm-fleet-sync.sh guarded its candidates with `rev-parse --is-inside-work-tree`, which such a directory passes, so every later git call read, pruned and fast-forwarded firstmate's own default branch and reported it under the project directory's label. A running session's AGENTS.md changed underneath it, and the report named a project that had nothing to do with the change. Require each candidate to be the root of its own work tree before any other git command: compare `rev-parse --show-toplevel` against the directory's own physical path. Both sides are physical, so a symlinked clone still compares equal. Anything else is skipped by name, naming the repository that would have been touched, and bootstrap relays that as a FLEET_SYNC line. Regression coverage reproduces the wrong-repo fast-forward against a home nested inside another repository, in both the whole-fleet and single-project forms, and pins that a symlinked clone dir still syncs. * no-mistakes(review): Keep enclosing fixture clean during clone-root regression
* fix(procevent): retry a transient Lavish poll interruption quietly
A live Lavish listener can be cut short by the server with exactly
error: Lavish Editor poll response was interrupted
code: SERVER_ERROR
while the session's marks remain available. Firstmate registered raw
`lavish-axi poll` output, so the generic process-event runner captured
that transient response as a result and woke the whole fleet over what is
really an internal retry.
The Lavish adapter now registers its own listener command, which reruns
the published blocking poll up to 12 times at 5 second intervals for that
one exact two-line response. The match is deliberately narrow: real
feedback, ended and missing sessions, any other SERVER_ERROR, and the same
interruption still standing once the bound is spent all pass straight
through and are captured and announced as before. The retry is a Lavish
fact, so the generic runner stays adapter-agnostic.
`FM_LAVISH_POLL_RETRY_DELAY` is a bounded 0 to 60 second override for the
interval only, refused rather than rounded when malformed, so a test can
exercise the real bound without waiting it out.
* no-mistakes(review): Harden Lavish retry matching, validation, and cleanup
* no-mistakes(review): Bound Lavish retry staging and stabilize regression
* no-mistakes(document): docs: explain Lavish retry adoption
* no-mistakes(lint): Restore Lavish trap ShellCheck suppression
… gate (kunchenguid#2838) The unguarded Herdr declaration quoted `{TASK}` in its own prose while the scaffold instructs firstmate to replace every `{TASK}` placeholder. The documented global replace therefore spliced the whole task body into the middle of the safety gate's sentence, silently destroying the one contract that exists precisely because the scaffold cannot inspect the task text. Reword the gate to refer to the task text filled in above, leaving the placeholder only at its genuine fill site. Rewording rather than renaming the token keeps the unfilled-charter guards in fm-home-seed.sh and fm-remote-home-seed.sh working unchanged. Add a regression test that performs the documented global fill on ship and scout scaffolds and asserts the body lands once and the gate survives.
…tat form (kunchenguid#2837) The writer lock's stale-lock branch read the lock's mtime with `stat -f %m ... || stat -c %Y ...`. On GNU coreutils `-f` is filesystem stat, so it consumed the format string as a path, complained on stderr, printed a partial filesystem dump (" File: ...") on stdout, and still exited 0. The GNU form in the fallback therefore never ran, and the following arithmetic evaluated the word `File`, aborting the writer under `set -u` with "File: unbound variable". fm-teardown.sh died there after returning the worktree, leaving state/<id>.meta, .status, .busy-gen, .busy-state, .busy-state.lock/ and .turn-ended behind. The surviving metadata kept the watcher monitoring an endpoint whose agent was gone, so a finished task produced stale wakes forever, and every re-run died identically because the abandoned lock was never broken. Detect the platform once and pick the right stat form, the pattern bin/fm-watch.sh already documents, and treat any non-numeric result as "just created" so a future portability surprise degrades to a lock-timeout refusal rather than killing teardown mid-way.
* fix(stow): give memory decay a per-pass horizon so the clock fires The tiered decay clocks were wall-clock only, while admission is per-pass: each /stow admits the findings that pass produced. In a home that stows daily those two rates diverge by the stow cadence, an entry the fleet keeps exercising never reaches 30 days unreinforced, and memory only grows while the pass reports decay evaluated. Give each dated marker an optional unreinforced-pass counter and make both tiers stale at whichever horizon comes first: 10 passes or 30 days for aging, 3 passes or 7 days for perishable. Reinforcement clears the counter and nothing else does, so the existing evidence-based restamp rule stays the only way an entry renews its lease. An absent /N means zero, so entries that stay exercised carry no extra marker bytes, and a rarely stowed home keeps its current behaviour through the unchanged date horizon. * no-mistakes(document): Align stow workflow with dual decay clocks * fix(stow): make the per-pass decay horizon opt-in The unreinforced-pass horizon shipped as a new default archival cadence, which is a product default rather than a restoration of the existing wall-clock contract. Keep the 30-day and 7-day horizons as the only default clock, and put the 10-pass and 3-pass horizons behind an explicit opt-in: config/stow-pass-horizon for the firstmate home, and the file's own header pointer for the public skill. With the opt-in absent no counter is written and no counter is read, so a home that does not ask for it decays exactly as it does today. * no-mistakes(review): Preserve frozen counters and correct archive provenance
… the stow pass horizon Batch 8 of eight in the fork reconciliation, and the last one. Brings the thirteen upstream commits from 738460d (kunchenguid#2758, make lint prerequisites and harness tests reliable) through f170ced (kunchenguid#2850, opt-in pass horizon for memory decay), including 59e7393 (kunchenguid#2684, watched tooling update reporting), 52d20f1 (kunchenguid#2764, decouple ask-user decisions from yolo), fbe37e9 (kunchenguid#2767, the spoken interface), dc0172c (kunchenguid#2763, Relay follow-up loops), 5b6d0fb (kunchenguid#2779, GitLab merge requests through the guarded merge path), 1231b6a (kunchenguid#2788, lost relay connection), 8714c9a (kunchenguid#2811, blocked pi composer), 801c083 (kunchenguid#2849, fleet-sync clone roots), 505c819 (kunchenguid#2846, Lavish poll retries), 86dd2f6 (kunchenguid#2838, the {TASK} fill and the Herdr gate) and 266fdb9 (kunchenguid#2837, the busy-state lock mtime stat form). Nine files conflicted. The one that matters is bin/fm-pr-merge.sh, where upstream kunchenguid#2779 added a GitLab merge path to a script the fork had grown three guards on. The merged script keeps all three - the merge-time static guard with its timeout refusal, the upstream-history guard, and the upstream-sync waypoint ancestry assertion - and runs every one of them on the GitLab path too, because a merge request can carry shared upstream history exactly as a pull request can. That took four provider-aware changes: the base branch now comes from glab's target_branch as well as gh's baseRefName, the head is fetched from refs/merge_requests/<n>/head as well as refs/pull/<n>/head, the project identity both guards compare against is the parsed forge-neutral path rather than an owner/repo pair GitLab has no equivalent of, and GitLab's merge method reads as rewriting because the project's own setting decides it and nothing on this command can prove or override that. Upstream's broader short-option cluster rejection replaces the fork's narrower one, and upstream's GitLab head-override rejection and live pre-merge verification are taken as written. All 24 fm-pr-merge cases, all 16 upstream-history and waypoint cases, and all 3 merge-time static guard cases pass on the merged script. Other resolutions: tests/fm-muse-harness.test.sh keeps the fork's symlink construction for the renamed muse-bin process, which macOS needs, and adds upstream's inherited-Cursor-marker scrubbing; tests/fm-remote-job.test.sh keeps both tail cases, with the fork's no-leaked-worker proof last so it still measures everything before it; tests/fm-pi-watch-extension.test.sh takes upstream throughout, whose session-lock case covers the fork's deterministic join and also exercises the hook entry point the fork's skipped; bin/fm-test-run.sh unions both weight-hint lists; docs/architecture.md, docs/configuration.md, docs/scripts.md and docs/documentation-audiences.json union both sides' additions.
Upstream kunchenguid#2779 added a GitLab merge path to bin/fm-pr-merge.sh, and this batch's resolution runs the fork's upstream-history and waypoint-ancestry guards on it. Nothing pinned that: every existing case addresses GitHub. Three cases on the existing fixture, whose origin now also publishes the request head where GitLab publishes it and whose GitLab variants drop refs/pull/<n>/head so the GitHub refspec cannot resolve the head for them: - a merge request carrying upstream history is refused, names the commits it would erase, and names a remedy that forge actually has - --merge does not excuse such a merge request, because glab takes no merge method flag and the project's own setting decides the result - an ordinary merge request with a reachable waypoint passes both guards and merges through glab at its verified head Each fails when its half is reverted: dropping the refs/merge_requests refspec leaves the first unable to name any erased commit, and restoring the squash default for GitLab lets --merge merge a batch that would be flattened.
Upstream kunchenguid#2763 expands PF_REGISTRY_LOCK_IDS and the release helper's local remaining array with the plain "${arr[@]}" form. Under set -u bash 3.2 treats that as an unbound variable when the array is empty, and macOS ships 3.2, so the first registry-lock check on a fresh run aborted: bin/fm-public-followup.sh: line 142: PF_REGISTRY_LOCK_IDS[@]: unbound variable not ok - could not register the public commitment Both sites take the guarded ${arr[@]+"${arr[@]}"} form this repo already uses in fm-test-run.sh, fm-spawn.sh and fm-pr-merge.sh. tests/fm-backlog-handoff.test.sh goes from exit 1 to 13 ok.
The thirteen commits this batch brings in were written against GNU userland
and bash 4, and four of their surfaces do not work under macOS's bash 3.2 and
BSD coreutils. Each was found by running the portable lanes here.
- bin/fm-inbox.sh queued a note with a PENDING id and rewrote it via `sed -i`,
whose in-place flag takes a mandatory suffix argument on BSD and none on
GNU, so there is no portable spelling. The staging name is what makes the id
unique, so the id is now resolved before the record is written and goes in
directly; the placeholder and the rewrite are gone. `note` was completely
broken here before this.
- tests/fm-tool-update-check.test.sh and tests/fm-voice-relay.test.sh compared
`$(wc -l ...)` against a bare integer with `=`. BSD wc pads its count with
leading spaces, so every such comparison was false here. They now strip the
padding the way tests/fm-backend-herdr.test.sh and tests/fm-procevent.test.sh
already do.
- tests/fm-public-followup.test.sh substituted an emit path with
${command/"$a/b"/"$c/d"}. bash 3.2 still splits on a slash written inside
the pattern's own quotes, so the substitution landed in the wrong place and
produced an unrunnable command. Both halves are bound to variables first.
Two fixture problems in the same new tests, neither platform-specific:
- Four teardown fixtures in tests/fm-public-followup.test.sh drive a kind=ship
teardown without the completion report this fork's bin/fm-teardown.sh
requires, so teardown refused at that gate before reaching the public-reply
gate each case is about. One of them asserted only the absence of a message
and was passing vacuously. They now call the file's own
write_completion_report helper, which exists for exactly this reason.
- The promised-final fixtures pinned followup_expires_at to a literal
2026-08-28, which was a future window when upstream wrote it and is not one
now, so every rechain case refused on an unreachable thread. The window is
anchored a week ahead of the run, and the expiry-escalation case derives its
FMX_NOW_OVERRIDE clock from the same anchor rather than a second constant.
tests/fm-public-followup.test.sh goes from 33 ok / 1 not ok to 52 ok,
tests/fm-voice-relay.test.sh from 4 ok / 1 not ok to 40 ok, and
tests/fm-tool-update-check.test.sh from 0 ok / 1 not ok to 37 ok.
…uard wording, and two weak fixtures
…uard in gitlab-merge-watch.md
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
Sync the fork forward to upstream waypoint f170ced (upstream PR kunchenguid#2850, "add opt-in pass horizon for memory decay") as a single reviewed MERGE. This is batch 8 of eight planned batches; batches 1-7 have landed and batch 7's waypoint 3d125ad is already in origin/main's ancestry. The batch brings thirteen upstream commits, 738460d (kunchenguid#2758) through f170ced (kunchenguid#2850): lint prerequisites and harness tests (kunchenguid#2758), watched tooling updates report (kunchenguid#2684), ask-user decoupled from yolo (kunchenguid#2764), the spoken interface (kunchenguid#2767, large), Relay follow-up loops (kunchenguid#2763), GitLab MR merge path (kunchenguid#2779), lost relay connection (kunchenguid#2788), pi composer blocked-pane (kunchenguid#2811), fleet-sync clone roots (kunchenguid#2849), Lavish poll retries (kunchenguid#2846), brief {TASK} Herdr gate (kunchenguid#2838), busy-state lock mtime stat form (kunchenguid#2837), and the stow pass horizon (kunchenguid#2850).
REQUIRED SHAPE, deliberate and not to be "corrected": branch off origin/main, git fetch upstream, git merge f170ced producing a MERGE COMMIT. Never rebase, squash or cherry-pick this branch. Stop exactly at f170ced; the ~77 upstream commits beyond it are a separate re-plan and are deliberately excluded. The Rebase step is skipped for this run for that reason, because rewriting the branch would drop the upstream commit identities from ancestry - that is the exact defect this batch procedure exists to prevent (upstream 7f5255a was erased from main that way on 2026-08-25). The merge commit and the preserved upstream commit identities ARE the deliverable, so a large diff with 14802 insertions across 72 files is expected and correct.
Nine files conflicted across 21 hunks and every hunk was resolved by hand with a stated reason.
The collision that mattered is bin/fm-pr-merge.sh. The fork carries three guards upstream lacks: the merge-time static guard including its timeout refusal (fork PR 21), the refusal to squash or rebase a PR carrying upstream history (fork f290988), and the upstream-waypoint ancestry assertion (fork PR 32). Upstream kunchenguid#2779 adds a GitLab merge-request path through the same guarded script. The accepted requirement was that the merged script must carry ALL of the fork's guards AND upstream's GitLab path, with every guard applying to the GitLab path too where it makes sense, because a GitLab merge request can also carry upstream history. That required four deliberate provider-aware changes: the base branch now comes from glab's target_branch as well as gh's baseRefName; the request head is fetched from refs/merge_requests//head as well as refs/pull//head; the project identity both guards compare a remote against is the parsed forge-neutral FM_PR_PATH rather than an owner/repo pair GitLab has no equivalent of, with upstream_remote_slug returning the whole project path rather than its last two segments so a nested GitLab namespace compares equal (both forms are byte-identical for every GitHub URL); and resolved_merge_method returns empty for GitLab unconditionally, so even an explicit --merge does not excuse a merge request carrying upstream history, because GitLab applies the project's own merge method, takes no flag for it, and glab has no --merge flag at all. That asymmetry with GitHub is intentional and fail-closed, and the refusal names a GitLab-appropriate remedy instead of a flag that forge does not have. Upstream's broader short-option cluster rejection (-yR) replaces the fork's narrower -R form because it is strictly broader; upstream's GitLab head-override rejection and live pre-merge verification are taken verbatim.
Other conflict resolutions, all deliberate: tests/fm-muse-harness.test.sh keeps the fork's symlink construction for the renamed muse-bin process, which is required because macOS SIGKILLs a copied system bash, and adds upstream kunchenguid#2758's inherited-Cursor-marker scrubbing - both halves are needed. bin/fm-test-run.sh unions both portable_serial_weight_hints lists into one sorted 122-entry list, taking upstream's value where both name a file, and leaves the fork's FM_TEST_INHERITED_OVERRIDES scrub owner untouched. tests/fm-pi-watch-extension.test.sh takes upstream throughout: its session-lock case is a strict superset of the fork's, covering the same deterministic join through ensureArmed plus the hooks.event entry point the fork's version stopped exercising, re-expressed through the sessionID binding that auto-merged around the hunk. tests/fm-remote-job.test.sh keeps BOTH tail cases because neither contains the other, with the fork's no-leaked-worker proof deliberately placed last so it still measures everything before it. The four documentation surfaces are additive on both sides and were unioned, and docs/documentation-audiences.json stays valid for bin/fm-doc-audience-check.sh.
One deliberate wording change outside any conflict: the merge-time static guard's message and the one sentence in docs/merge-time-static-guard.md that owns it now say "merge result" and "the branch it targets" rather than "squash result" and "default-branch tip", because the guard now also runs on GitLab merge requests where the GitHub squash default does not apply.
Three regressions were added to tests/fm-merge-waypoint-guard.test.sh because nothing pinned the fork's guards on the new GitLab path: a merge request carrying upstream history is refused and names both the erased commits and a GitLab-appropriate remedy; --merge does not excuse such a merge request; and an ordinary merge request with a reachable waypoint passes both guards and merges through glab at its verified head. The fixture's GitLab variants deliberately delete refs/pull/7/head so the GitHub refspec cannot resolve the head and the GitLab one is actually proven. Each regression was demonstrated to fail when its half of the implementation is reverted.
Running the full portable lanes then surfaced six defects the upstream tail brings in that had never run on macOS or against this fork's stricter teardown gate. All six are upstream defects rather than merge damage, and all six are fixed on this branch: an empty-array expansion under set -u in bin/fm-public-followup.sh that bash 3.2 refuses; sed -i with no suffix in bin/fm-inbox.sh, which has no portable spelling and left fm-inbox.sh note entirely broken here, fixed by resolving the record id before the record is written rather than rewriting a placeholder; wc -l counts compared as strings in two new test files, which BSD wc pads with leading spaces, fixed the way this repo's other tests already do it; a path substitution written inline in a parameter-expansion pattern in tests/fm-public-followup.test.sh, which bash 3.2 splits on the quoted slash, fixed by binding both halves to variables; four ship-teardown fixtures that do not satisfy this fork's completion-report teardown gate, one of which was passing vacuously, fixed by calling the file's own write_completion_report helper; and a promised-final expiry hardcoded to a literal 2026-08-28 that was a future window when upstream wrote it and is now in the past, fixed by anchoring the window a week ahead of the run with the expiry-escalation case deriving its clock override from the same anchor.
Upstream kunchenguid#2764 rewrites AGENTS.md section 7 and the ask-user-authority skill so finding authority is that skill's criteria rather than the project's yolo posture. It merged cleanly and was deliberately taken exactly as upstream wrote it, not adapted; the policy consequences are reported to the captain separately rather than resolved here.
Repo constraints that apply: one sentence per line in tracked Markdown and plain dashes rather than em dashes, except that a file carried byte-identical from upstream is exempt; bin/.sh and bin/backends/.sh must pass bin/fm-lint.sh; no agent name as a commit co-author; and projects/, data/, state/ and config/ are not to be modified.
Validation already run locally and green: bin/fm-test-run.sh --check-coverage, bin/fm-doc-audience-check.sh, bin/fm-lint.sh, all of tests/fm-pr-merge.test.sh, tests/fm-merge-waypoint-guard.test.sh, tests/fm-static-guard.test.sh, tests/fm-muse-harness.test.sh, tests/fm-kimi-harness.test.sh, tests/fm-pi-watch-extension.test.sh, tests/fm-remote-job.test.sh, tests/fm-public-followup.test.sh, tests/fm-voice-relay.test.sh, tests/fm-tool-update-check.test.sh, tests/fm-backlog-handoff.test.sh, tests/fm-lint.test.sh and tests/fm-lint-workflows.test.sh, plus all six portable lanes. Two known lane failures remain and are NOT from this merge: tests/fm-backend-herdr-focus-flash-e2e.test.sh fails on the environmental Herdr fleet-state tripwire, and tests/fm-watcher-lock.test.sh is red on this machine while its test file and every script it exercises are byte-identical to origin/main on this branch (its filed guard-xmode failure, plus a load-sensitive fixed-wait HUP case that masks it under concurrent lanes).
What Changed
bin/fm-pr-merge.shwith upstream's GitLab merge-request path while keeping all of the fork's existing guards (merge-time static guard, squash/rebase-of-upstream-history refusal, waypoint-ancestry assertion) applying to GitLab requests too: provider-aware base-branch resolution (gh baseRefName/glab target_branch), head fetch fromrefs/merge_requests/<n>/head, forge-neutral project-identity comparisons, a GitLab-specific head-divergence TOCTOU check, andresolved_merge_methodreturning empty for GitLab so--mergecannot bypass the guard.bin/fm-inbox.sh,bin/fm-tool-update-check.sh,bin/fm-voice-client.py,bin/fm-voice-relay.py,bin/fm_voice_frame.py,bin/fm_voice_records.py,docs/voice-relay.md,.greptile/rules.md, plus matching test suites (tests/fm-voice-relay.test.sh,tests/fm-tool-update-check.test.sh, and expanded coverage intests/fm-pr-merge.test.sh,tests/fm-merge-waypoint-guard.test.sh,tests/fm-public-followup.test.sh, and others).bin/fm-public-followup.sh,sed -iwith no portable suffix inbin/fm-inbox.sh,wc -lstring-vs-numeric comparisons in new tests, a quoted-slash parameter-expansion split intests/fm-public-followup.test.sh, teardown-gate fixtures that didn't call the completion-report helper, and a hardcoded test expiry date that had moved into the past.AGENTS.mdand.agents/skills/ask-user-authority/SKILL.mdper upstream fix: decouple ask-user decisions from yolo kunchenguid/firstmate#2764 so authority-finding is driven by the skill's own criteria rather than the project's yolo posture.Risk Assessment
✅ Low: All nine prior findings, including the critical GitLab head-divergence TOCTOU bypass, were verified fixed by tracing the actual guard/merge control flow (merge_refs_ensure is unconditionally invoked for GitLab via upstream_guard_evaluate, populating MERGE_REFS_BASE/HEAD before the new post-verification equality check gates the merge), backed by a correctly constructed regression test; only a minor, non-functional documentation-wording gap remains.
Testing
Ran the four test suites most directly exercising this batch's collision area and macOS/bash-3.2 fixes (fm-merge-waypoint-guard, fm-pr-merge, fm-static-guard, fm-public-followup) plus fm-doc-audience-check — all green, no failures, matching the counts the author reported locally. Independently confirmed the merge commit has two parents (true merge, not rebase/squash) and manually exercised the fm-inbox.sh note fix end-to-end in an isolated temp home since no automated test covers it, producing a correctly formed note file. Spot-checked the wording, wc -l, and bash-3.2 array-guard fixes directly in source to corroborate the intent's specific claims. No regressions or flakiness found; working tree left clean.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
Step was skipped.
bin/fm-pr-merge.sh:459- On the GitLab merge-request path, every fork guard (upstream-waypoint ancestry, upstream-history refusal, and the merge-time static guard) evaluates MERGE_REFS_HEAD, but the actual merge is pinned to a separately-read FM_PR_MERGE_HEAD, and the two values are never compared anywhere in the script. This was demonstrated end-to-end in a sandbox: with the fetched refs/merge_requests/7/head pointing at a clean commit and the glab mr view JSON's .sha pointing at a commit carrying two upstream waypoints, the run printed 'merge-guard: UNGUARDED' and 'verified: ... at head 35d7071...', then executedglab mr merge 7 -R ... --sha 35d7071... --yes(rc=0) merging the upstream-carrying commit the guards never actually inspected. This is exactly the 'erases upstream history' outcome the fork's three guards exist to prevent, reachable whenever a merge request's head moves between the guard reads and the merge (the static guard alone runs a project linter in between, so the window is not merely theoretical). This directly undercuts the user intent's required property that all fork guards apply to the GitLab path the same way they do on GitHub.bin/fm-pr-merge.sh:446- When the refs/merge_requests/<n>/head fetch fails, the GitLab path falls back to reading pr_head from task meta - but bin/fm-pr-check.sh only ever records pr_head= for GitHub, never for GitLab. So an unfetchable GitLab head silently degrades the static guard to UNGUARDED (merges anyway), and if an operator has hand-written a pr_head= value (a supported shape per tests/fm-pr-merge.test.sh:659), the guards evaluate that stale commit while the merge is still pinned to the live head - a deterministic instance of the same head-divergence class as the HIGH finding above, rather than a racy one. The same fix that resolves the HIGH finding (verifying the guarded head equals the merged head before merging) would close this path too.docs/gitlab-merge-watch.md:236- The doc states 'Every output is reproduced exactly' (line 5), but the adopted transcripts for the merge path (lines 236-276) show only 'armed: ...' followed directly by 'error: refusing to merge ...', with no merge-guard line in between. Against the fork's actual guarded script, every one of those invocations emits a 'merge-guard: ...' line before the refusal and writes merge_guard= into the task meta (bin/fm-pr-merge.sh:938) - reproduced directly in the background review's sandbox. The evidence appears to have been carried from upstream's unguarded transcript rather than re-run against the fork's merged, guard-bearing script.bin/fm-static-guard-lib.sh:7- The user intent states the merge-time static guard's wording was updated from 'squash result'/'default-branch tip' to 'merge result'/'the branch it targets' because the guard now also runs on GitLab merge requests. That rewording only landed in bin/fm-pr-merge.sh's guard message and docs/merge-time-static-guard.md:13. This file's header comment (lines 7-8), which documents bin/fm-pr-merge.sh's contract and explicitly says the two 'must not drift', still reads 'checks the exact squash RESULT of a PR against the current default-branch tip', and lines 30-31's TRUST comment still says the check command is discovered 'ONLY from a trusted revision - the current default-branch tip', which no longer matches merge_guard_base_branch() resolving GitHub baseRefName / GitLab target_branch (not necessarily the default branch) before falling back to the default branch.tests/fm-public-followup.test.sh:1440- The a995859 commit message says one of four teardown fixtures 'asserted only the absence of a message and was passing vacuously' and claims this is fixed by calling write_completion_report. In test_dropped_baton_now_surfaces_open_loop (~lines 1440-1443) the teardown call's exit code is still discarded with|| true, and the sole gating assertion checks for the absence of the string 'still owes a public reply'. A teardown refusal at an earlier, unrelated gate (e.g. a missing completion report) also would not contain that string, so this assertion would still pass even if write_completion_report were absent or ineffective for this fixture - unlike the other three fixtures, which gained a positive assertion (exit-code check or assert_contains on the expected message).docs/merge-time-static-guard.md:34- The same doc file the user intent says now 'owns' the updated sentence (line 13) still describes the guard as reading 'the current default-branch tip' at lines 34 and 69, inconsistent with the branch-it-targets wording introduced at line 13 for the GitLab-aware guard.bin/fm-pr-merge.sh:31- Within the same header comment block where line 22 was updated from 'default-branch tip' to 'base-branch tip', line 31 nine lines later ('THE GUARD DOES NOT SURVIVE AN AIRGAPPED SITE... the current default-branch tip') was left unupdated.AGENTS.md:334- Pre-existing fork-authored line ('That merge path re-checks the exact merge result against the current default branch and refuses a red one') is now stale for the GitLab/non-default-target case this batch's wording change was made for, while the equivalent sentences in docs/architecture.md and docs/scripts.md were updated to base-branch language.bin/fm-public-followup.sh:160- pf_registry_lock_release'sfor held in "${PF_REGISTRY_LOCK_IDS[@]}"remains the unguarded form (inconsistent with the sibling fix at line 144). Not currently reachable with an empty array because pf_registry_lock_held returns 1 (early return at line 158) whenever the array is empty, but it is a latent trap: a future edit that removes or reorders that early return would silently reintroduce the bash-3.2 'unbound variable' crash this same commit set out to eliminate.🔧 Fix: Fix GitLab merge head-divergence TOCTOU, stale guard wording, and two weak fixtures
1 info still open:
bin/fm-static-guard-lib.sh:25- The 'THIS DOES NOT SURVIVE AN AIRGAPPED SITE' paragraph (lines 25-29) still reads '...the guard also fetches the current default-branch tip from the forge.' The fix round updated the header contract (lines 7-8) and the TRUST paragraph (lines 31-36) to say the merge-time guard (bin/fm-pr-merge.sh) reads 'the current tip of the branch being checked against' rather than the default-branch tip, but left this paragraph, which describes both callers together, still asserting the default-branch-tip framing unconditionally. This is the same wording-drift class the fix round was meant to finish in this exact file, which its own header says the two callers 'must not drift' from.✅ **Test** - passed
✅ No issues found.
git show --no-patch --format="%H %P" b6cfd62— verified two-parent merge commit (base + f170ced), confirming true merge rather than rebase/squashbash tests/fm-merge-waypoint-guard.test.sh— 20/20 pass, incl. 3 new GitLab regression tests for upstream-history refusal, --merge non-excuse, and clean-MR pass-throughbash tests/fm-pr-merge.test.sh— 24/24 pass, covering fork guards + upstream GitLab MR merge path in bin/fm-pr-merge.shbash tests/fm-static-guard.test.sh— 17/17 passbash tests/fm-public-followup.test.sh— 52/52 pass on macOS bash 3.2bash bin/fm-doc-audience-check.sh— 77 surfaces / 282 local links validatedManual:FM_STATE_OVERRIDE=<tmp> bin/fm-inbox.sh note "..."in an isolated temp home — confirmed the id-resolved-before-write fix produces a correctly named/content-matching .note file (previously described as entirely broken on macOS)Manual: grepped bin/fm-pr-merge.sh and docs/merge-time-static-guard.md for 'merge result'/'the branch it targets' vs stale 'squash result'/'default-branch tip' wording — confirmed consistentManual: confirmedwc -l | tr -d ' '/[:space:]present in tests/fm-public-followup.test.sh and tests/fm-fleet-sync.test.sh to neutralize BSD wc paddingManual: confirmed PF_REGISTRY_LOCK_IDS array expansions use the bash-3.2-safe${arr[@]+"${arr[@]}"}guard pattern✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.