Skip to content

test(calm-pi): use Pi 1.0.1 getToolRenderers dependency key - #6516

Closed
Thoughts-One wants to merge 0 commit into
kunchenguid:mainfrom
Thoughts-One:fm/up-pi-renderer-key
Closed

Thoughts-One wants to merge 0 commit into
kunchenguid:mainfrom
Thoughts-One:fm/up-pi-renderer-key

Conversation

@Thoughts-One

@Thoughts-One Thoughts-One commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Intent

Approval words: "d4 proven api approved." Scope as relayed: D4 authorizes only the proven Pi API mismatch upstream fix, following CONTRIBUTING.md. The other 15 timing-cap tests remain outside scope.

Context (proven on the fork): tests/fm-calm-pi-extension.test.sh builds Pi's internal createToolHtmlRenderer by hand with the dependency key getToolDefinition. Pi 1.0.1 (published 2026-10-03, CI installs @earendil-works/pi-coding-agent unpinned) renamed it to getToolRenderers, so the test fails on 1.0.1 and passes on 1.0.0. With only the key renamed it passes 15/15 on 1.0.1 and the calm extension itself is unaffected. The fork fix is a two-line rename (Thoughts-One/firstmate PR 49).

What Changed

  • In tests/fm-calm-pi-extension.test.sh, the two hand-built createToolHtmlRenderer calls now pass the dependency key getToolRenderers instead of getToolDefinition. Pi 1.0.1 renamed this key.
  • The lookup function and the other arguments (theme, cwd) are unchanged. The change touches only the test file, not the calm extension.

Risk Assessment

✅ Low: The change is a two-line, test-only key rename that matches the authorized scope. The calm extension is unaffected, and the only open item is an unverified sibling test that may need the same rename.

Testing

I ran the real fm-calm Pi extension test (renderer, lifecycle and tmux TUI checks) against a freshly installed Pi 1.0.1. The pre-change test fails there after 7 ok lines with 'grep disappeared from /export calm.html HTML while calm mode was on'. With the fix, all 15 checks pass and the exit code is 0. The fixed test fails on the installed Pi 1.0.0 with the same error. Pi 1.0.0 reads only getToolDefinition, and 1.0.1 reads only getToolRenderers. The diff touches only the two renamed keys. The other timing-cap tests were left alone, as the recorded decision requires. The temporary Pi install is removed. The worktree is clean. Logs are in the evidence directory.

  • Live validation: ✅ go - 3 of 3 scenarios driven live against the product
Scenario Result Live Evidence
Run the calm Pi extension test on Pi 1.0.1 with the key renamed; all checks pass ✅ pass live after-pi-1.0.1.log: exit 0, 15 ok lines, no failures
Reproduce the original failure: run the pre-change test on Pi 1.0.1 ✅ pass live before-pi-1.0.1.log: exit 1 after 7 ok lines, 'grep disappeared from /export calm.html HTML while calm mode was on'
Adversarial: run the fixed test on the installed Pi 1.0.0 to see if the rename breaks the older version ✅ pass live after-pi-1.0.0.log: exit 1, same error as the pre-change test on 1.0.1. The test no longer works on 1.0.0. The change is still acceptable, because the stated intent is to track the latest Pi that CI i…
Evidence: Before fix on Pi 1.0.1 (fails after 7 ok)

Source: Before fix on Pi 1.0.1 (fails after 7 ok)

ok - Pi calm resolves its persistent home independently of Pi's launch directory
ok - Pi calm compatibility evidence never rejects a Pi version for being newer than 0.82.0, and still fails closed on a missing or malformed version
ok - a missing collapsed-thinking presentation API degrades only that Calm adapter with a clear skip reason, while the rest of Calm still registers
ok - missing Pi presentation class exports reach the independent adapter degradation path
ok - Calm hides queued Firstmate rows only on a session that can keep them, keeps hidden ones out of the editor on Escape, delivers them once in order, and leaves unsupported sessions and Calm off stock
ok - Calm registers none of its 7 built-in tool wrappers at load while config/calm is off, and all 7 synchronously at load while config/calm is on
ok - Calm's first same-session /calm activation claims every uncontested built-in, leaves a foreign bash tool fully intact and callable, warns prominently and logs the contested name, and only rows constructed before that activation - the documented bound - fail to retroactively collapse
not ok - Pi calm renderer and lifecycle contract failed: file:///tmp/fm-calm-pi-extension.SBUqIN/renderer/[eval1]:546
      throw new Error(`${name} disappeared from ${command} HTML while calm mode was on`);
            ^

