feat(bin): merge upstream/main into fork, adding workflow linting and decision-hold bindings - #8
Merged
Merged
Conversation
…d mate (kunchenguid#2457) The lightweight Relay follow-up link lives in the answering home's own state/<task-id>.meta, so it can only bind work that home owns. When a Relay-linked request is routed to a second mate, the task record lives in the second mate's home, fm-x-link.sh failed with a bare "no such task ...meta", and nothing else picked the promise up: only the soft acknowledgement was ever posted. The typed promised-final path already supports --work-home secondmate:<id>; the playbook simply never chose it. - fmx-respond now states the routing rule crisply: a task in this home takes the lightweight link, and second-mate-routed work takes a promised-final commitment bound to that home, registered up front with the brief command carried into the routed worker's instructions. - fm-x-link.sh refuses a task with no local record by naming the registered second mate whose home actually holds it and printing the promised-final registration command, with the exact --work-home when the match is unambiguous. A home with no registered second mates keeps the plain error. - fm-backlog-handoff.sh reports, after a successful move, any moved key that still owes a public reply bound to main/<key>, since that binding no longer names the home owning the work. The move itself is never blocked. Docs and the secondmate handoff prose follow the same rule. Tests cover the refusal, its scoping, the unchanged local-link path, and both handoff outcomes at the script boundary.
…verdicts (kunchenguid#2456) * fix(skills): hint that remote secondmate liveness verdicts false-negative fm-crew-state and fm-send routinely misreport a live remote secondmate as dead; confirm against the pane before relaunching, and relaunch only through fm-spawn.sh, never raw herdr pane surgery. * no-mistakes: apply CI fixes
Pi 0.83.0 added a status line to every tool-expansion change, and Pi updates the previous status line in place when two status messages arrive back to back. Calm's post-export redraw cycled tool expansion on the macrotask right after Pi printed "Session exported to: <path>", so both expansion status lines coalesced over that confirmation and the captain was left with no record of where their export landed. Calm now repaints only the tool rows it presents, by invalidating each row through the render context Pi hands its render slots, and requests the surrounding redraw through setStatus. Neither appends to the transcript. The repaint is still needed because Pi can re-render a row asynchronously - the built-in edit row invalidates itself once its diff is ready - and that re-render can land inside the window where /export forces stock rendering. The real-terminal /export case now asserts the confirmation is still on screen after the redraw has settled, and that the redraw restored every Calm-hidden row, instead of only racing the moment the confirmation first appeared.
…nguid#2488) * feat(stow): persist the open records a session is holding /stow curated memory and captured session knowledge, but never touched record state, while AGENTS.md called it an "unfinished-work sweep" and the receipt declared the session "safe to reset" - wording that implied a record-correctness guarantee stow does not make. A shipped PR with no backlog item, a queued umbrella whose phases had merged, and four decision holds left open after their answers shipped all survived repeated stows. Add a bounded pass that files record state from the same volatile input the rest of stow already uses: the open threads in context, minutes before the reset destroys them. It creates a record for an unfiled thread and corrects one the session knows is wrong, through the owning path, and states its boundary as part of the contract - it never enumerates the backlog, lists holds, or queries a forge, because it cannot be a reconciliation and must not be read as one. Correct the wording in AGENTS.md and the completion receipt so reset-safe means what it actually guarantees: nothing this session knew was lost. * no-mistakes(review): correct stow decision-hold inspection to read hold via tasks-axi * no-mistakes(document): note /stow open-record persistence in README command catalog * refactor(stow): state open-record persistence as principle, not procedure The first version enumerated triggers, named commands, and prescribed an ordered procedure. That is too rigid for an agent skill: it invites literal execution of a checklist instead of judgment, and every enumerated example is a way for the guidance to go stale. Reduce it to the intent - before a reset, the important open work you are holding in context must end up durably recorded rather than dying with the session, filing what is unfiled and correcting what is stale - and let the agent judge importance, the record, and the owning write path. Keep the scope bound, since it is a decided contract and not a mechanic: this covers the open work the session is holding, never a reconciliation of durable records against repository or forge reality. The wording corrections in AGENTS.md and the completion receipt are unchanged.
…eyed-answer path (kunchenguid#2490) * fix(decisions): close captain holds at answer time Firstmate had two "a decision is open" ledgers with asymmetric closing mechanics. The live status-log ledger closes atomically at answer time, because bin/fm-send.sh --resolve-key makes answering a decision be the act that closes it. The durable backlog hold ledger had no such coupling: answering and recording were two separate acts, and only the first was forced by the workflow. That asymmetry lost four real captain decisions. Their answers were captured durably to disk, keyed character for character by the hold decision keys, acknowledged, and even implemented and shipped, yet the holds stayed open for two days and the captain was asked to re-answer decisions already on his own disk. Give the hold ledger the same answer-time-closure property: - bin/fm-decision-hold.sh gains an `answer` subcommand, the hold ledger's counterpart to --resolve-key. It shares one unrouted close implementation with `decline`, so it carries every existing guard - the captain decision file, the active-hold requirement, retry identity, and the refusal to release still-routed work - and differs only in the resolution mode it records. `decline` keeps its stronger meaning that the answer routes no follow-up work at all. - bin/fm-procevent-lavish.sh wires the channel that actually carried the lost answers. `arm --decisions-origin` binds a deck to the origin whose holds it carries, `answers` reads the structured choices out of a captured poll result, `close-decisions` maps each key to its hold and closes it through the command above, and `autohandle` lets the runner apply that at capture time. Safety is preserved rather than traded away. Only rows tagged `choice` are read, so freeform captain prose cannot forge a decision key. Closure is confined to the one bound origin. The decision text is a pure function of the captured result, so a replayed capture is idempotent. A hold that is absent, already closed, or still blocking routed work is skipped and left for `resolve`, never forced. A deck armed without the binding touches no hold at all. And autohandle deliberately never reports full handling, because recording an answer is transcription while acting on it is firstmate's judgement - so the check wake still reaches the handler. fm-send --resolve-key is untouched. * no-mistakes(document): document state/lavish-decisions binding dir in AGENTS.md state inventory * refactor(decisions): make keyed-answer closure one general capability The previous pass gave holds answer-time closure but built it as bespoke Lavish wiring: the review adapter carried the source-to-origin binding, mapped keys to hold identities, wrote decision records, decided what to skip, and closed holds itself. That treated a review deck as a special decision source. It is not - it is an ephemeral discussion format that happens to carry answers. Collapse it into ONE general capability with one owner. bin/fm-decision-hold.sh now owns the whole of "a keyed answer closes its matching hold": - `answers <origin> --source <provenance>` is the channel-agnostic intake. It reads key/answer/label lines on stdin, maps each key to its hold, and closes it through the same `answer` path, so every guard applies identically whatever channel the answer came from. --source is provenance recorded in the decision, never a behavior switch; there is no per-channel branch and no knowledge of chat, decks, or transports. - `bind`/`unbind`/`binding` own the source-to-origin binding for any channel whose answers arrive detached from their origin. Every channel is now an ordinary caller that only turns what it received into keyed lines: - bin/fm-send.sh (chat) feeds the intake for a key that names an active hold. This also fixes a real gap: once `complete` transfers a decision to its hold it closes the live status copy, so --resolve-key alone could never answer a transferred decision. - bin/fm-procevent.sh feeds it generically. A bound source's captured result goes to `<adapter> answers <result-file>` and whatever that prints is piped into the intake. The runner names no adapter, parses no result, and carries no decision rule, so any future adapter with an `answers` command works with no change here. - bin/fm-procevent-lavish.sh keeps only `answers`, which reports the structured choices a review captured and stops. It maps nothing to a hold and closes nothing; it lost ~160 lines of decision logic. Feeding is independent of handling, so it never acknowledges a result and never suppresses a wake - recording an answer is transcription, acting on it stays firstmate's judgement. The regression that proves closure now drives a FIXTURE adapter that is not the review adapter, so what is proven is that any bound channel reaches the intake rather than that one channel is wired specially. A new regression drives the real fm-send over a stubbed transport for the chat side. Every prior guarantee still holds, and fm-send's status-log behavior is unchanged. * no-mistakes(review): test(decisions): drop source-content grep from hold-closure regression
…mlink (kunchenguid#2512) A Write aimed at CLAUDE.md followed the symlink and destroyed AGENTS.md. The installer now creates and migrates to a recoverable two-line pointer file.
* fix(lint): catch malformed GitHub workflows before merge A self-broken ci.yml cannot report its own breakage, so parse every workflow in the local lint path that no-mistakes already runs. * fix(lint): pin actionlint instead of Ruby for workflow lint A self-broken ci.yml still has to fail in the local lint path, and the named tool for that gate is actionlint, not a new Ruby runtime. * no-mistakes(document): Clarify pinned workflow lint documentation
…d#2546) * fix: install pinned shellcheck and actionlint on macOS and linux arm64 The installers were hardcoded to linux amd64 and sha256sum, so a Mac dev could not satisfy the refuse-on-mismatch lint gate. Select the official per-platform archive and checksum, and fall back to shasum -a 256. * no-mistakes(document): Document cross-platform pinned lint installers
…uid#2548) .no-mistakes.yaml has set test.evidence.store_in_repo: true since kunchenguid#2355, but CONTRIBUTING.md, docs/configuration.md, and docs/architecture.md still described the old policy of keeping evidence out of the repo in a temp directory. The current no-mistakes behavior for store_in_repo: true is to publish each run's test evidence to the orphan no-mistakes/evidence branch and link it from the PR body. That branch shares no history with code branches, so evidence never enters a pushed feature branch or the default branch, and CI's tracked personal fleet paths rule stays accurate. Docs only. No change to .no-mistakes.yaml or any workflow.
* docs: correct test evidence storage comment in .no-mistakes.yaml * no-mistakes: apply CI fixes
Brings in 11 upstream commits. Three conflicts resolved by keeping both sides' meaning: AGENTS.md section 2 layout block, the stow skill description, and the process-event-sources arming section.
…resolution The conflict resolution had kept our dropped 'e.g.'; the captain's standing direction is to prefer upstream wording where the two genuinely conflict. Only the block-scalar reflow remains ours, so the file keeps one sentence per line as the repo style requires.
Owner
Author
Merge resolution recordMerge base: Upstream commits brought in
Behaviour changes we rely on, flagged:
Conflicts and how each was resolvedThree conflicts, all charter prose. No functional file conflicted. 1.
2.
3.
No conflict resolution dropped a rule, and no case arose where preferring upstream wording would have cost one of our constraints. Before/after countsEvery touched file is exactly
* An automated sweep additionally confirmed that every line upstream added since the merge base is present in the result; the only two it flagged were the two resolutions above that were reflowed to fit, both verified semantically complete. Our work, confirmed intactVerified byte-identical to
Note on the decision-key parsing fix ( Test evidenceThe full suite reports 21 failing scripts on this branch. A control run of the same scripts on pre-merge
Findings not fixed
|
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
2 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 this fork (digbycampbell/firstmate) with upstream (kunchenguid/firstmate) so pending charter fixes land on top of current upstream rather than under it. origin/main was 6 ahead and 11 behind upstream/main, with genuinely diverged histories.
MECHANIC: use a MERGE, not a literal rebase. The captain said 'rebase our fork' meaning 'bring it up to date'. Rebasing published history was explicitly ruled out: our main is already published and tracked by six worktrees and two secondmate homes, so rewriting it would strand all of them, and it contains a merge commit that rebase handles badly. The established precedent is our own f18ba40 'Merge remote-tracking branch upstream/main' landed as PR #6; this follows that shape: branch, merge upstream/main into it, resolve conflicts, PR.
SCOPE: merge only. Deliberately do NOT improve, reword, re-slim, or refactor anything while merging. Do not touch projects/, data/, state/, or config/. This is why the diff contains no independent improvements - that is intended, not an oversight.
CONFLICT RULES the captain set, in force:
THREE CONFLICTS, all charter prose, no functional conflicts:
(a) .agents/skills/stow/SKILL.md description - genuine competing wording; took upstream's sentence verbatim (including its 'e.g.'), reflowed into our YAML block scalar only so the file keeps the repo's one-sentence-per-line Markdown style. A follow-up commit corrects an earlier resolution that had kept our dropped 'e.g.', to comply with rule 2.
(b) .agents/skills/process-event-sources/SKILL.md - NOT competing wording: both sides added different paragraphs at the same spot. Kept both, our Slack adapter paragraph plus upstream's new decision-hold binding rule verbatim.
(c) AGENTS.md section 2 layout block - the one place upstream was not taken wholesale, deliberately. Upstream made only two changes inside that block while our side had restructured the whole block in PR #1; taking upstream's block wholesale would not be a wording choice, it would revert our landed restructure. So upstream's two facts were applied into our structure instead: the 'CLAUDE.md is a real @AGENTS.md pointer' correction, and the new state/decision-bindings/ entry carrying its full semantics (written only by fm-decision-hold.sh bind, dropped by unbind or source retirement, plus the docs/decision-hold-lifecycle.md pointer). This is why AGENTS.md is 451 lines rather than the mechanical 449: upstream's one-line decision-bindings entry needed three lines in that block's column layout.
CLAUDE.md is intentionally no longer a symlink. Upstream commit 4913723 deliberately replaced it with a real two-line @AGENTS.md pointer because a Write through the symlink destroyed AGENTS.md, and CI (bdae21e) now enforces that shape. Our side never changed CLAUDE.md, so the merge correctly took upstream's file. .claude/skills remains a symlink to .agents/skills - that is a different, still-valid symlink.
VERIFICATION DONE: every touched file's line count is exactly ours + (upstream - base), and an automated sweep confirmed every line upstream added since the merge base is present in the result. Full suite showed 21 failing scripts; a control run of the same scripts on pre-merge origin/main in a throwaway clone reproduced 18 identically. The three that differ are all missing-tool environmental, not regressions: tests/fm-lint.test.sh and the new tests/fm-lint-workflows.test.sh both pass once upstream's pinned actionlint is installed (verified by installing it to a temp dir and re-running), and tests/fm-watch-checkpoint.test.sh passed 3/3 in isolation after being polluted in the full run by a watcher killed in the preceding script. bin/fm-lint.sh and bin/fm-doc-audience-check.sh are clean, and bin/fm-session-start.sh produces a full digest on the merged tree.
KNOWN PRE-EXISTING DEFECT, deliberately NOT fixed here because this is a merge-only change: tests/fm-procevent-slack-captain.test.sh already fails on origin/main - the poll leaves its /tmp/fm-slack-captain.* staging directory behind, containing the captured message payload. That is our own adapter's bug, present before this merge, and it is reported to the captain as a separate finding rather than folded into a merge.
What Changed
upstream/maininto the fork'smain(merge, not rebase, since our published history is tracked by multiple worktrees and secondmate homes and contains a merge commit). This lands upstream's tooling and charter delta: newbin/fm-install-actionlint.shandbin/fm-lint-workflows.shwithtests/fm-lint-workflows.test.sh, workflow linting wired intobin/fm-lint.shand.github/workflows/ci.yml, decision-hold binding support inbin/fm-decision-hold.shplusdocs/decision-hold-lifecycle.md, and updates acrossbin/fm-send.sh,bin/fm-procevent*.sh,bin/fm-ensure-agents-md.sh,bin/fm-backlog-handoff.sh,bin/fm-x-link.sh,.pi/extensions/fm-calm.ts, and the.agents/skills/charter docs.CLAUDE.mdchanges from a symlink to a real two-line@AGENTS.mdpointer, taking upstream's file (our side never modified it); CI enforces that shape..claude/skillsremains a symlink to.agents/skills..agents/skills/stow/SKILL.mddescription — took upstream's sentence verbatim (including itse.g.), reflowed only to fit the YAML block scalar and the repo's one-sentence-per-line style; a follow-up commit corrects an earlier resolution that had dropped thee.g...agents/skills/process-event-sources/SKILL.md— both sides added different paragraphs at the same spot, so both were kept: our Slack adapter paragraph plus upstream's new decision-hold binding rule verbatim.AGENTS.mdsection 2 layout block — our restructured block was kept and upstream's two facts applied into it (the "CLAUDE.md is a real @AGENTS.md pointer" correction and the newstate/decision-bindings/entry with its full semantics), rather than taking upstream's block wholesale, which would have reverted our landed restructure.projects/,data/,state/, orconfig/was touched. Our Slack adapter, the decision-key parsing fix, and the ship-brief real-application verification clause are byte-identical toorigin/mainafter the merge.Findings not fixed
tests/fm-procevent-slack-captain.test.shfails onorigin/mainbefore this merge: the poll leaves its/tmp/fm-slack-captain.*staging directory behind, containing the captured message payload. Pre-existing defect in our own adapter, left out of a merge-only change and reported separately.Risk Assessment
✅ Low: A pure merge with three prose-only conflict resolutions, each verified line-by-line against both parents: no upstream content lost, no our-side functional file altered, no safety rule dropped, and the only cross-side semantic couplings (fm-send --resolve-key refusal text, the new adapter answers seam) still hold.
Testing
Validated the merge as a merge: blob-identity proof that the protected functional work is untouched, an automated completeness sweep showing every upstream-added line landed, per-file line-count arithmetic, and a safety-boundary census on AGENTS.md showing no 'never'/'all'/boundary was lost. The stow resolution now matches upstream's wording exactly under YAML parse. Targeted suites over the merged surfaces (decision-hold lifecycle, decision-key classification, procevent, ensure-agents-md) all pass, the two lint suites pass once upstream's pinned actionlint is installed - confirming they are missing-tool environmental rather than regressions - and CI's repo-invariants checks pass locally, including the new CLAUDE.md real-pointer shape. bin/fm-session-start.sh produces a complete digest on the merged tree. The only failure is tests/fm-procevent-slack-captain.test.sh, which the author disclosed and which is provably pre-existing since that adapter's blobs are unchanged. No visual artifact applies: this change is charter prose, shell scripts, and CI config with no rendered end-user surface; the CLI transcripts are the end-user surface. Temp tooling and the throwaway home were removed and the worktree is clean.
Evidence: Merge verification: preservation, line-count arithmetic, safety-boundary census
Source: Merge verification: preservation, line-count arithmetic, safety-boundary census
Evidence: Upstream-line completeness sweep and the three conflict resolutions
Source: Upstream-line completeness sweep and the three conflict resolutions
Evidence: Targeted test run + CI repo-invariants checks on the merged tree
Source: Targeted test run + CI repo-invariants checks on the merged tree
Evidence: fm-session-start.sh digest rendered on the merged tree (end-user surface)
Source: fm-session-start.sh digest rendered on the merged tree (end-user surface)
Evidence: AGENTS.md boundary census, origin/main vs merged
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-procevent-slack-captain.sh:457- Post-merge, upstream's generic keyed-answer seam (bin/fm-procevent.sh feed_keyed_answers) callsfm-procevent-<adapter>.sh answers <result-file>for any source bound viafm-decision-hold.sh bind. Our Slack captain adapter has noanswerssubcommand, so binding a Slack source would silently feed nothing (the call is guarded by an existing binding and fully silenced, so nothing breaks today). Noting the capability gap only; adding it is out of this merge-only scope.AGENTS.md:72- The reflowed decision-bindings/ entry carries upstream's full semantics (bind-only writer, dropped by unbind or source retirement, docs/decision-hold-lifecycle.md pointer) but omits upstream's incidental "section 13" cross-reference. No safety boundary or rule is lost; recorded for completeness against conflict rule 3.tests/fm-procevent-slack-captain.test.sh- tests/fm-procevent-slack-captain.test.sh fails on the merged tree ("the poll left its staging directory behind: /tmp/fm-slack-captain.*"). Confirmed pre-existing and not a merge regression: bin/fm-procevent-slack-captain.sh and its test are byte-identical blobs to origin/main (6ed3006), so this merge cannot have introduced it. Deliberately not fixed here per the merge-only scope.git rev-parse <rev>:<path>blob-identity check of bin/fm-procevent-slack-captain.sh, tests/fm-procevent-slack-captain.test.sh, bin/fm-brief.sh, bin/fm-classify-lib.sh against origin/main (6ed3006)Automated sweep: every line upstream added between merge-base f1a4af4 and upstream tip 64d61ae checked for presence in the merged tree, across all 42 upstream-touched filesLine-count arithmetic merged == ours + (upstream - base) for all 42 upstream-touched filesAGENTS.md safety-boundary census (never / always / must / '## ' sections / layout-block entries) compared origin/main vs mergedYAML-parsed comparison of .agents/skills/stow/SKILL.mddescriptionbetween upstream 64d61ae and merged HEADbash tests/fm-decision-hold-lifecycle.test.shbash tests/fm-classify-decision-key.test.shbash tests/fm-procevent.test.shbash tests/fm-ensure-agents-md.test.shbash bin/fm-install-actionlint.sh <tmpdir>thenbash tests/fm-lint-workflows.test.shbash tests/fm-lint.test.shwith pinned actionlint on PATHCIinvariantsjob steps run locally: CLAUDE.md non-symlink + canonical @AGENTS.md pointer byte compare,readlink .claude/skills,git ls-files -- data state config projects .no-mistakesFM_HOME=<throwaway> bash bin/fm-session-start.shon the merged tree (full digest captured)bash tests/fm-procevent-slack-captain.test.sh(reproduces the disclosed pre-existing staging-directory leak)docs/scripts.md- docs/scripts.md has no rows for upstream's new bin/fm-lint-workflows.sh and bin/fm-install-actionlint.sh. This gap arrived from upstream (CONTRIBUTING.md documents both), was not created by this merge-only change, and adding rows here would exceed merge scope — worth a follow-up on the script inventory table.✅ **Push** - passed
✅ No issues found.