Skip to content

ums: a background watcher reports failure as silence by default - #1012

Merged
d-morrison merged 2 commits into
mainfrom
ums/2026-08-01-silent-watcher
Aug 1, 2026
Merged

ums: a background watcher reports failure as silence by default#1012
d-morrison merged 2 commits into
mainfrom
ums/2026-08-01-silent-watcher

Conversation

@d-morrison

@d-morrison d-morrison commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Merge-time UMS top-up after #1009, recording the one learning from that session that the entry itself was too busy to cover.

The gap

fail-fast already covers a by-hand check whose failure path and pass path print the same thing (grep ... || echo "none"), and the fan-out version where every worker fails identically into /dev/null.

Neither covers a background watcher, and the watcher case is worse for a reason specific to its medium: its output channel is a notification, and you deliberately stop reading it. So its silence is not an oversight, it is the thing you asked for --- quiet means nothing to report, which is exactly what a healthy long-running job looks like. The failure is not merely unnoticed; it is reassuring.

The concrete shape is a bounded poll loop that emits only on the happy path and falls out of its range without a word.

Why it is worth an entry rather than a shrug

I wrote this bug twice in one hour, the second time immediately after telling the user about the first. Reading the Monitor tool's own coverage guidance ("if this process crashed right now, would my filter emit anything?") did not prevent it, which is the argument for a corpus entry over a mental note.

The first instance hid a red validate on #1009 --- caught only by querying the PR directly, not by anything the watcher did.

Verification

  • 57 added lines: 0 banned glyphs, 0 multi-sentence lines, 0 over 80 characters, 0 lines beginning #<digit>.
  • markdownlint-cli2@0.22.1: 446 files, 0 errors.
  • Dupe-check, corrected. The original wording here claimed no existing entry covers watcher silence. That overstated a narrow grep (silence is not success|watcher), which cannot match prose saying "the monitor's silence". Review found a related entry I had missed: memories/claude-code.md records a pipe-stage route to the same silence, where grep -q/-l/-c upstream starves a later stage of content. Different cause, identical symptom, and both surfaced only by running the query by hand. Not a duplicate --- the fixes differ --- so the new section now cross-links it rather than restating it.

fail-fast covers a by-hand check whose failure and pass paths print the
same thing. A watcher is worse: its output channel is a notification, so
its silence is what you asked for, and a bounded poll loop that prints
only when the condition is met exits quietly when it never is.

Adds the terminal-else and emit-on-every-terminal-state fixes, and
records that reading the Monitor tool's own coverage guidance did not
prevent writing the bug twice in one hour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMmB6db4xpbuqcqaQKArZb
Copilot AI review requested due to automatic review settings August 1, 2026 05:19
@d-morrison

Copy link
Copy Markdown
Collaborator Author

Working on this --- paws off until I'm done.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Code Review — #1012

PR: ums: a background watcher reports failure as silence by default
Scope: Adds one new subsection (57 lines) to shared/principles/fail-fast.md, documenting a failure mode where background poll/watcher loops report failure as silence. Documentation-only change to an AI-agent instruction corpus; no application code.

I ran a full parallel review pass (2 CLAUDE.md-compliance agents + 2 bug-scan agents), then independently validated the one finding that surfaced, with a fifth adversarial pass. All four initial agents converged on the same issue independently, and the validator confirmed it with fresh eyes.

Findings

1. [High] New section breaks an existing cross-reference in unchanged text
shared/principles/fail-fast.md:236 (new heading), affecting :293-327 (pre-existing, unchanged text)

The new ### A background watcher reports failure as silence by default section is inserted directly between the existing ### A fan-out makes this worse... (H3, ends ~line 234) and its existing child subsection #### A zero-shaped summary can be sound... (H4, line 293). Before this PR, that H4 immediately followed the fan-out H3, and its own text uses positional references that specifically point at the fan-out section's rule:

  • L295: "The rule above has a false-positive direction, and it lands on exactly the tools that already comply with it." — this describes the fan-out section's scanned N of M / "distrust a sweep that reports zero" remedy (L213-228), not anything the new watcher section says.
  • L298-299: "which is the remedy this section asks for" — again the fan-out section's "print your scope" instrument, not the watcher section's "terminal else + widen the filter" fixes.
  • L323: "the original rule stands unchanged" — same fan-out rule, now 57 lines and a whole H3 section away.

