Conversation
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (3): Last reviewed commit: "test(watch): pin the paused-wait read or..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
Speaking as Kun's firstmate: this is waiting on the author (draft). HEAD Contract-class: restore. Moves the absorb age gate unconditional while keeping only the re-surface throttle declaration-scoped, so a replacement declared wait is absorbed while fresh (status-signal already woke once) then re-surfaces on its own window — restoring VISION:
Security: clean. No Attestation: MATCH (body binds tip; review/test/document completed). Tip NM run Land-eligible: NO (draft). Firstmate-flag: no. Please mark ready for review when you want merge consideration, keep attestation bound to tip, and address or accept the Greptile P2 as you prefer. Do not expect auto-merge while draft. |
The scoped re-surface throttle let a replacement declared wait skip the absorb age gate as well as the throttle gate, so a worker that swapped one declared wait for another was alarmed immediately instead of being absorbed. That crossed two contracts. handle_paused_stale absorbs a freshly declared wait and re-surfaces it once per window, and a replacement is still a freshly declared wait. The status append that declared it has already woken the supervisor through the status-signal path, so firing here as well reports one event twice, on the path whose whole purpose is not to nag. It was also asymmetric: a first declaration was absorbed while fresh, a replacement was not, decided only by whether a throttle marker happened to exist. Apply the absorb age gate unconditionally and keep only the throttle gate scoped to the declaration. A replacement is absorbed while fresh, then re-surfaces once its own age crosses the window instead of serving out the remainder of the previous wait's window. The concern the scope was added for, that a new wait must not inherit the old wait's active throttle, is preserved exactly. This can only remove a wake, never add silence: nothing here suppresses a wait that would otherwise surface. The test pins both halves and fails against either error. Skipping the absorb age gate on a scope mismatch fails as "alarmed instead of absorbed"; removing scope awareness entirely fails as "inherited the old throttle". Both were confirmed by breaking the code deliberately and restoring it. Two details in it are deliberate rather than incidental: it waits for enough completed poll cycles that the absorbed path is actually reached, so an absent wake is evidence about that path rather than about timing, and it advances the seen marker so the status-signal path stays quiet, because that path independently reports the same event and without the isolation the assertion would pass whatever this path did.
The absorb-while-fresh guarantee depends on the order in which handle_paused_stale reads the status file. Workers append to it without a lock, so an append landing between the reads can pair values that describe different declarations, and only one of the two orderings makes that pairing harmless. Cover it deterministically rather than by chance. The case suspends the watcher at the status-read boundary, appends the replacement declaration while it is held there, and releases it, so the interleaving is forced rather than raced. It fails as "alarmed instead of absorbed" if the mtime is read before the declaration, and as "inherited the old throttle" if the declaration scope is dropped entirely. The suspension itself is asserted: if the watcher never reaches that boundary the case fails as "watcher did not reach the concurrent status-read boundary", so it cannot pass without having exercised the interleaving. No production code changes here.
6484b16 to
b8e02ef
Compare
mremond
changed the title
|
The three conditions in the triage above were all met after it ran. That triage was written at 10:20:44Z against head
Three commits and a force-push landed between the triage and now, so its |
|
Speaking as Kun's firstmate: this is a CAPTAIN-DECISION hold — it is waiting on Kun, not on you. There is nothing for you to fix and nothing to push; please do not rebase or force-push it. I have flagged it to him with the specifics below. Thank you for the correction on the earlier stamp — you were right on all three points, and I re-verified each against tip rather than taking them on trust. This restamp supersedes the Head Attestation: MATCH. The body's Required checks on this exact head: green. CI run Security: clean after full diff review. Two files only, no Contract-class: new-default — and this is the whole reason it is now with the captain rather than merged. I read That is a change to a specified default path, not a restoration of a broken one. Why it is genuinely his call and not mine: the effect is that an unconfigured watcher now stays silent for up to VISION verdict (per rule, fetched fresh from
Not merged. Not closed. Not stale (your push and comment are today). No competing PR from me. If Kun accepts the new default, this merges as-is with no further work from you. |
|
Speaking as Kun's firstmate: captain word on this hold — hold for now. Skip is not yes. Not waiting on the author. HEAD still Held with the same standing as #3440: Pi supervision is in active flux; the captain will revisit when it settles. Firstmate will not re-flag this unless the tip product surface changes or Firstmate asks. |
…placement-wait # Conflicts: # bin/fm-watch.sh
mremond
changed the title
|
Following up on the note that this would not be re-flagged "unless the tip product surface changes or The behaviour this PR changes is reachable without any replacement wait. The throttle scope is a Measured on Reproduced on a second task, predicted before it was run: Both figures are recomputable from the status file mtime and the wake record; nothing here needs to Independently, in a separate installation the same day, two That last part is what seems to speak to the stated reason for the hold. The concern was that this The branch has not been rebased and nothing on it has changed since the hold. If any change from this side would help - a rebase, a narrower scope, a different approach, or splitting it - it will be done; it only needs to be known which. |
|
Closing in favour of #4742, which replaces this change and keeps a newly declared wait surfacing immediately. |
2 participants
Intent
Clear the merge conflict that blocks the already-open pull request #3605, and do nothing else.
SCOPE, AND WHY IT IS THIS NARROW. A development pause has been deliberately in force in this fleet since 2026-09-09 to stop saturating the upstream maintainer with open requests. It has been lifted for exactly one purpose: clearing merge conflicts on requests that are ALREADY OPEN. No new deliveries. Six of eleven open requests fell into conflict as the trunk advanced; 3605 is one of them. Clearing the conflict is the entire deliverable.
Therefore the following are deliberately NOT in this change and must not be treated as omissions or as incomplete work: any fix noticed in passing, any stale value refreshed while in the file, any tidier neighbouring line, any generalization, consistency sweep or extra hardening. Those were explicitly ruled out by the captain and are to be reported as follow-up work, never folded in here. The change is intentionally minimal by order.
WHAT WAS DONE. An INCOMING MERGE of main (b182d0f) into the existing published branch fm/fm-absorb-fresh-replacement-wait (head b8e02ef, merge base 3d2a08b). A rebase was explicitly ruled out because it rewrites published history; the captain ordered a rebase for one other branch by name, not this one. The result is signed merge commit 67dab27 with parents b8e02ef and b182d0f.
Exactly one file conflicted: bin/fm-watch.sh. tests/fm-watch-triage.test.sh auto-merged; everything else auto-merged.
HOW THE CONFLICT WAS RESOLVED, and why it is a reconciliation rather than a choice between the two sides. Both sides had changed the same absorb-age gate in resurface_absorbed() for orthogonal reasons:
The resolution keeps BOTH: the gate is hoisted (branch intent) AND uses min_age (trunk intent). That is the only combination that preserves each side - hoisting the constant PAUSE_RESURFACE_SECS instead would have silently reverted the trunk's just-passed-'until' behaviour. All four quadrants (scope match/mismatch x min_age 0/default) were checked and behave as each side intended.
The second hunk keeps the branch's load-bearing read order in handle_paused_stale (declaration and last read BEFORE mtime, so a concurrent status append can pair an old declaration only with a fresher age, never the reverse) and therefore drops the trunk's now-duplicate re-reads of the same two variables below mtime, while keeping all of the trunk's new logic: now, min_age, the away-posture early return, and the declared-'until' branch. The locals line is the union of both sides.
VERIFICATION ALREADY RUN LOCALLY on the merge result: syntax check clean, shellcheck -x clean, and the full tests/fm-watch-triage.test.sh suite passes (226 cases, exit 0). That suite is the direct proof of the resolution because both sides' decisive tests coexist in it after the merge: the branch's test_absorbed_replacement_wait_does_not_inherit_the_old_throttle and the trunk's test_paused_until_that_passed_is_rechecked_before_the_cadence. Choosing either side alone would fail one of them.
DELIVERY CONSTRAINTS. Do NOT open a new pull request: 3605 already exists and stays as it is; this validation publishes onto its existing branch. Do NOT merge - merging belongs to the captain and the upstream maintainer, and our access to this repository is pull-only. The pull request's published body ties its validation attestation to the old head and will not match the new head; that is true of every branch being cleared today and is being settled once for all of them, so it is out of scope here.
A document-stage commit that brings a file's description into line with behaviour this delivery already changed is in scope and should stand - that is the delivery keeping its own description true. Anything that changes BEHAVIOUR beyond the conflict resolution is out of scope.
What Changed
min_ageoverride for expireduntiltimes.Risk Assessment
Testing
The unchanged triage suite and six live tmux scenarios passed after correcting test-driver setup errors. CLI and persisted-state evidence was captured, temporary files removed, and source files left unchanged.
Evidence: Production behavior excerpts
Source: Production behavior excerpts
Evidence: Live watcher transcript
Source: Live watcher transcript
Evidence: Live deadline crossing
Source: Live deadline crossing
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
bin/fm-watch.sh- merge conflict rebasing onto origin/mainbin/fm-watch.sh:1008- A concurrent deadline replacement can resurface twice. After reading ordinary wait A’s signature here, a replacement B can arrive before last_status_line reads it, with B’suntilalready due. The deadline branch combines B’s line with A’s signature, sets min_age=0, and records A:due. After acknowledgement and rearm, unchanged B produces B:due, bypasses the fresh throttle, and emits another stale notification. Taking mtime last does not protect this zero-age path. A follow-up should revalidate that the signature and deadline line describe the same declaration in handle_paused_stale, while retaining mtime-last ordering. The remedy needs authorization because it exceeds the explicit conflict-only scope; keep it out of this delivery.✅ **Test** - passed
✅ No issues found.
git diff b182d0f908b78d08c7ccb8dce3775bdca8c5d657..HEAD -- bin/fm-watch.sh tests/fm-watch-triage.test.shand merge-parent inspection.TMPDIR="$PWD/.phase-test/tmp" FM_HOME="$PWD/.phase-test/home" bin/fm-test-run.sh tests/fm-watch-triage.test.sh— unchanged suite exited 0.python3 ~/.no-mistakes/evidence/01M2FR2HYYDP8NP8HDDX4D1MXS/live-watch.py— real isolated tmux, production watcher, wake drain, acknowledgements, and away-record commands; corrected driver setup and reran successfully.FM_LIVE_SELECT=deadline-crosses-while-watching python3 ~/.no-mistakes/evidence/01M2FR2HYYDP8NP8HDDX4D1MXS/live-watch.py— natural deadline crossing.Captured production CLI output and persisted state; stopped isolated tmux, removed.phase-test, and verified clean Git status.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.