fix: 144 watcher tails the right task and survives an empty one - #397
Open
s205109 wants to merge 27 commits into
Open
fix: 144 watcher tails the right task and survives an empty one#397s205109 wants to merge 27 commits into
s205109 wants to merge 27 commits into
Conversation
Zero-byte output file: PowerShell unrolls the empty else branch to $null, so Set-TailReaderCheckpoint gets $null. Reproduced end to end. Also measured the inflated running count (39 running, -List shows 20, 4 of those running) and found CLAUDE_CODE_SESSION_ID names the session folder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Draft PR #397 exists, so the Pickup exit condition is met. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running task = output file held open for writing, matched on HResult 0x80070020 only. Marker rule keeps exit code and killed state. Terms pinned in CONTEXT.md Process; spec and ADR pointers in the item. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ADR 0014 said 123's rule was still true; session and checkout preference means newest is now the last resort, so say superseded. Adds the third terminal state and follow-stop to Consequences. Item's out-of-scope no longer excludes -List, since criterion 5 cannot hold with a fixed window. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Also add AllowEmptyCollection to the New-FakeTaskOutput test helper so the new zero-byte fixture can pass -Lines @().
Watch-Record gains the Session/Checkout/OwnSessionId parameters and their two print lines here rather than in a later task, because the session preference test asserts the 'Session: <id> (this session)' line.
The new list-window test binds its result to a local variable that is not named $list: the dot-sourced script leaves a [switch] $List in scope and a PSCustomObject cannot be stored there.
The nine follow child cases pin Test-TaskFileHeldOpen to held. Eight of them run inside Start-Job, where a bare $heldStub reference is empty, so those pass the stub text through -ArgumentList and a param() entry.
Watch-Record's parameters and print lines landed in the session-preference task; this adds the -Index path wiring and the checkout assertions.
The watch-task.ps1 reorg moved lines cited by 144's Background section, ADR 0014, and shipped item 123. All three are pre-fix records, so freeze each file with citation-check:ignore-file rather than repoint the line numbers.
Get-TaskState and Test-TaskFileHeldOpen observed the file at two instants. A writer that appended its marker and closed between them left a stale "no marker" state next to a fresh "not held" probe, and the watcher then reported "stopped without a terminal marker" and dropped the marker line. Probe liveness before the state read at all three sites (discovery, the follow loop's end-of-file check, the settle loop), so the state read is the newer observation. In the settle loop, when the probe says the writer is gone but the state still says running, drain to the end and read the state once more on the now-stable file. Also from review: - add an end-to-end case for the checkout preference (own-checkout task older than a task from another checkout) - tighten the zero-byte checkpoint assertion, which passed vacuously - correct the header text that still described "newest running" selection
From review: a read handle opened with no sharing also trips the sharing violation, so the probe cannot single out a writer. Soften the ADR and the Test-TaskFileHeldOpen docstring to say so; nothing but the harness opens these files, so in practice the violation is still the writer's handle. Also replace the brittle line numbers in 144's acceptance evidence with identifiers, since the code edits kept moving them.
Swapping the settle probe and state read (previous review round) opened a new gap: a replacement run that opens the file between the probe and the read leaves "not held" stale. The reconciliation drained its output and then reported it stopped. It now probes liveness once more, on the drained file, and keeps following when something holds it again. Also from review: the checkout-preference end-to-end case only worked from a worktree, because the script derives its own checkout from its own path. It now builds an isolated repo with a real main and worktree, drops a copy of the script in the worktree, and runs that, so it is checkout-independent.
…ks-the-wrong-sess-03536c53
Test Results62 tests 61 ✅ 3m 29s ⏱️ For more details on these failures, see this check. Results for commit bc29b28. ♻️ This comment has been updated with latest results. |
SummarySummary
Coverageahkflow - 96.6%
AHKFlowApp.API - 84.1%
AHKFlowApp.Application - 98.3%
AHKFlowApp.Domain - 100%
AHKFlowApp.Infrastructure - 100%
AHKFlowApp.Launcher - 71.7%
AHKFlowApp.TestUtilities - 93.6%
AHKFlowApp.UI.Blazor - 91.2%
Per-assembly thresholds: Domain line>=85% br>=70% ; Application line>=85% br>=45% ; Infrastructure line>=70% br>=50% ; API line>=57% br>=50% ; UI.Blazor line>=65% br>=28% |
s205109
changed the title
s205109
marked this pull request as ready for review
September 9, 2026 15:46
Round-3 review found two P2 gaps in the extra drain added last round: a read that failed there broke out with no retry and no deferred-read check, and its liveness re-probe ran after the state read, so a marker written in that gap was still missed and dropped from the output. Removed the separate drain. The settle loop now does up to three extra "quiet" rounds -- nothing read, no marker, no writer -- before it reports a markerless stop. A marker or a replacement writer that appears during one round's probe is seen by the next round's read, through the same catch-up path that already carries the bounded retry and the deferred-read handling. Also from review: Select-WatchTaskRecord dropped its OwnCheckoutPath parameter. The value was never compared -- only its emptiness gated a filter that reads the record's own OwnCheckout flag. The preference now always applies and self-skips when no running task belongs to the checkout.
Get-WatchTaskRecord probed liveness, then read the file text, then stored the earlier probe as Running. A replacement writer that opened in the gap was missed: the record read stopped while a writer held the file, and Select-WatchTaskRecord dropped it, so the caller could follow another session. Re-probe once when the state says running and the first probe said not held, matching the follow loop's reconciliation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Stale task file glossary entry said such a file reads as a Running task. That is the old marker-only rule. The Watcher now checks the write handle, so it reads a held-by-nobody file as stopped. Say so. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
2 participants
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.
Backlog 144. The watcher tailed a task from another session, then stopped with a
parameter-binding error instead of printing output. Either fault alone made the watcher
unusable during a long run.
Closes backlog 144 (moved to
backlog/done/in this PR).The two original defects
A zero-byte output file crashed the tail. PowerShell unrolls an array when it captures a
statement's value, and a zero-length array unrolls to nothing. Three
elsebranches stored$nullinstead of an empty array, andSet-TailReaderCheckpointrejected it. The commaoperator (
return , [byte[]]::new(0)) fixes it. A zero-byte file was also why the watcherchose that file: an empty file has no terminal marker, so the old rule counted it as running,
and a fresh file has the newest write time.
A file nobody was writing counted as running. The old rule called a task running when its
text did not end with
[exited with code N]or[killed]. Nothing checked whether the sessionthat wrote it still existed. Measured on one machine: 489 output files matched this repository,
41 counted as running, 4 were actually being written, and the oldest "running" file had not
changed for 15 days.
What changed
Test-TaskFileHeldOpenasks theoperating system whether a writer holds the output file open. The marker rule stays for the
terminal state (exit code, killed) and for ending the follow loop.
terminal marker". Under the old rule a missing exit code could only mean killed.
CLAUDE_CODE_SESSION_ID),then the checkout the script runs from, then the newest by write time. Each preference is
skipped when it matches no running task. This narrows the half of backlog 123's "newest
across everything" rule that ADR 0014 supersedes; the search still reaches every worktree.
-Listshows every running task, then fills the rest to twenty rows with the neweststopped tasks.
-Indexaddresses that same list, so the count line names only tasks thereader can reach.
Tailing <path>.without a marker no longer keeps the watcher polling a dead file for ever.
Review rounds
Five review rounds after the first implementation:
Get-TaskStateandTest-TaskFileHeldOpenobserved the file at two instants: amarker written between them produced a wrong "stopped without a terminal marker" verdict and
dropped the marker line. Fixed by probing liveness before the state read at all three sites,
with a settle-loop reconciliation on the now-stable file.
file during the first probe and be wrongly reported stopped.
two-checkout git repo.
the same error); header text still described "newest running".
read-failure retry and the deferred-read check, and its final liveness re-probe still ran
after the state read (the marker race again). Consolidated: the separate drain is removed;
the settle loop does up to three quiet confirming rounds through the one catch-up path, so a
late marker or a replacement writer is seen by the next round.
Select-WatchTaskRecordalsodropped its
OwnCheckoutPathparameter — its value was never compared.site.
Get-WatchTaskRecordstored the first liveness probe asRunning, so a replacementwriter that opened before the state read was recorded stopped and
Select-WatchTaskRecorddropped it. It now re-probes once when the state says running and the first probe said not
held. The
CONTEXT.md"Stale task file" entry, which still said such a file "reads as aRunning task", now names that as the old rule.
Verification
Repository tooling, no compiled surface, so the artifact is the PowerShell suite.
pwsh ./scripts/run-powershell-suites.ps1— 56/56 pass, includingWatchTask.Tests.ps1(17 new cases),
CitationFreshness.Tests.ps1,BacklogStaleOpen.Tests.ps1,BacklogPlanPointer.Tests.ps1.dotnet build(Release),dotnet format --verify-no-changes, PowerShellsuites, coverage slice (all per-assembly thresholds met),
git diff --check— all green.scripts/watch-task.ps1andCONTEXT.md, both outsidethe C# gate per
.github/code-paths-filter.yml; its gate was the PowerShell suites, re-rungreen (56/56).
Records
docs/superpowers/) frozen at ship.backlog/done/, all acceptance boxes ticked.🤖 Generated with Claude Code