fix(procevent): apply remote replies during capture - #1831
Conversation
…uction A remote secondmate's reply was captured and announced, but never applied. Nothing dispatched the reply adapter's `handle` on a `procevent remote-reply` wake, and the handling instruction named only the generic acknowledgement, so the wake was retired while everything it carried was dropped: the reply never reached the secondmate's local status mirror, the request it answered kept escalating as a missed report, and the relay - whose registration each capture retires, and which only that same handling re-arms - was left dead until the next session start armed it again. Applying such a result carries no judgement, so it belongs in code. After publishing, the runner now calls `bin/fm-procevent-<adapter>.sh autohandle <source-id> <sequence> <result-file>` and lets the adapter apply and acknowledge its own result, through the same kind of seam that already owns the terminal verdict. It runs strictly after terminal retirement, because a handling adapter re-arms its own next source and retiring afterwards would drop that fresh registration. An adapter with no such command, or one whose pass does not complete, leaves the result unacknowledged and therefore still announced, so a handler receives it exactly as before. Resolving the request was not enough on its own either. An escalation opens a durable keyed decision in the parent status log, and nothing ever closed it, so a request the remote had answered kept surfacing in every later open-decisions fold. The pending-reply library now owns both ends of that decision: it opens one under a per-request key rather than the shared default key, and closes it once the record resolves, appending the closing line only while that exact decision is still open in the fold so it can neither double-close nor clear an unrelated decision that has since taken the same key. The handling instruction still routes a wake to its adapter, now as the idempotent confirmation of what the runner already did rather than as the guarantee. Verified end to end in a throwaway isolated home driving the real armed source, blocking delta reader, runner, and wake queue, with the handler doing only the generic acknowledgement and no part of the ingest stubbed: before, seven failed observations reproducing the incident; after, none. Each half is independently load-bearing - without the runner change the reply never reaches the mirror, without the escalation close the settled request still surfaces as an open decision.
Owed record: E2E reproduction evidence, and one deliberately accepted residualTwo items the captain asked to have on this PR. They are recorded here so they survive 1. End-to-end reproduction, before and afterRun in a throwaway isolated firstmate home (its own The reproduction acts exactly as firstmate acted in the incident: on the published wake it Two scenarios:
Before (base commit
|
Integrates PR #1846's remote status-stream mirroring, which deleted this adapter's line validator: gating the stream made a remote mate's uncorrelated progress lines unrepresentable, and rejecting one line failed a whole delta and wedged the channel. The reserved-key guard this branch had put inside that validator was therefore in the wrong place twice over. It was batch-fatal, and it protected only the remote path while a local mate appends into the very same status stream unchecked. Both writers meet at the open-decision fold, so the rule now lives there: a key like `pending-reply-<id>` names a decision one library raises and is the only writer that ever closes it, and the fold lets a reserved key be opened or closed only by a line whose note speaks that namespace's own vocabulary. Any other line naming the key folds as ordinary status, so it can neither take the key over and block the owner's close forever, nor clear the owner's decision. The rule is generic, so the fold needs no knowledge of any particular owner, and being consumer-side it can never fail a delta or wedge a stream. Also converts the serialized pending-reply lifecycle off subshell function bodies. Sourcing the wake library inside a subshell assigned its globals there, which read as a lost subshell write at every later use in every script that sources this library and failed the repo's own lint gate; main lints clean, so that was this branch's regression rather than a pre-existing one. The globals are declared local and the lock is released explicitly instead, leaving the per-correlation serialization unchanged with its concurrency regressions passing.
…nt main The previous integration commit planted a precomputed tree that had been built against an older main, so it silently reverted work that had landed since: the NUL-safe durable parent binding, the inherited secondmate domain intake procedure, and the network-free session start. Review caught it. This merges current main and carries the corrected tree, in which every file main changed is byte-identical to main and the only paths that differ are this branch's own. The relay fix itself is unchanged.
kunchenguid
changed the title
1 participant
Intent
Fix a correctness bug in firstmate's remote-secondmate reply relay, and integrate it onto a new base. A remote secondmate's reply did not settle the parent's pending-reply records, so completed remote work escalated as false 'pending-reply-missed' alarms and kept re-surfacing in the OPEN DECISIONS fold across sessions. Evidence: secondmate eddies-wallet-e1, procevent seq 25 (2026-08-06), correlation ids e0ee2bec0478140f and e3a050e490cc5abf escalated even though the secondmate had reported the release chain done and gone idle. Systemic across every remote secondmate.
ROOT CAUSE (this SUPERSEDES an earlier framing that said the adapter was missing reconciliation logic; that framing was explicitly withdrawn by the user and must not be reintroduced). The adapter's handle/ingest ALREADY mirrored each reply line into the local status stream and already resolved every corr in the payload. The real defect was that handle was never INVOKED on a 'check: procevent remote-reply ' wake: nothing dispatched to it, and the process-event-sources instruction told the handler to run only the GENERIC acknowledgement. The wake was acked as seen while everything it carried was dropped. The user then sharpened the fix preference: an instruction-only fix cannot be proven to fire, so the code-guaranteed path is the PRIMARY guarantee and the instruction fix must not be the sole one.
MANDATORY DIAGNOSIS GATE the user imposed: before writing any fix, build a standalone repro confirming the diagnosis, and stop and report if it did not reproduce as understood. Done first against the pristine base: 7 failed observations, confirming it exactly. It also exposed a consequence not in the stated diagnosis - each capture is terminal for its registration and only the adapter's own handling re-arms the next one, so with handling never running the relay was left retired after every reply and only revived by session start.
FIX. bin/fm-procevent.sh now calls 'bin/fm-procevent-.sh autohandle ' right after publishing, mirroring the existing adapter-owned 'terminal' seam, so a result whose application carries no judgement is applied and acknowledged by code. It runs STRICTLY AFTER terminal retirement, because a handling adapter re-arms its own next source and retiring afterwards would drop that fresh registration. A missing command, an error, or any other exit is deliberately not a capture failure: the result stays unacknowledged and therefore still announced, so a handler receives it exactly as before, and an adapter without the command (Lavish) needs no change. bin/fm-procevent-remote-reply.sh gained the matching 'autohandle' entry keyed by canonical source id. bin/fm-pending-reply-lib.sh closes the durable keyed decision its own escalation opens, since resolving the record alone left a settled request surfacing in every later fold.
USER DECISIONS ALREADY MADE ON REVIEW FINDINGS, all still in force: (1) legacy unkeyed escalations - the user authorized the guarded containment and asked ONLY for wording accuracy, explicitly rejecting an unconditional default-key force-close because it would clear an unrelated decision that has taken over the shared default key; (2) the user then required RESTORING the guarded legacy close after an auto-fix round had replaced it with an unconditional refusal, keeping the invariant 'a different decision taking over the key cannot be cleared' correct; (3) the escalation lookup was restricted by construction to lines this library itself writes, so a foreign line merely mentioning a request id can never be mistaken for its escalation.
THIS INTEGRATION'S REWORK, sequenced by the user after PR #1846 (remote status-stream mirroring) landed on main. #1846 established that the remote reply channel is a MIRROR of the mate's status stream and deleted the adapter's line validator, because gating the stream made a remote mate's uncorrelated progress lines unrepresentable and one bad line failed a whole delta and wedged the channel. The reserved-key guard this branch had added inside that validator was therefore in the wrong place twice: it was batch-fatal, and it protected only the remote path while a local mate appends into the same status stream unchecked. Per the user's remote-vs-local principle the guard MOVED OUT of the adapter and INTO the shared consumer both writers flow through - the open-decision fold in bin/fm-classify-lib.sh. A key like 'pending-reply-' names a decision one library raises and is the only writer that closes it, so the fold now allows a reserved key to be opened or closed only by a line whose note speaks that namespace's own vocabulary; any other line naming the key folds as ordinary status, so it can neither squat the key and block the owner's close forever nor clear the owner's decision. The rule is generic, so the fold needs no knowledge of any particular owner, and being consumer-side it can never fail a delta or wedge a stream.
Main is integrated as a real merge onto the existing branch head rather than a rebase, deliberately: every prior pipeline fix commit stays present in the ancestry and the branch only moves forward, as this gate requires. An earlier attempt at this integration planted a precomputed tree on the merge commit, which silently reverted changes that had landed on main in the meantime; review caught it and it was redone as an ordinary merge with each conflict resolved individually. The three conflicting paths - the remote reply adapter, its suite, and the remote secondmate document - are ones this branch had already integrated against the status-stream mirror, and main has not touched them since, so their integrated versions carry forward unchanged. The result was then verified rather than assumed: every file main changed since this branch's base is byte-identical to main on this branch, there are no deletions relative to main, and the only paths that differ from main are this branch's own intended ones. In particular the NUL-safe durable parent binding, the inherited secondmate domain intake procedure, and the network-free session start are all preserved intact.
LINT CORRECTION made in this rework: earlier review rounds had put three pending-reply functions in subshell bodies, which assigned the wake library's globals inside a subshell and made every later use of them across the repo read as a lost subshell write, failing the repo's own lint gate (bin/fm-lint.sh). Main lints clean, so this was a regression from this branch, not pre-existing. Those functions are now plain function bodies with the sourced globals declared local and the lock released explicitly instead of from an EXIT trap; the per-correlation serialization is unchanged and both concurrency regressions still pass. bin/fm-lint.sh now exits 0 with no diagnostics.
TWO ITEMS THE USER REQUIRES IN THE PR BODY.
(1) E2E REPRODUCTION EVIDENCE. All of it ran in a throwaway ISOLATED home with its own FM_HOME and state, never a live home, driving the real path end to end: a real armed source, the real blocking cursor-anchored delta reader over the real remote entrypoint, the real process-event runner, and the real wake queue, with NO stubbing of the ingest. The repro deliberately runs only the GENERIC acknowledgement on the wake, so every observation that passes afterwards comes from code rather than handler discipline. Two scenarios: a reply arriving while the request is still open, and a reply arriving after the request already escalated, which is the incident's own shape. BEFORE, on the pristine base: 7 failed observations - the reply never reached the local status mirror, both requests escalated as missed reports for work the remote had completed, the fold kept surfacing them, and the relay was left unarmed. AFTER: 0 failed observations, and it still passes on the integrated branch. Each half of the fix was verified independently load-bearing by reverting one file at a time: without the runner's adapter-owned application the reply never reaches the mirror, and without the escalation close the settled request still surfaces as an open decision.
(2) DELIBERATELY ACCEPTED RESIDUAL, decided by the user: adapter-owned application is hooked into CAPTURE only, NOT into the watcher's reconcile, because reconcile runs synchronously on the watcher's cycle and a remote call there could stall supervision for the whole fleet. The residual is the narrow window where the process crashes after a result is durably captured but before it is applied. In that window the result simply stays unacknowledged, so it remains eligible for re-announcement and returns to the handler on a later drain or restart - the existing announce-until-handled backstop plus the corrected handling instruction. This is deliberate and accepted, not an oversight: the normal path is guaranteed by code and the crash path degrades to the behavior that existed before this change rather than to silent loss.
CONSTRAINTS. The adapter's non-destructive read, cursor continuity, deduplication, and identity guarantees are preserved, as is #1846's contract that no single line can stop or wedge the stream. This is firstmate's own shared tracked material, so firstmate-coding-guidelines applies: the one-owner rule for contracts, colocated tests in tests/ extending the existing runner, tests that exercise behavior through an executable interface and never assert implementation source text, one sentence per line in tracked Markdown, plain dash never an em dash, no agent name as a commit co-author, and shellcheck-clean via bin/fm-lint.sh.
TESTS. tests/fm-remote-reply.test.sh is #1846's suite adapted: it now asserts capture-time application directly, the local-document-storage-failure case obstructs storage BEFORE capture so the automatic application fails for real and covers the unapplied-capture fallback, and the obsolete adapter-side reserved-key case is replaced by one driving a real capture in which a forged reserved-key line mirrors normally and advances the cursor while the fold refuses to let it take the key over, after which a genuine reply resolves the request and clears the fold. tests/fm-wake-drain-open-decisions.test.sh gained a focused regression for the reserved-key rule where that fold contract lives. Both new regressions were verified to fail without their fix. Full changed-test sweep on the integrated tree: 72 scripts, 0 failed; bin/fm-lint.sh and bin/fm-doc-audience-check.sh clean.
What Changed
Risk Assessment
✅ Low: The durable reply-relay fix and follow-up cache/publication safeguards satisfy the stated invariants without a substantiated remaining source defect.
Testing
After correcting rejected manual fixture setups whose temporary homes violated remote-home safety checks, all five targeted suites passed and a fresh isolated real-path E2E run demonstrated capture-time application without handler action, durable request resolution, decision-fold closure, acknowledgement, and relay re-arming; no UI surface was involved, so the reviewer evidence is a direct state and CLI transcript rather than a screenshot.
Evidence: Remote reply relay E2E transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-classify-lib.sh:261- The required rule says any foreign line using a reserved key must fold as ordinary status, but existing.open-decisions-cursorfiles retain open sets computed under the old semantics. Since the cursor format has no fold-version check, an already-consumed foreignblocked [key=pending-reply-…]line continues surfacing indefinitely after upgrade while the whole-file fold correctly ignores it. Version or invalidate the incremental cache when fold semantics change, and cover an upgrade from a pre-existing cursor.🔧 Fix: Version fold cache and gate autohandle on publication
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
git diff 4b6b89d995285accdc77e743f5dd76cfdc12629c..fb4c8f0abf3d4e2aa75eeecaa5e5f84cdf474ba5to identify the affected executable paths and focused regressionstests/fm-procevent.test.shtests/fm-remote-reply.test.shtests/fm-pending-reply.test.shtests/fm-wake-drain-open-decisions.test.shtests/fm-wake-drain-open-decisions-cursor.test.shIsolatedbash -sE2E fixture using the real remote entrypoint, blocking delta reader, process-event runner, remote-reply adapter, pending-reply store, status mirror, and open-decision fold; captured the resulting state inremote-reply-e2e.txtgit status --shortand targeted temporary-directory checks confirmed no testing artifacts remained in the worktree✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.