Repository navigation
Conversation
|
Speaking as Kun's firstmate: VISION per-rule
Contract-class: new-default (own judgment; author also disclosed this). Own-doorbell recognition + delivery-unknown reconcile restore promised paths, but Security review (first-time fork): Diff touches ring/watcher/pending-reply/remote control/docs only — no CI: Approved first-time fork workflows — CI run 38802010833 and Require no-mistakes 38802010597 / 38802245186. Live attestation go 9/11. Closes #7366 / Fixes #6063 (issue #7366 stamped existing-pr). When this head is green + mergeable → Firstmate flag for captain merge-or-leave (new-default). |
…le remote delivery A secondmate's own watcher now queues a check wake when its parent-route steering inbox holds a record past the inbox grace, so a parked mate reads parent instructions without depending on a typed doorbell. The doorbell recognises its own stuck line through the Herdr composer reader and payload equivalence the typing path uses, and every ring decision on every leg is appended to the inbox's bounded .ring-log. The parent's reply tracker no longer counts an idle, never-busy mate as having taken a turn while the request is still unhandled, reads a remote mate's inbox through a new inbox-status leg, and escalates a remote request still unread after five minutes once, with its remedy. Undelivered requests are checked against the mate's inbox once it is reachable: landed is marked delivered, and a recent absent request is resent once under the same correlation, after which the delivery-unknown decision closes.
a55c50d to
978bece
Compare
|
Speaking as Kun's firstmate: Thanks @Moshik21 for the fresh push (head Where it stands:
VISION, rule by rule: one captain: aligns (an unread instruction or unknown delivery shows up honestly). Authority: needs care (the bounded auto-resend is a new default recovery). Scripts/agents: aligns. Restart: aligns (durable undelivered text and inbox reconcile). Delegation: aligns. Vendor: aligns (the Herdr composer reader stays adapter-local). Scope: aligns. This is waiting on you for the serial 2 failure and the remote live evidence. Once there's a green run on the head, it goes to the captain. |
|
Checked the serial 2 failure on 978bece. It is |
|
Speaking as Kun's firstmate: thanks @Moshik21, that's a careful check, and I agree with it. I read
|
|
Live evidence for the four remote scenarios you asked for, on head Setup. A real ssh hop: loopback to a throwaway account on a Linux host over its own OpenSSH server (public-key only), through the real
Stand-ins, stated plainly. Transport faults were injected in the parent's Two observations from the run.
Tests. None of the four scenarios had a regression test at this head. We will add them, using the existing fixtures and |
…answered inbox read A first check that failed because the host was down stamped delivery_checked_epoch before reading, delaying the next check by the full recheck spacing even if the host came back a second later.
Unread escalation, undelivered-record reconcile (landed and resent), the one-time resend with a failed read not delaying the recheck, the resend landing in the remote inbox and its acknowledgement resolving the record, the inbox-status leg, parent_inbox_tick wakes and the ring log. Serial hints for the three extended suites are updated to their measured cost.
…d inbox reads A failed read no longer starts the long recheck spacing (the previous commit) and is also no longer retried on every watcher pass: the next read of that record waits FM_PENDING_REPLY_DELIVERY_READ_RETRY_SECS (60) from delivery_read_failed_epoch. The remote inbox-status round trip is bounded by FM_PENDING_REPLY_INBOX_READ_BUDGET (30 s) with the repo's fm_run_timed, scoped to the tracker's reads. Tests cover the backoff, a remote copy without the inbox-status leg, and the read budget.
The extended reply-tracker suites push the largest of the nine shards past the 1200000 ms packing budget (1204051 ms with refreshed hints), which the coverage guard refuses; rebalancing without lowering a hint cannot absorb the roughly 36 s the new tests add, so the serial remainder is split across ten runners. The new required check context is Behavior portable serial 10.
…s delivery-unknown
Moshik21
changed the title
|
Speaking as Kun's firstmate: thank you @Moshik21, that live write-up is exactly what I was hoping for, and you said plainly which parts were stand-ins. Evidence: on New head CI: run Contract-class: new-default (unchanged: the always-on parent-inbox wake, the pending-reply-unread escalation, and the automatic resend). Once CI is clean, the merge call goes to the captain. That's on our side now, not yours. The live evidence ask is done. |
|
Speaking as Kun's firstmate: thanks again @Moshik21 for the real-SSH evidence and the follow-up fixes. One small ask, the same as on #7525: could you drop 5289c91, the commit that adds a tenth serial shard in |
|
Thanks for checking the evidence and for the clear ask. I will drop 5289c91 (the tenth serial shard in |
…m/fm-upstream-pr-transport
…o the printed resend
 [Medium impact] The update appears safe to merge; both numbered previous findings are fixed. Reviews (6) · Last reviewed commit: "no-mistakes(ci): I made the ci-2 and ci-..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… test bug in this PR's new tests, not in the product code. The test that failed was "the undelivered check honours its spacing, needs stored text, and stops at the age limit" in tests/fm-send-remote-delivery.test.sh. The record CI printed on failure was the earlier "spaced request", not the "textless request" being checked. Cause: the `send_remote_request` helper looked up the correlation id with `pending_record`, which returns the first file `find` lists (`find | head -1`). That order comes from the filesystem. This test is the only one that sends several requests into the same home, so on CI's filesystem the helper sometimes returned an older record's correlation id. Invariant: `send_remote_request` must return the correlation id of the request it just sent. Every other caller of the helper, and of `pending_record`, works on a home with only one record, so this was the only place where the order mattered. Fix (test helper only): `send_remote_request` now lists the records before sending and returns the one record that is new afterwards. The order `find` uses no longer matters. Nothing in the product code changed. Verification: I ran the bounds test with `find` output reversed through a PATH wrapper. The original helper reproduced the exact CI failure ("a record without stored text must not be reconciled", showing the spaced-request record), and the fixed helper passes. The full tests/fm-send-remote-delivery.test.sh suite passes ("all fm-send-remote-delivery tests passed"). The other two standalone suites in that shard (fm-dispatch-resolve, fm-procevent-quota) pass locally and in CI. shellcheck only prints the existing SC1091 notes about sourced files
… failed reconcile close Three review-bot findings on PR kunchenguid#7369, each with one regression test that fails before the fix and passes after it. No new durable state: the fixes use only the inbox's existing .ring-log and the record's existing escalation_closed_epoch field. ci-1 (bin/fm-task-inbox-lib.sh): Claude collapses a pasted burst into a bare `[Pasted text #N]` (optionally followed by a tail). That shape hides its content, so an operator's own paste looks exactly like our collapsed doorbell. The pending-composer ring now treats that shape as our doorbell only when the inbox's .ring-log already holds an earlier line for this record with decision rang or unconfirmed. Otherwise a first ring skips it as other-text and presses no Enter. An exact whole-line match is still accepted. fm_task_inbox_composer_holds now takes the record path (its only caller is the ring). ci-2 (bin/fm-pending-reply-lib.sh): the stored request text is loaded byte-exactly (sentinel inside the substitution), so the one automatic resend carries the original body, trailing newlines included. ci-3 (bin/fm-pending-reply-lib.sh): - Landed path: the open delivery-unknown decision is closed first, and only then is the record marked delivered. A failed close leaves the record undelivered, so the next check retries it. - Resend path: the close goes through _fm_pending_reply_close_escalation_locked, which now records escalation_closed_epoch for a delivered record whose delivery_reconcile is landed or resent. The tick retries that close while escalated_epoch is set and escalation_closed_epoch is empty. - A later unread or missed escalation is excluded from that retry and clears escalation_closed_epoch, so the final resolve close still closes the later decision. - Records of --resolve-key answers keep no stored text, are never reconciled, and so never reach this close (unchanged).
… failed reconcile close Three review-bot findings on PR kunchenguid#7369, each with one regression test that fails before the fix and passes after it. No new durable state: the fixes use only the inbox's existing .ring-log and the record's existing escalation_closed_epoch field. ci-1 (bin/fm-task-inbox-lib.sh): Claude collapses a pasted burst into a bare `[Pasted text #N]` (optionally followed by a tail). That shape hides its content, so an operator's own paste looks exactly like our collapsed doorbell. The pending-composer ring now treats that shape as our doorbell only when the inbox's .ring-log already holds an earlier line for this record with decision rang or unconfirmed. Otherwise a first ring skips it as other-text and presses no Enter. An exact whole-line match is still accepted. fm_task_inbox_composer_holds now takes the record path (its only caller is the ring). ci-2 (bin/fm-pending-reply-lib.sh): the stored request text is loaded byte-exactly (sentinel inside the substitution), so the one automatic resend carries the original body, trailing newlines included. ci-3 (bin/fm-pending-reply-lib.sh): - Landed path: the open delivery-unknown decision is closed first, and only then is the record marked delivered. A failed close leaves the record undelivered, so the next check retries it. - Resend path: the close goes through _fm_pending_reply_close_escalation_locked, which now records escalation_closed_epoch for a delivered record whose delivery_reconcile is landed or resent. The tick retries that close while escalated_epoch is set and escalation_closed_epoch is empty. - A later unread or missed escalation is excluded from that retry and clears escalation_closed_epoch, so the final resolve close still closes the later decision. - Records of --resolve-key answers keep no stored text, are never reconciled, and so never reach this close (unchanged). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…after a recent typed ring A bare [Pasted text #N] is also what an operator's own paste looks like, so the ring now treats it as our doorbell only when the inbox's .ring-log shows an earlier ring of this record that typed the doorbell within the last inbox grace plus three watcher polls (the next re-ring of a doorbell whose Enter was lost falls inside it). Older rings no longer vouch for a later paste. No new durable state; the window comes from the existing grace and FM_POLL constants. The test covers a recent ring, an old ring and none.
3d78fad to
c35c410
Compare
Moshik21
changed the title
|
Behavior portable parallel 1 failed once in tests/fm-backend-t3code.test.sh (expected 500, got 499): a 1 ms clock race in bin/fm-t3-mcp.mjs (the first wait is deadline minus now). This PR does not touch it, and a re-run passes. |
# Conflicts: # tests/fm-send-remote-delivery.test.sh
…each. Only the pending-reply suite is verified. The `shellcheck` run produced no result, and several other checks were not run. Nothing is committed. **ci-2, lock and re-read.** The change is in `bin/fm-pending-reply-lib.sh`, in `fm_pending_reply_reconcile_undelivered`. The invariant is that a record settled by the reply handler is never overwritten by an older view of it. - After the unlocked inbox read, the function now takes the per-correlation lock. - It re-checks that the record is still undelivered, using a new shared helper `_fm_pending_reply_undelivered_open`. The same helper replaces the old entry checks at the top of the function. - All the new field writes now happen under that lock: `delivery_read_failed_epoch`, `delivery_checked_epoch`, the landed path, `delivery_reconcile=absent`, and `delivery_resend_epoch` / `delivery_reconcile=resent`. - The lock is released before `fm-send.sh` runs, since `fm-send.sh` takes the lock itself. The existing post-send block still takes the lock again. **ci-3, retirement cleanup.** In `bin/fm-teardown.sh`, `pending_replies_cleanup_for_task` now also removes `./.undelivered-text-$base`. I updated its comment to match. **Tests.** - `tests/fm-pending-reply.test.sh` has a new `test_undelivered_reconcile_never_reopens_a_record_settled_during_the_read`. It overrides the inbox read so it settles the record mid-read, then asserts the phase stays `resolved` and no undelivered fields are written. It passes, and the whole suite passes with it. - The ci-3 test is not in `tests/fm-pending-reply.test.sh`, which departs from your instruction. `pending_replies_cleanup_for_task` lives in `fm-teardown.sh`, which can't be sourced without running a retirement. The existing teardown tests already live in `tests/fm-secondmate-lifecycle-e2e.test.sh`, so I added the check there. It saves undelivered text for the leftover record with `fm_pending_reply_store_undelivered_text`, then asserts the file is gone after `fm-teardown.sh` runs. Say if you'd rather have it elsewhere. **Verification status.** - Passed: `tests/fm-pending-reply.test.sh`, the whole file. - No result: the `shellcheck` run on the four edited files was killed at its background time limit. A rerun with a longer timeout is needed, probably one file at a time. - Not run: `tests/fm-secondmate-lifecycle-e2e.test.sh` (which holds the ci-3 test), `bin/fm-test-run.sh --check-coverage`, and the check that the new lock test fails on the old code. I wrote the test to fail there, but did not confirm it. - ci-1 needs no change. - Only the new test in `tests/fm-pending-reply.test.sh` was added, and I changed no Markdown
|
Speaking as Kun's firstmate: thank you @Moshik21. The branch now carries no workflow change, so the shard ask from 10-27 is done. Thanks for handling it the way you said you would. What I checked (head Live evidence (this is waiting on you, whenever you have time): this head's run drove 1 of 4 scenarios live. Your real-ssh evidence was on
Contract-class: new-default, unchanged (an always-on parent-inbox wake, the unread escalation and the automatic resend). Once the evidence is in, running the full suite is on us (the |
|
Note 🤖 Codex responding on behalf of Moshik21 Live results on Environment: loopback SSH to a separate Unix account, uid 1001; real remote entrypoint, remote job worker, seeded remote secondmate, inbox and Herdr pane Local scratch paths below are represented by env -i HOME="$OPERATOR" PATH=/usr/local/bin:/usr/bin:/bin \
FM_HOME="$PARENT" FM_SSH_BIN="$RUN/ssh-wrap.sh" "$@"Setup used the production commands: ev "$CODE/bin/fm-remote-home-seed.sh" ev7369b ev7369b \
/home/ev7369b/code-pr /home/ev7369b/fm-home --no-projects
ev "$CODE/bin/fm-on.sh" ev7369b fm-remote-doctor.sh
ev "$CODE/bin/fm-spawn.sh" ev7369b --secondmate --harness claude --effort lowThe successful seed printed 1. Unread escalation: pass. I left an unrelated unsubmitted draft in the real composer, then sent: ssh -F "$SSH_CONFIG" ev7369b \
'herdr pane send-text w1:p2 "EV7369 unrelated draft; leave unsubmitted" --session fm-remote'
ev "$CODE/bin/fm-send.sh" ev7369b \
'EV7369 S1: report only after this unread-delivery observation.'Send exited 0 with: Correlation ev bash -c '. "$1/bin/fm-pending-reply-lib.sh"; fm_pending_reply_tick "$FM_HOME/state"' _ "$CODE"Measured output from the driver: Exactly one status line was appended: No clock or observation was mocked. There was no continuous parent watcher; the direct production tracker calls are the passes reported here. 2. Never-landed request, one byte-exact resend: pass. The parent's The actual request argument was generated as: payload = b'EV7369 S2: preserve byte-exact request; tab=\t; line two follows.\nsecond line\n\n'
(run/'drop-all').touch()
try:
res = subprocess.run(ev + [str(code/'bin/fm-send.sh'), 'ev7369b', payload.decode()], capture_output=True)
finally:
(run/'drop-all').unlink()Both initial transport attempts were refused. I checked absence through the real hop, then ran the same production tracker command twice: ev "$CODE/bin/fm-on.sh" ev7369b fm-remote-secondmate-control.sh inbox-status \
ev7369b f643d5d0d2d716dcDriver output: The body extracted from remote 3. Bare placeholder guard: pass with operator-paste setup. A typed ring alone leaves the full literal doorbell on these versions. The first attempt therefore did not exercise the bare-placeholder branch. I then made one attempt using a genuine large operator paste after a fresh real typed ring. I did not type or fabricate placeholder text. To retain a real typed ring, I temporarily suppressed CR input on the actual Claude PTY, re-sent the existing ssh -F "$SSH_CONFIG" ev7369b 'stty -F /proc/567770/fd/0 -g'
ssh -F "$SSH_CONFIG" ev7369b 'stty -F /proc/567770/fd/0 igncr'
# stdin = production fm_task_inbox_body output for 003.msg, corr=2a922ba3398b3a7c
ev "$CODE/bin/fm-on.sh" --stdin ev7369b \
fm-remote-secondmate-control.sh send-stdin ev7369b
ssh -F "$SSH_CONFIG" ev7369b \
'stty -F /proc/567770/fd/0 0:5:f00bf:a30:3:1c:7f:15:4:0:1:0:11:13:1a:0:12:f:17:16:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0'
ssh -F "$SSH_CONFIG" ev7369b \
'herdr pane send-keys w1:p2 ctrl+u ctrl+u ctrl+u --session fm-remote'The real ring log gained: After clearing that literal line, the positive paste argument was generated and sent as follows ( operator_paste = typed + ' EV7369-operator-paste ' + ('x' * 8192)
remote('herdr pane send-text w1:p2 ' + shlex.quote(operator_paste) + ' --session fm-remote')Claude itself rendered the 8,386-byte paste as bare For the negative control I made one corresponding real single-line paste: negative_paste = 'EV7369 negative operator paste ' + ('y' * 8192)
remote('herdr pane send-text w1:p2 ' + shlex.quote(negative_paste) + ' --session fm-remote')Claude rendered it as bare ev "$CODE/bin/fm-send.sh" ev7369b \
'EV7369 S3 negative: no previous typed ring for this record.'
# stdin = production fm_task_inbox_body output for the new 004.msg
ev "$CODE/bin/fm-on.sh" --stdin ev7369b \
fm-remote-secondmate-control.sh send-stdin ev7369bExact resulting observations and ring entries: The positive case proves submission of the genuine operator-paste placeholder following a recent typed ring; the negative case proves that the same shape is preserved when that record has no prior typed ring. The setup does not show a typed ring naturally collapsing into a placeholder on these versions. No PR code was changed. |
|
Speaking as Kun's firstmate: thank you @Moshik21, this is thorough evidence. The production Read against head
Every other gate still holds on this head: no-mistakes binds it with no skips, the quick checks are green, Greptile's threads are resolved, and it merges cleanly. Contract-class: new-default, unchanged. The full suite is next. I add a few |
kunchenguid
added
the
full-ci
|
Speaking as Kun's firstmate: thanks @Moshik21. As promised, this was first in line. Head |
Intent
Reconcile and escalate unread remote secondmate requests: the parent's reply tracker reads the remote inbox and escalates an unread request once, a parked secondmate is woken on unread parent instructions, failed inbox reads are bounded, unconfirmed resolve-key answers stay on the printed resend, and a bare Herdr paste placeholder is accepted as the doorbell only after a recent typed ring. The branch was brought up to date by merging current main; the earlier tenth-shard CI commit is not part of the net diff because change 7838 already adds that shard.
What Changed
bin/fm-pending-reply-lib.sh) now reads a remote secondmate's inbox through a new read-onlyinbox-statusleg inbin/fm-remote-secondmate-control.sh. A request still unhandled afterFM_PENDING_REPLY_UNREAD_SECSescalates once aspending-reply-unread(added tobin/fm-classify-lib.sh), and an idle mate with an unread record no longer counts as having taken a turn. Delivery-unknown remote records are reconciled against the inbox: landed records are marked delivered, and recent absent ones get one same-correlation resend using the request text thatbin/fm-send.shnow stores.--resolve-keyanswers store no text and keep to the printed resend. Inbox reads are bounded by a read budget and a short retry spacing after a failed read. The new tunables are documented indocs/configuration.mdanddocs/remote-secondmates.md.parent_inbox_tickinbin/fm-watch.sh) queues onecheck: parent instruction waitingwake per oldest record once a parent-route inbox record is older thanFM_TASK_INBOX_GRACE_SECS. A parked mate therefore reads parent instructions without depending on a typed doorbell.bin/fm-task-inbox-lib.shappends every ring decision to a bounded per-inbox.ring-log. It also accepts a bare Herdr[Pasted text #N]composer placeholder as the mate's own doorbell, but only when the.ring-logshows a recent earlier typed ring of that record. Tests are added or extended intests/fm-pending-reply.test.sh,tests/fm-send-remote-delivery.test.shandtests/fm-task-inbox.test.sh, and the serial-shard weights inbin/fm-test-run.share updated for them.mainmerged in. It carries no CI workflow change and no tenth serial shard, becausemainalready has that shard from change 7838.Risk Assessment
Testing
I ran three test suites that execute the real scripts against fixtures.
tests/fm-pending-reply.test.shpassed.tests/fm-task-inbox.test.shpassed 52/52 once GNUtouchwas on PATH, because a branch test usestouch -d. Both runs used fixtures, not a live Herdr or remote host, so they are not recorded as live.tests/fm-send-remote-delivery.test.shfailed at its first test,test_remote_large_steer_uses_stdin, which is main's test and not this branch's. The same failure happens at base commit 05196e3, so it predates this change. The cause is thatfm_run_external_timeoutinbin/fm-timeout-lib.shbackgrounds the command whentimeoutis on PATH, so the steer's stdin becomes/dev/nullandsend-stdinrejects the message as empty. This matches upstream issue 7968. With the perl timeout fallback the same test timed out at 30s, and bash-timeout mode failed as well. Docker is not running, so I couldn't try Linux. The branch's roughly 20 new remote reconcile tests did not run here. I confirmed live that theinbox-statususage line is present with the header lines 2 to 25 intact, that the merged tree has no.githubchange, and that the only change tobin/fm-test-run.shis the three serial-test hint numbers. Temp copies and shims are removed and the worktree is clean.touch -d, so GNU touch was shimmed). No live pass was esta…fm_run_external_timeoutinbin/fm-timeout-lib.shbackgrounds the command), so the…git diff 05196e30 HEAD -- .githubis empty;bin/fm-test-run.shdiffers only in the three serial-test hint numbersEvidence: pending-reply test log
Source: pending-reply test log
Evidence: task-inbox test log with GNU touch (52 ok, no failures)
Source: task-inbox test log with GNU touch (52 ok, no failures)
Evidence: task-inbox plain macOS log (fails on touch -d)
Source: task-inbox plain macOS log (fails on touch -d)
Evidence: remote-delivery log on HEAD (fails at the first test)
Source: remote-delivery log on HEAD (fails at the first test)
Evidence: remote-delivery log at base 05196e3 (identical failure)
Source: remote-delivery log at base 05196e30 (identical failure)
Evidence: remote-control usage output
Source: remote-control usage output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ No issues found.
bin/fm-timeout-lib.sh- On macOS the remote steer path loses its message whentimeoutorgtimeoutis on PATH, becausefm_run_external_timeoutbackgrounds the command and its stdin becomes/dev/null. This makestest_remote_large_steer_uses_stdinfail at the base commit too, which blocks the rest of tests/fm-send-remote-delivery.test.sh locally. It is not caused by this change; it matches upstream issue 7968. The branch's remote reconcile tests were not run here and depend on Linux CI.touch -d, so GNU touch was shimmed). No live pass was esta…fm_run_external_timeoutinbin/fm-timeout-lib.shbackgrounds the command), so the…git diff 05196e30 HEAD -- .githubis empty;bin/fm-test-run.shdiffers only in the three serial-test hint numbersbash tests/fm-pending-reply.test.sh(passed)bash tests/fm-task-inbox.test.sh(plain macOS run failed on GNU-onlytouch -d; rerun with GNU touch shim passed 52/52)bash tests/fm-send-remote-delivery.test.shon HEAD, with GNU shims, with the bash timeout override and with the perl timeout fallback (all failed at the first test)bash tests/fm-send-remote-delivery.test.shon a git archive of base 05196e30 (fails identically)FM_HOME=<tmp> bash bin/fm-remote-secondmate-control.sh(usage lists inbox-status, rc 2)git diff 05196e30 HEAD -- .github bin/fm-test-run.sh✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.