Error: grep disappeared from /export calm.html HTML while calm mode was on
    at assertStockHtmlRendering (file:///tmp/fm-calm-pi-extension.SBUqIN/renderer/[eval1]:546:13)
    at file:///tmp/fm-calm-pi-extension.SBUqIN/renderer/[eval1]:553:7
    at process.processTicksAndRejections (node:internal/process/task_queues:104:5)

Node.js v24.18.0
Evidence: After fix on Pi 1.0.1 (15/15 ok)

Source: After fix on Pi 1.0.1 (15/15 ok)

ok - Pi calm resolves its persistent home independently of Pi's launch directory
ok - Pi calm compatibility evidence never rejects a Pi version for being newer than 0.82.0, and still fails closed on a missing or malformed version
ok - a missing collapsed-thinking presentation API degrades only that Calm adapter with a clear skip reason, while the rest of Calm still registers
ok - missing Pi presentation class exports reach the independent adapter degradation path
ok - Calm hides queued Firstmate rows only on a session that can keep them, keeps hidden ones out of the editor on Escape, delivers them once in order, and leaves unsupported sessions and Calm off stock
ok - Calm registers none of its 7 built-in tool wrappers at load while config/calm is off, and all 7 synchronously at load while config/calm is on
ok - Calm's first same-session /calm activation claims every uncontested built-in, leaves a foreign bash tool fully intact and callable, warns prominently and logs the contested name, and only rows constructed before that activation - the documented bound - fail to retroactively collapse
ok - Pi calm centralizes transcript visibility, preserves execution/export data, keeps Pi's stock working row visible while no run is active, and persists its choice across session starts
ok - Pi calm on collapses mid-turn assistant working notes to zero height while Calm off keeps them, leaves streaming, truncated-final, and genuine final replies untouched, never mutates the messages, ignores every /calm argument, and restores a legacy persisted max as ordinary Calm on
ok - Pi operational follow-up E2E processes exact user-role notifications once while Calm hides current and adjacent rows, Calm off and absent render them, and restart preserves semantics
ok - Pi 1.0.0 with Calm on hides and retains queued Firstmate input through Escape, delivers it once, and leaves Calm off stock
ok - Pi Calm native /skill:ahoy geometry keeps every collapsed thinking and tool block at zero height while preserving expansion, history, restart, and Calm-off rendering
ok - Pi Calm working ship keeps its centered two-row asymmetric Unicode boat inside a deterministic long-wave trough, paints all water standard blue and the whole boat standard yellow with balanced resets, keeps ANSI-stripped width exact, reverses cleanly at both edges and every width, clamps visible and hidden resizes, falls back deterministically when narrow, freezes and resumes across settle/start without hidden-time jumps or duplicate timers, resets only on a fresh session, and leaves Calm-off visibility untouched
ok - the rendered-export-DOM guard renders in one pass, retries a bounded number of Chrome start-up failures, and reports the Chrome binary, Chrome version, Pi version, exit status, and Chrome diagnostic when every attempt fails
ok - Pi calm native E2E replaces the stock working row with a moving, resize-clamped working ship that freezes and resumes across two working periods in one Pi session, clears on abort, keeps captain turns visible, hides exact operational user rows without changing persistence, restores stock rendering Calm-off, survives restart, and preserves export plus Ctrl+O behavior
Evidence: After fix on Pi 1.0.0 (fails after 7 ok; the rename is not backward compatible)

Source: After fix on Pi 1.0.0 (fails after 7 ok; the rename is not backward compatible)

ok - Pi calm resolves its persistent home independently of Pi's launch directory
ok - Pi calm compatibility evidence never rejects a Pi version for being newer than 0.82.0, and still fails closed on a missing or malformed version
ok - a missing collapsed-thinking presentation API degrades only that Calm adapter with a clear skip reason, while the rest of Calm still registers
ok - missing Pi presentation class exports reach the independent adapter degradation path
ok - Calm hides queued Firstmate rows only on a session that can keep them, keeps hidden ones out of the editor on Escape, delivers them once in order, and leaves unsupported sessions and Calm off stock
ok - Calm registers none of its 7 built-in tool wrappers at load while config/calm is off, and all 7 synchronously at load while config/calm is on
ok - Calm's first same-session /calm activation claims every uncontested built-in, leaves a foreign bash tool fully intact and callable, warns prominently and logs the contested name, and only rows constructed before that activation - the documented bound - fail to retroactively collapse
not ok - Pi calm renderer and lifecycle contract failed: file:///tmp/fm-calm-pi-extension.leLLQw/renderer/[eval1]:546
      throw new Error(`${name} disappeared from ${command} HTML while calm mode was on`);
            ^

Error: grep disappeared from /export calm.html HTML while calm mode was on
    at assertStockHtmlRendering (file:///tmp/fm-calm-pi-extension.leLLQw/renderer/[eval1]:546:13)
    at file:///tmp/fm-calm-pi-extension.leLLQw/renderer/[eval1]:553:7
    at process.processTicksAndRejections (node:internal/process/task_queues:104:5)

Node.js v24.18.0
- Outcome: ⚠️ 1 info across 1 run (2m11s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 warning
  • ⚠️ tests/fm-pi-branch-extension.test.sh:5369 - The same renamed dependency key getToolDefinition is still passed to Pi's createToolHtmlRenderer at tests/fm-pi-branch-extension.test.sh:5369 and :5370 (the stockHtml and actualHtml renderers). I could not confirm 1.0.1 behavior because the installed Pi is 1.0.0. If 1.0.1 only reads getToolRenderers, those calls throw a TypeError at renderCall and fail the same way. The intent authorizes only the proven calm-test mismatch and puts other tests out of scope, so this is left as a scope decision. The remedy is the same two-line rename in that file.
⚠️ **Test** - 1 info
  • ℹ️ tests/fm-calm-pi-extension.test.sh:1666 - The renamed key is not backward compatible. Pi 1.0.0 (the version installed on this machine) reads only getToolDefinition in export-html/tool-renderer.js, and Pi 1.0.1 reads only getToolRenderers. With the fix, tests/fm-calm-pi-extension.test.sh now fails on 1.0.0 (7 ok, then 'grep disappeared from /export calm.html HTML'). This follows the stated intent, because CI installs Pi unpinned and so gets the latest version. Anyone running the test against a 1.0.0 install must upgrade Pi.
  • Live validation: ✅ go - 3 of 3 scenarios driven live against the product
Scenario Result Live Evidence
Run the calm Pi extension test on Pi 1.0.1 with the key renamed; all checks pass ✅ pass live after-pi-1.0.1.log: exit 0, 15 ok lines, no failures
Reproduce the original failure: run the pre-change test on Pi 1.0.1 ✅ pass live before-pi-1.0.1.log: exit 1 after 7 ok lines, 'grep disappeared from /export calm.html HTML while calm mode was on'
Adversarial: run the fixed test on the installed Pi 1.0.0 to see if the rename breaks the older version ✅ pass live after-pi-1.0.0.log: exit 1, same error as the pre-change test on 1.0.1. The test no longer works on 1.0.0. The change is still acceptable, because the stated intent is to track the latest Pi that CI i…
  • Installed Pi 1.0.1 into a throwaway npm prefix under /tmp and ran FM_PI_PACKAGE_DIR=<that install> bash tests/fm-calm-pi-extension.test.sh on the target commit
  • Ran the pre-change test file from base commit 1f3e769 against the same Pi 1.0.1 install
  • Ran the fixed test file against the installed Pi 1.0.0
  • Compared the dependency key each Pi version's export-html/tool-renderer.js reads (getToolDefinition in 1.0.0, getToolRenderers in 1.0.1)
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@greptile-apps

greptile-apps Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Low risk] Updates test code to match a dependency API change.

The PR appears safe to merge for its stated Pi 1.0.1 test target.

Reviews (1) · Last reviewed commit: "test(calm-pi): use Pi 1.0.1 getToolRende..."

mremond added a commit to mremond/firstmate that referenced this pull request Oct 3, 2026
… loop counter in the failure diagnostic. Source-aware lint, all 34 inbox tests, and git diff --check pass. Behavior portable serial 6 reproduces identically on base 1f3e769 and target 78156b8 with Pi 1.0.1: an unrelated renderer API change breaks the unchanged Calm test. No Calm changes made; that failure is addressed separately by kunchenguid#6516. Logs retained in scratchpad-ci/
@Thoughts-One
Thoughts-One force-pushed the fm/up-pi-renderer-key branch from e72c23c to fede619 Compare October 4, 2026 04:14
kunchenguid pushed a commit that referenced this pull request Oct 4, 2026
* Escalate inbox instructions stuck behind a busy worker

Count consecutive busy-deferred due doorbells durably and escalate at the configured bound without typing into the worker pane.

Fixes #6445

* fix(review): Fix inbox escalation deduplication and busy streak resets

* fix(review): Preserve busy inbox escalations through daemon supervision

* fix(document): Correct busy-inbox escalation documentation

* fix(ci): Fixed SC2034 in tests/fm-task-inbox.test.sh by including the loop counter in the failure diagnostic. Source-aware lint, all 34 inbox tests, and git diff --check pass. Behavior portable serial 6 reproduces identically on base 1f3e769 and target 78156b8 with Pi 1.0.1: an unrelated renderer API change breaks the unchanged Calm test. No Calm changes made; that failure is addressed separately by #6516. Logs retained in scratchpad-ci/

* fix(ci): Fixed ci-2, ci-3, and ci-4: successor failures surface, reset alerts deduplicate, and oversized busy limits fall back to two. Passed 47 inbox tests, 7 focused daemon checks, all 13 mutation checks, lint, documentation checks, and diff checks. Evidence: scratchpad-ci-selected/summary.json. ci-1 remains unchanged and unwaived. Fresh live Herdr proof remains with the outer driver
yehezkieled added a commit to yehezkieled/firstmate that referenced this pull request Oct 5, 2026
* fix: reclaim orphaned watcher arms on the next park (kunchenguid#6335)

* fix(bin): take over the watcher cycle a main-only pass-through leaves

An attended main-only pass-through leaves a successor watcher cycle
running through main's handling turn. The session's next park attached
to that cycle instead of owning it, so the successor's arm, orphaned by
its host's exit, kept owning the watcher while the new park's arm polled
it twice a second until the next close or the park boundary, hours later
in a quiet second mate. A second-mate restart hit this every time, since
its persist request is a main-only close.

The host now records the successor it leaves for main, and the next
host's first cycle runs bin/fm-watch-arm.sh --take-over on it: when that
arm still owns the healthy watcher, the new arm stops it, reports a
reason the cycle delivered first, and otherwise owns a fresh cycle. The
stop's own downtime publication is undone over an acknowledged episode
when no wake was appended in between, so the handover wakes nobody.

* no-mistakes(review): Keep left-arm record until the orphaned arm is gone

* no-mistakes(review): Relinquish successor arm only after durably recording it

* no-mistakes(review): Relinquish successor only after its record reads back

* no-mistakes(document): Clarify successor takeover guarantees and authoritative documentation

* no-mistakes(ci): Fixed ci-1: acknowledgement restore now requires the exact taken-over arm/watcher ledger row with signal=TERM, awaited within a short bound. Otherwise takeover proceeds without erasing downtime. Added a self-exit regression confirmed failing before the fix and passing afterward; ordinary takeover tests and the full watcher-arm suite pass. Updated Generation reuse documentation. Syntax, diff checks, and ShellCheck pass with existing SC1091/SC2034 warnings excluded. ci-2 remains unchanged

* no-mistakes(document): Clarify watcher take-over recovery and restart limits

* fix(bin): restore downtime on supervision-host hand-back when the successor already closed (kunchenguid#6355)

* fix(bin): restore supervision host hand-back continuity

* no-mistakes(review): Scope host hand-back failure fallback to lost pending:handling

* no-mistakes(review): Remove stray scratch test copy tests/.tmp-rest.test.sh

* no-mistakes(test): Initialise successor globals so early hand-back survives set -u

* no-mistakes(document): Document host hand-back downtime failure and Claude lost-handback notice

* no-mistakes(ci): I fixed the Greptile finding. The rule that must hold: when the supervision host hands back an actionable wake, its rewake is refused, and no watcher is healthy, the hand-back still has to reach main as a delivered notice. That must be true whether the recovery marker is `pending:handling` or `announced:handling`. Only one place applies this check: the lost hand-back fallback in `bin/fm-claude-stop-autoarm.sh`. **Fix:** that check now accepts both `pending:handling:*` and `announced:handling:*` tokens (a one-line change). Nothing else in the fallback changed: - Refusals on any other marker, such as an already acknowledged one, still exit 0 silently and open no failure episode. - The notice is still sent once per episode, and repeats are recorded as `failed-suppressed`. **Tests:** - `tests/fm-claude-stop-autoarm.test.sh`: the lost hand-back test now runs as a shared helper with two variants, one writing a `pending:handling` marker and a new one writing `announced:handling` (`test_host_lost_announced_handback_notifies_once_per_episode`). - `tests/fm-supervision-host.test.sh`: the end-to-end test where downtime restoration fails is now a shared helper too, with a new `announced` variant (`test_claude_stop_hook_notifies_when_closed_announced_successor_downtime_restore_fails`). It moves the handling episode to `announced` before the host hands back. Without the fix the hook would exit 0 here; the test requires exit 2, `outcome=failed` and a delivered failure notice. **Verification (all under nice -n 10):** - The full `tests/fm-claude-stop-autoarm.test.sh` suite passed (rc=0), including both lost hand-back variants and the benign-refusal test. - In `tests/fm-supervision-host.test.sh` I ran only the four hand-back test functions, all passing (rc=0). The suite can't run single functions, so I used a temporary copy with a trimmed test list and deleted it afterwards; `git status` shows only the 3 intended files changed. - shellcheck is clean on all three changed files. I did not run `bin/fm-lint.sh`. - I did not run the new tests against the unfixed code; the claim that they fail without the fix comes from reading the old check

* fix: reduce remote-job polling process churn (kunchenguid#6363)

* perf: cut remote-job idle process creation in the three hot loops

Post-update host measurement still attributes most idle churn to three
per-sample loops: result-consumer state reads and date calls, the delta
reader's capture/hash pass on every poll, and the lane preemption scan's
per-field pipelines. This drops each to its minimum without touching the
contracts around them.

* fm_remote_job_read_state gains an optional result-variable form backed
  by fm_remote_job_read_line, a builtin-only bounded record read (regular
  non-symlink file, byte bound, one newline-terminated line, tolerated
  unterminated tail, no carriage returns). fm_remote_job_wait samples
  state and the SECONDS clock with no per-sample children; one date call
  converts the epoch deadline once.
* fm-remote-delta-read stats the log each poll and re-runs the bounded
  capture and hashing only when size, mtime, ctime, inode, or device
  change. The snapshot's own stat writes the comparison key, so a log
  that moves between the gate and the capture is never read as stable.
* worker_preempting_waiter_exists reads state, home, and the staged argv
  head with builtins only. The now-unused worker_job_command goes away.

The bounded reads use -d '' -n, which behaves identically on the macOS
stock bash 3.2 and current bash; -N does not exist on 3.2. Tests cover
the malformed-record corpus, delta identity gating, fork-free lane
scanning through counting PATH shims, and same-home versus cross-home
preemption. No signal traps or sleep contracts change.

* no-mistakes(review): Restore subsecond delta keys and byte-bounded builtin record reads

* no-mistakes(document): Clarify delta snapshot caching and coarse-timestamp fallback

* no-mistakes(lint): Scope UTF-8 regression locales to individual function calls

* no-mistakes(ci): Fixed both lint failures by applying the documented production-library analysis boundary at the two affected test imports. Runtime behavior is unchanged; the library remains independently linted. Canonical full-analysis lint passed for the library and both suites, as did bash syntax checks and git diff --check

* fix(bin): recognize titled Claude top rules and preserve grey slash commands (kunchenguid#5963)

* fix(composer): read a titled Claude top rule as the composer's edge

A named Claude Code session draws its title into the composer's top rule.
The strict separator predicate rejected that row, so the closing rule read
as a lower unmatched separator and an idle, empty composer classified
unknown on every cursorless backend, refusing fm-send, exit, and relaunch.

Spare a bare agent-glyph row sandwiched between a width-proven titled rule
and the screen's only unmatched separator directly below it. The strict
separator predicate, dead-shell rule, and blank-row posture are unchanged.

Fixes kunchenguid#5601
Fixes kunchenguid#5558

* no-mistakes(test): Keep Claude's grey slash command in Herdr payload proof

* no-mistakes(test): Make missing-herdr version check hermetic to installed herdr

* fix: prevent Pi trust prompts in seeded secondmate homes (kunchenguid#6387)

* fix: pre-approve Pi trust for seeded secondmate homes

Unattended first launches of Firstmate-seeded Pi secondmate homes stalled on
"Trust project folder?" until Enter. Probe --approve like --tui-mode and pass
it only for --secondmate when help advertises it (.fm-secondmate-home signal),
leaving ordinary workers and older Pi unchanged.

* no-mistakes(document): Consolidate Pi seeded-home trust documentation ownership

* no-mistakes(ci): Fixed Lint 1’s unused polling counter. Diagnosed Behavior portable serial 4 as a pre-existing delta-reader test clock race; replaced timing-dependent rewrite and deletion with deterministic executable-boundary synchronization. ShellCheck, Bash syntax checks, and git diff --check passed. Delta-reader tests passed three consecutive runs; all three live Pi trust cases passed. Production behavior unchanged

* fix(bin): retry ShellCheck roots that hit the memory ceiling without --external-sources (kunchenguid#6443)

* fix(lint): retry memory-bound roots without external sources

* no-mistakes(review): Make fallback tests portable and correct source-following telemetry

* no-mistakes(review): Remove committed parity fixtures and use disposable test roots

* no-mistakes(document): Document ShellCheck memory fallback and telemetry

* no-mistakes(review): Cover bounded and unbounded fallback RSS behavior

* no-mistakes(document): Correct stale lint fallback documentation

* no-mistakes(document): Correct stale lint test documentation

* no-mistakes(ci): The memory fallback (the retry without --external-sources) now gets only the time left in its root's original deadline, so it can no longer outlast the CI job. Invariant: one root's first attempt plus its fallback must fit inside a single FM_LINT_ROOT_SECONDS deadline, plus the cleanup grace. Only one site started a new deadline: the fallback call in fm_lint_run_root. The deadline is the only budget involved, because the memory limit already applies to each process separately. Changes in bin/fm-lint.sh: - fm_lint_exec_root now takes a <seconds> argument instead of always reading FM_LINT_INTERNAL_ROOT_SECS. - The first attempt passes the full deadline. - The fallback passes floor((start + deadline - now) / 1000) seconds. - When bounds are enforced and less than 1 second is left, no retry starts. fm_exec_timed rejects 0 seconds, so the retry cannot run with no time. The root keeps reason=memory, and the shard output says "no time left in its Ns deadline to retry without it". - Unbounded local runs have no deadline and behave as before. - The header comment now describes the shared deadline. Changes in tests/fm-lint.test.sh: a new test, test_memory_fallback_spends_only_the_remaining_root_deadline, runs only on hosts that can enforce bounds. It uses a 6 s deadline and 1 s grace. - Case 1: the first attempt runs 3 s and then fails with memory status 251. The test asserts one fallback ran, reported reason=timeout, and the root's recorded duration is under 7000 ms. - Case 2: the first attempt runs 5.2 s. The test asserts no fallback starts, the skip is explained, and the sidecar records memory with source-following 1. Verification: - Full `nice -n 10 bash tests/fm-lint.test.sh` passed, including the new test, in about 5 minutes. - Case 1 run against the HEAD script: the root took 9168 ms, so the under-7000 ms check fails before the fix. - `bin/fm-lint.sh bin/fm-lint.sh tests/fm-lint.test.sh` reported no findings. - The CI workflow is unchanged, so the Test step still runs only tests/fm-lint.test.sh with nice -n 10 and the 12 GiB ShellCheck limit

* fix(pi): restore watcher continuity across successor gaps and make extension log opt-in (kunchenguid#5489)

* Fix Pi watcher successor-gap confirmations and add extension log

Accept an already-acknowledged handling confirmation as a no-op when the
generation matches, confirm the restoration's own recovery token with a
superseded (not rejected) outcome on generation mismatch, retire an arm on
confirm failure only when the failed token names that exact pid, and record
restore attempts, readiness timeouts, and confirm results in the bounded
state/.watch-extension.log. Regression tests: already-acked no-op plus
mismatch/dead-pid/lock-mismatch rejections and the manual-restart churn
contract in fm-watch-arm.test.sh, and a mid-restore marker advance with no
rejection appendix in fm-pi-watch-extension.test.sh.

* Treat a dead arm child as an empty slot so repair and retry recover

startArm and scheduleRetry answered unchanged while holding a ChildProcess
whose OS process was already gone but whose close had not fired, so neither
the repair tool nor the retry timer started anything until that close fired.
Gate slot occupancy on a liveness check (exit/signal codes plus pid probe)
and start a fresh arm instead, with a regression test driving the repair
tool against a dead-but-unclosed child.

* no-mistakes(document): Document new Pi extension log knob

* no-mistakes(review): Fix confirm-failure retire token match, add distinct-pid test

* no-mistakes(document): Clarify retire guard needs pid and generation

* Make the Pi extension diagnostic log opt-in and default-off

Only a positive FM_WATCH_EXTENSION_LOG_KEEP_LINES enables
state/.watch-extension.log. Unset, empty, non-numeric, zero, and
negative values disable logging entirely, so the default run writes
nothing and never creates the file. The shared positiveInteger
fallback semantics stay untouched for the retry and timeout knobs.
docs/configuration.md owns the knob contract and
docs/watcher-continuity.md points at it. Tests: the superseded-delivery
case runs opted in, and a new case proves unset, zero, and non-numeric
values create no log file while delivery still succeeds.

* no-mistakes(document): Qualify extension-log coverage bullet as opt-in

* no-mistakes(ci): The two reported checks (CI run 36372002913, Require no-mistakes run 36372069208) show conclusion action_required with 0 jobs and no logs because this is a fork PR (RibatTRW/firstmate) and GitHub is holding the workflow runs awaiting maintainer approval; that gate is external to the code and needs a maintainer to approve the runs. While verifying the change locally I found a real defect the approved CI run would hit: the PR's new test test_handling_delivered_rejects_a_superseded_generation failed deterministically. Invariant violated: reopen-announced (a non-successor/manual arm start) mints a fresh recovery generation only when the durable wake queue holds unrecovered work; an announced episode with an empty queue must be left untouched so idle arm starts never churn generations (the [ -s queue ] guard from kunchenguid#4819, relied on by bin/fm-watch.sh:2413 and covered by the append-reopens and announcement-bound sibling tests). The test called reopen with an empty queue and expected churn, so the fix establishes the queued-work precondition (append one wake, re-announce, re-read the generation) before asserting the reopen mints and the old confirmation mismatches. Test-only change, 14 lines in tests/fm-watch-arm.test.sh. Verified: fm-watch-arm.test.sh 25/25 ok on two consecutive runs, fm-pi-watch-extension.test.sh 55/55 ok, and shellcheck reports only one pre-existing warning outside the edited region

* no-mistakes(document): Restore blank line in watcher-continuity docs

* Route superseded Pi deliveries like confirmed ones and cover the retire guard

A superseded handling confirmation now falls through to the normal delivery
path, so an accepting supervision branch owns the wake instead of main.
Scope the watcher-continuity token-pinned confirmation and narrowed retire
rule to Pi, since omp and OpenCode still confirm against the current
successor. Add tests that fail when the retire guard, the scheduled-retry
gate, or the deferred-close gate is reverted, relabel the churned-generation
characterization test, and use a reaped pid for the dead-pid rejection.

* fix(pi): hide duplicate assistant finals from hidden processing retries (kunchenguid#5863)

* fix(pi): silence unacknowledged processing retry replies

Suppress autonomous processing prose before persistence and during streaming while retaining tool calls, signed reasoning, and retryable outcomes. Restore ordinary output after acknowledgement or a user message.

Fixes kunchenguid#4954

* no-mistakes(review): Silence only processing retries, keep first presentation visible

* fix(pi): preserve differing processing retry replies

* test(pi): accept Pi 1.0.1's renamed HTML export renderer lookup (kunchenguid#6530)

Pi 1.0.1's createToolHtmlRenderer reads getToolRenderers and ignores
getToolDefinition. Calm /export still includes stock grep HTML; the
fixture has to pass the lookup key the installed Pi actually reads.

* fix(bin): tolerate transient quota read failures (kunchenguid#6490)

* fix(procevent-quota): tolerate consecutive slow quota-axi reads

The quota allowance poll treated any quota_json failure as terminal, so one
slow quota-axi --json (measured max ~29s under a 48s derived bound) shut the
watch down until someone re-armed it, and the detail always said
"missing/incompatible". Tolerate three consecutive failed or timed-out reads
before going terminal, reset the streak on any good read, and report a
timeout distinctly from a missing or incompatible tool. Each timed poll runs
exactly one bounded --version and one bounded --json: validate the captured
version text through fm_quota_axi_version_compatible rather than launching a
second probe, and describe a mixed failure streak by count plus last cause.

* no-mistakes(document): Document quota polling failure tolerance

* no-mistakes(ci): Fixed ci-2 and ci-3. Permanent quota read failures (rc 2 missing, rc 3 incompatible) now report on the first poll, while transient rc 1/4 failures retain the existing three-failure retry behavior. The missing-binary test now uses an isolated PATH without quota-axi and asserts both permanent failures stop at condition_polls: 1. Verification passed: tests/fm-procevent-quota.test.sh, canonical fast lint for both changed files, bash syntax checks, and git diff --check

* no-mistakes(review): Classify untimed quota version failures as transient

* no-mistakes(document): Clarify quota polling failure budget

* fix(bin): clarify scratch guidance and dirty teardown refusals (kunchenguid#6505)

* fix(teardown): clarify scratch guidance and dirty worktree refusals

Keep ship proof material outside the task worktree and distinguish untracked-only leftovers from tracked edits without changing cleanup guards.

Fixes kunchenguid#6319

* fix(ci): Fixed ci-3 only. Both promotion outputs now replace the scout restriction and require external scratch storage and a clean worktree before done. Verification: 27 delivery tests passed, five mutations caught, restored test passed, pinned ShellCheck and syntax/whitespace checks passed. ci-1, ci-2, and ci-4 remain untouched

* fix(bin): escalate inbox instructions blocked by busy workers (kunchenguid#6518)

* Escalate inbox instructions stuck behind a busy worker

Count consecutive busy-deferred due doorbells durably and escalate at the configured bound without typing into the worker pane.

Fixes kunchenguid#6445

* fix(review): Fix inbox escalation deduplication and busy streak resets

* fix(review): Preserve busy inbox escalations through daemon supervision

* fix(document): Correct busy-inbox escalation documentation

* fix(ci): Fixed SC2034 in tests/fm-task-inbox.test.sh by including the loop counter in the failure diagnostic. Source-aware lint, all 34 inbox tests, and git diff --check pass. Behavior portable serial 6 reproduces identically on base 1f3e769 and target 78156b8 with Pi 1.0.1: an unrelated renderer API change breaks the unchanged Calm test. No Calm changes made; that failure is addressed separately by kunchenguid#6516. Logs retained in scratchpad-ci/

* fix(ci): Fixed ci-2, ci-3, and ci-4: successor failures surface, reset alerts deduplicate, and oversized busy limits fall back to two. Passed 47 inbox tests, 7 focused daemon checks, all 13 mutation checks, lint, documentation checks, and diff checks. Evidence: scratchpad-ci-selected/summary.json. ci-1 remains unchanged and unwaived. Fresh live Herdr proof remains with the outer driver

* fix: prevent unnecessary remote worker turnover (kunchenguid#6431)

* fix: prevent healthy remote job worker turnover

* no-mistakes(review): Serialize full LaunchAgent repair and verify launchd-tracked owners

* no-mistakes(review): Let launchd-tracked unpublished spawns start before reloading

* no-mistakes(document): Document remote worker heartbeat and serialized LaunchAgent recovery

* no-mistakes(ci): Full CI log showed the idle-worker regression exceeded its outdated command budget (82 versus 80) after independent heartbeat ownership checks were added. Raised the budget to 120 while retaining the separate busy-poll sleep limit. Remote-job and LaunchAgent executable tests passed, as did bash syntax validation and git diff --check. No production behavior changed

* no-mistakes(ci): Fixed missing readiness recovery under verified live ownership, preserving the serving PID and private file mode. Added executable regressions for deletion during a blocked sweep and stale readiness diagnostics without LaunchAgent reload. Deletion regression failed before the fix. Both remote-job test suites, bash syntax validation, and git diff --check passed

* no-mistakes(ci): Full CI log identified a flaky ownership-loss test racing an already-authorized heartbeat refresh. Replaced backdating and a fixed sleep with bounded observation of readiness expiry through the public probe. Production behavior unchanged. Remote-job and LaunchAgent executable suites passed; bash syntax validation and git diff --check passed

* fix: wait for launchd bootout cleanup

* no-mistakes(document): Document remote worker recovery and read-only turnover verification

* no-mistakes(review): Publish worker identity before lock owner records

* no-mistakes(document): Document worker identity publication safety invariant

* no-mistakes(ci): Fixed ci-1: replacement workers discard predecessor readiness before publishing identity and roll back identity if lock-owner recording fails. Added executable regressions reproducing both defects. LaunchAgent and orphan-reap suites, shell syntax checks, and git diff --check passed. No live service state was modified

* no-mistakes(ci): Restored lock-owner-before-identity publication and removed the identity rollback and reordering-only tests. Retained an executable regression proving predecessor readiness is rejected until replacement startup completes. LaunchAgent and orphan-reap suites, shell syntax checks, and git diff --check passed. Other changes remain intact; the retained-identity interrupted-repair edge remains out of scope. No live service state was modified

* fix: speed up remote-job sequence claim cleanup (kunchenguid#6575)

* fix(remote-job): reap expired seq claims with one directory walk

The hourly claim sweep forked uname+stat per .seq-claims entry and blocked
serving for ~85s at ~17k dirs. Delete expired empty claim dirs with a single
find -exec rmdir batch and cache the host uname for remaining mtime reads.

* no-mistakes(review): Restore original path mtime helper and drop uname cache

* no-mistakes(test): Restore claim retention eligibility; focused retention and serving tests pass

* no-mistakes(document): Document single-walk claim cleanup and regression entrypoints

* no-mistakes(ci): Fixed Lint 2’s reproduced SC1091 by adding the tests/lib.sh ShellCheck source directive to the retention test. Runtime behavior is unchanged. ShellCheck passed for both new claim tests; Bash syntax, the retention behavior test, and git diff --check passed

* fix(tests): keep fixture registries out of git worktree roots

A TMPDIR pointed at a repository root placed live .fm-test-* registries
beside tracked files, and a concurrent git add during the claim-walk CI
fix round committed three of them. Route registries and fixture roots
through a TMPDIR that refuses git worktree roots, remove the stray files,
and pin the escape with a behavioral cleanup test.

* no-mistakes(review): Preserve whole-second claim expiry in single-walk sweep

* no-mistakes(document): Correct temporary-directory resolution documentation

* no-mistakes(ci): Fixed ci-3 by changing only the stale, fresh, and read-only orphan fixture paths in tests/fm-test-fixture-cleanup.test.sh to use $FM_TEST_TMPDIR. All seven tests passed both normally and with TMPDIR set to the worktree root. Shell syntax and git diff --check passed

* no-mistakes(document): Align merged docs with upstream hand-back and policy owners

---------

Co-authored-by: Kun Chen <3233006+kunchenguid@users.noreply.github.com>
Co-authored-by: Tiago <tiagop@hey.com>
Co-authored-by: RibatTRW <aydinhrrs@gmail.com>
Co-authored-by: Ian Brown <742554+zestysoft@users.noreply.github.com>
Co-authored-by: Joseph Kim <jokim1@gmail.com>
Co-authored-by: Mickaël Rémond <mremond@process-one.net>
Co-authored-by: Hezki <hezki@users.noreply.github.com>
max-metaplanet pushed a commit to max-metaplanet/firstmate that referenced this pull request Oct 6, 2026
…nguid#6518)

* Escalate inbox instructions stuck behind a busy worker

Count consecutive busy-deferred due doorbells durably and escalate at the configured bound without typing into the worker pane.

Fixes kunchenguid#6445

* fix(review): Fix inbox escalation deduplication and busy streak resets

* fix(review): Preserve busy inbox escalations through daemon supervision

* fix(document): Correct busy-inbox escalation documentation

* fix(ci): Fixed SC2034 in tests/fm-task-inbox.test.sh by including the loop counter in the failure diagnostic. Source-aware lint, all 34 inbox tests, and git diff --check pass. Behavior portable serial 6 reproduces identically on base 1f3e769 and target 78156b8 with Pi 1.0.1: an unrelated renderer API change breaks the unchanged Calm test. No Calm changes made; that failure is addressed separately by kunchenguid#6516. Logs retained in scratchpad-ci/

* fix(ci): Fixed ci-2, ci-3, and ci-4: successor failures surface, reset alerts deduplicate, and oversized busy limits fall back to two. Passed 47 inbox tests, 7 focused daemon checks, all 13 mutation checks, lint, documentation checks, and diff checks. Evidence: scratchpad-ci-selected/summary.json. ci-1 remains unchanged and unwaived. Fresh live Herdr proof remains with the outer driver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant