fix(remote): accept autonomous lifecycle reports - #70
Merged
dnth merged 5 commits intoAug 30, 2026
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
dnth
reviewed
Aug 30, 2026
dnth
left a comment
Owner
There was a problem hiding this comment.
Review: approve — clean, scoped, and it closes a real hole
Reviewed against upstream kunchenguid#2618 ("preserve pending replies and defer remote reposts", merged 2026-08-19 @ a693f40) and this fork's current pain (two pending-reply-missed lapses on 2026-08-30).
What this does right:
- Closes a substring false-resolution hole. The old
fm_pending_reply_text_has_corrmatched the correlation token as any substring — a status line containingnotcorr=<hex>in prose could resolve a marked parent request. The new exact-token extraction (corr=<16hex>as a standalone token,[corr=…]:, or(corr=…), canonicalized) matches only genuine status tokens, and the new adversarial tests prove all four corr-like decoy forms (notcorr=,not-corr=,not.corr=,not[corr=]) ingest without resolving. Correlation security is strictly stronger. - Unblocks autonomous lifecycle reports. Requiring
corr=on every line meant ordinaryworking:/paused:/blocked:reports from a secondmate were rejected outright — the exact condition behind our recentpending-reply-missedstorms. Verb allow-list, bounded-printable checks, and one-shot deduplicated append are all preserved. - Fail-closed ordering.
payload_lines_validvalidates the whole delta before any append — no partial-append when a later line is invalid. - Order-preserving mixed-delta semantics. Autonomous + correlated lines in one delta ingest in order, resolve only the exact matching request, advance the cursor, and re-arm. Wrong correlation never resolves. Idempotent replay, continuity-break escalation, and retirement refusal paths all retained and still tested.
- No riders. Six files, all in-scope; docs (
architecture.md,remote-secondmates.md,scripts.md) updated consistently with the code.
Notes for the record:
- vs upstream kunchenguid#2618 (11 files): this is the fork-adapted subset covering the pending-reply correlation contract. The remote-repost-deferral half of kunchenguid#2618 is largely absorbed by this fork's durable steering inbox (#68); if any residue matters it is a small follow-up, not a blocker.
- 15/15 checks green.
This is the form #69 should have been. Recommend merge.
This was referenced Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Repair the remote-secondmate reply adapter so it accepts autonomous material lifecycle status lines permitted by the generated secondmate charter without weakening correlation checks for replies to marked parent requests.
Accept autonomous working, needs-decision, blocked, paused, done, failed, and matching resolved lifecycle reports without corr= as status input. Only a status line carrying the exact correlation token for a marked parent request may resolve that pending reply. A mixed delta with autonomous blocked/resolved lines and a valid correlated done reply must ingest each valid line once in order, resolve only the matching expectation, advance from the captured prefix to the new cursor, acknowledge the captured generation, and re-arm normally.
Preserve bounded printable-line validation, allowed lifecycle verbs, exact correlation-based pending-reply resolution, safe data/*.md document-pointer fetching, idempotent ingest, prefix/cursor continuity, durable acknowledgement, re-arm behavior, and rejection boundaries for unknown verbs, malformed or non-printable input, unsafe or unbounded content, unsafe document pointers, prefix discontinuity, duplicate capture, and mismatched correlation.
Keep the repair limited to the existing adapter and colocated executable regression tests using isolated fixtures. Do not inspect, modify, stop, restart, or send to the live nsm-5950x route or any private fleet record. Do not broaden into FNM discovery, wrapper management, remote routing, or general status-protocol redesign. Do not merge a PR.
Acceptance coverage must include autonomous-only, correlated-only, mixed, idempotent replay, and rejected-input behavior through the real remote-reply adapter interface.
Firstmate-Validation-Generation: 829c5ec05fc152f31d15921e5257d6ce
What Changed
corr=<16hex>tokens when resolving pending parent replies, preventing corr-like text or mismatched tokens from resolving expectations.Risk Assessment
✅ Low: The tightly scoped change preserves adapter durability while separating autonomous lifecycle ingestion from canonical exact-token pending-reply resolution.
Testing
Inspected the target diff and kept the worktree clean. Focused adapter and pending-reply suites passed, covering capture, autonomous and correlated ingestion, exact uppercase correlation resolution, replay idempotency, cursor continuity, acknowledgement/re-arm, document-pointer handling, and rejected lifecycle input. The persisted-state transcript shows autonomous corr-like text remained awaiting_report while the exact uppercase corr field resolved only its matching request and advanced the durable cursor.
Evidence: Manual mixed-delta adapter transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (3) ✅
bin/fm-procevent-remote-reply.sh:250- A valid lifecycle line such asdone: notcorr=0123456789abcdefpasses the new validation, then thecorr=substring is extracted and resolves pending request0123456789abcdef. This violates the required exact-token correlation invariant; enforce token boundaries in the shared pending-reply matcher (which also protects reconciliation) and cover this adapter path.docs/remote-secondmates.md:191- The changed operator contract now permits autonomous lifecycle deltas, but the script index still describes this adapter as relaying only “correlated” deltas. Update that index to describe lifecycle ingestion with exact-correlated pending-reply resolution.🔧 Fix: Enforce exact remote reply correlation boundaries
3 issues (2 errors, 1 warning) still open:
bin/fm-pending-reply-lib.sh:141- The intent requires that “Only a status line carrying the exact correlation token for a marked parent request may resolve that pending reply.” The changed matcher at this hunk accepts any non-word character beforecorr=, sodone: not-corr=<pending-id>is ingested, extracted ascorr=<pending-id>, and resolves the request even though the field isnot-corr, notcorr. Tighten the shared matcher (and adapter extraction) to recognize only permitted correlation-field delimiters, preserving reconciliation protection.bin/fm-pending-reply-lib.sh:141- Lifecycle validation explicitly accepts uppercase hexadecimal correlation values, and the adapter canonicalizes them to lowercase before resolution, but the new exact matcher compares the raw status text case-sensitively. Thusdone [corr=ABCDEF0123456789]: ...is valid and names the stored lowercase request, yet it is appended without resolving it. Preserve token boundaries while matching canonical hexadecimal IDs case-insensitively.docs/scripts.md:80- The script index still describes this adapter as relaying only “correlated” deltas, which is now false because autonomous lifecycle deltas are intentionally ingested. Update the row to distinguish lifecycle-delta ingestion from exact-correlated pending-reply resolution.🔧 Fix: Harden remote reply correlation parsing
1 error still open:
bin/fm-pending-reply-lib.sh:140-done: not[corr=<pending-id>] is not a replypasses lifecycle validation, then this splitter turns the bracketed substring into standalonecorr=<pending-id>. Both adapter ingestion and later reconciliation therefore resolve the pending request, despite no exact correlation field being present. Recognize correlation fields only at admissible status-token boundaries, and add this adapter-path regression.🔧 Fix: Require whole-token remote reply correlations
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-remote-reply.test.shbash tests/fm-pending-reply.test.shManual isolated mixed-delta ingest throughbin/fm-remote-delta-read.shandbin/fm-procevent-remote-reply.sh ingest ios✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.