The insertion also re-parents the H4 under the new H3 in the document's heading hierarchy, structurally implying the "zero-shaped summary" caveat belongs to the watcher rule when it's actually entirely about scan-count instruments (the worked markdownlint-cli2 example has nothing to do with polling loops).

Fix: Move the new section to after the "zero-shaped summary" H4 — i.e., insert it immediately before ### The pattern itself is the other half... (line 340) instead. This preserves the fan-out → zero-shaped-summary adjacency the existing cross-references depend on, and the new section's own opening line ("The two cases above are checks you read the output of") still reads correctly there.

Minor notes (non-blocking, not posted as formal findings)

  • The PR body's "Dupe-checked... no existing entry covers watcher silence" claim is phrased as a categorical conclusion from what looks like a narrow watcher-string grep, which sits close to what shared/workflow/grep-is-not-coverage.md cautions against. One reviewer found a related-but-distinct existing entry (memories/claude-code.md, on Monitor loops timing out) — not a true duplicate, but the claim slightly overstates the search performed. This is in the PR description, not the shipped file content.
  • The quoted attribution at L270-271 ("the Monitor tool's own documentation gives...") could not be independently verified from this environment (Monitor tool docs weren't reachable). No evidence it's wrong — flagging only as worth a spot-check by someone with access.

Verified clean

Banned-glyph scan, multi-sentence-line scan, 80-char line length, markdownlint structure, Do/Don't pairing, dated case citation, and the shell example's correctness (matches the prose and the cited 25-minute timeout) all checked out exactly as the PR body claims.

Per the invocation arguments, no --comment flag was provided, so no GitHub comments were posted — this is a terminal-only report.

Verdict

Needs minor changes. One confirmed structural defect (broken cross-reference from the insertion point) needs a one-line fix — relocate the new section below the "zero-shaped summary" subsection — before this is ready to merge. Everything else checks out.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

💰 Cost: $21.7487 (review) — run

The insertion sat between the fan-out section and its own H4 caveat,
breaking cross-references that depend on that adjacency and re-parenting
the H4 under the new H3 -- implying the scan-count caveat was about
polling loops.

Also drops a count from the opening line ("the two cases above" would now
read as three) and cross-links the pipe-stage route to the same silence
already recorded in memories/claude-code.md, which my dupe-check grep
missed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HMmB6db4xpbuqcqaQKArZb
Copilot AI review requested due to automatic review settings August 1, 2026 05:38
@d-morrison

Copy link
Copy Markdown
Collaborator Author

Round 1: one finding Addressed, two minor notes handled, in bb5628e

Structural defect --- Addressed, exactly as proposed. My section had been inserted between the fan-out section and its own #### caveat, so it broke the cross-references that depend on that adjacency and re-parented the H4 under my new H3 --- structurally implying the scan-count caveat was about polling loops. Moved to immediately before ### The pattern itself is the other half. Heading order is now fan-out -> zero-shaped-summary -> watcher -> pattern.

Worth naming, because it is the second time tonight: this is the same class of finding as ai-config#1009's third one. Both are defects in unchanged text caused by where a new block landed, invisible in the added lines and invisible to every check. Two for two on multi-paragraph insertions into an existing section. The lesson is not "be careful" --- it is that after inserting a block I should read the paragraph before and the paragraph after the seam, not just the block.

A count I introduced while fixing it. The section opened "The two cases above are checks you read the output of". With the H4 now above it, a reader could count three. Changed to "The cases above are all checks...", dropping the number rather than updating it --- an unnumbered reference cannot go stale on the next insertion, which is the failure that produced #1009's round-1 findings.

Dupe-check note --- accepted, and the PR body is corrected. You are right that the claim overstated the search. My grep was silence is not success|watcher, which cannot match prose saying "the monitor's silence". memories/claude-code.md does carry a related entry: a pipe-stage route to the same silence, where grep -q/-l/-c upstream starves a later stage. Not a duplicate --- that one is about what reaches the filter, this one about what the filter matches, so the fixes differ --- but it belongs cross-linked, and the new section now does that.

Monitor-docs quote --- verified from this side. You flagged it as unverifiable from the review environment, which was the right call rather than a guess. Confirmed accurate: the Monitor tool's description carries a Coverage --- silence is not success section, including the sentence "if this process crashed right now, would my filter emit anything?" and the paired wrong/right grep examples. The entry's paraphrase matches.

Re-verified locally: markdownlint 446 files 0 errors; 0 banned glyphs, 0 multi-sentence lines, 0 lines over 80 characters.

@github-actions
github-actions Bot removed the request for review from Copilot August 1, 2026 05:38
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Code Review — #1012

Scope: Single-file documentation change to shared/principles/fail-fast.md — adds one new ### subsection (57 lines) documenting a failure mode where a background poll/watcher loop silently reports nothing when it times out without the condition it's waiting for ever arriving.

Prior review: I (previous run) found one structural defect at commit a8d1efa: the new section was inserted between an existing ### block and its own #### child subsection, breaking that child's positional cross-references ("the rule above", "the remedy this section asks for") and re-parenting it under the wrong heading.

Fix verification (current head bb5628e): The section was moved to immediately after the "zero-shaped summary" #### subsection and before "The pattern itself is the other half" ### section, restoring the original adjacency. Confirmed directly against the file:

  • Heading order is now: fan-out → zero-shaped-summary (####) → background-watcher (###, new) → pattern-itself (###) — the child H4's positional references ("the rule above", "the remedy this section asks for") again correctly point at the fan-out section.
  • The count-reference bug introduced while moving the block ("The two cases above" → "The cases above are all") was fixed to an unnumbered form, matching the corpus's own avoid-hardcoding-external-data guidance against hand-maintained counts that drift.
  • The dupe-check claim in the PR body was corrected and the cross-link to memories/claude-code.md's pipe-stage-starvation entry was added — verified that file and section exist and match the description.
  • Re-scanned the diff myself: 0 banned non-ASCII glyphs, 0 lines over 80 characters, 0 multi-sentence lines in the added text.
  • Content check (independently, plus a second-opinion agent pass): the shell example (25 iterations × 60s sleep) correctly matches the cited "exited silently after 25 minutes" case; the two stated fixes map cleanly onto the four Do/Don't bullets; the memories/claude-code.md cross-reference describes a genuinely distinct failure mode rather than duplicating this one.
  • Checked for other files needing sync (index/count references to this file) — none exist; this is a self-contained addition.

No new findings.

Verdict

Ready for merge.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

💰 Cost: $4.5507 (review) — run

@d-morrison

Copy link
Copy Markdown
Collaborator Author

Ready for merge

Round 2 verdict is Ready for merge, all checks pass, zero unresolved threads. Head is bb5628e.

Round Verdict Outcome
1 Needs minor changes 1 structural defect + 2 minor notes, all Addressed in bb5628e
2 Ready for merge fix verified against the file, no new findings

The round-2 review also independently checked the two things I had asserted rather than shown --- that the shell example's 25 iterations at 60s matches the cited 25-minute case, and that the memories/claude-code.md cross-reference points at a genuinely distinct failure mode rather than a duplicate. Both confirmed. It went further than I did on one point too, noting the unnumbered-reference fix matches avoid-hardcoding-external-data's guidance against hand-maintained counts that drift --- which is the right home for that instinct and one I had not connected.

Not merging --- yours to call.

@d-morrison
d-morrison merged commit 7cb4b8d into main Aug 1, 2026
9 checks passed
@d-morrison
d-morrison deleted the ums/2026-08-01-silent-watcher branch August 1, 2026 05:45
@claude claude Bot mentioned this pull request Aug 1, 2026
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