[halindrome]

Summary

Pace watcher re-indexes by how long the last successful one took, so a large project whose full rebuild takes minutes no longer re-indexes back-to-back while an agent session keeps writing files.

Refs #2835

Today the next poll after a successful re-index is the normal interval (5 s + 1 s per 500 files, max 60 s). Any write changes the dirty signature, and a new file forces a full rebuild, so on a large monorepo indexing becomes nearly continuous. It competes for CPU with interactive queries the whole time.

Change

• cbm_watcher_success_delay_ms(interval_ms, index_ms) (src/watcher/watcher.c/.h) is a pure helper that returns max(interval, 3 × index duration), capped at 5 minutes (the existing INDEX_FAIL_CEILING_MS). It never returns less than the interval, treats negative inputs as zero, and divides instead of multiplying so it cannot overflow.
• poll_project times the index_fn call with now_ns(). After a success it schedules the next poll at index end + success_delay, so short rebuilds use at most about 1/4 of wall time; a 60 s rebuild is followed by 3 minutes idle, and anything from 100 s up by 5 minutes. Under continuous writes a project falls at most one rebuild plus 5 minutes behind. It logs watcher.cooldown (project, delay_ms) only when the delay is extended.
• Failure backoff and busy-skip are unchanged. Sub-second indexes keep today's cadence.
• Two CBM_ENABLE_TEST_SEAMS hooks (test builds only): cbm_watcher_test_set_clock_ns freezes the watcher's scheduling clock, and cbm_watcher_test_next_poll_ns reads a project's scheduled next poll.

Acceptance criteria

1. After a successful watcher re-index, the next poll of that project waits 3 × index duration after the index finishes (capped at 5 minutes), and never less than the normal interval.
2. The delay is capped, so one very slow run cannot stop the watcher for long.
3. Small or fast repos keep their current poll cadence.
4. Failure backoff and busy-skip behaviour are unchanged.
5. The delay computation cannot overflow and handles degenerate inputs (zero or negative).
6. Existing watcher and daemon suites stay green.

Testing

• tests/test_watcher.c: four unit tests for the helper (fast index, scaling, clamp/degenerate, monotonic) and one behavioural test, watcher_success_cooldown_measures_index_issue2835. The behavioural test uses a real git repo and the frozen-clock seam: the index callback advances the clock instead of sleeping, and the test asserts the exact scheduled poll time. An instant index keeps the plain interval; a 60 s index schedules the next poll 180 s after the index ends (anchoring at the start would give 120 s); a 200 s index is capped at 300 s. No fixed delays.
• scripts/test.sh --suites watcher: 96 passed.
• The earlier revision of this PR also ran scripts/test.sh --suites watcher,daemon,daemon_runtime,daemon_application,daemon_frontend,daemon_bootstrap,daemon_ipc,httpd,mcp: 841 passed, 0 failed, 7 skipped.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CDDUoWEZMNZ9fLvzp2pPGT

[halindrome]

QA round 1 for this change. It ran on the fork PR (halindrome#6) before the commits were squashed into 5c54201, so any commit SHAs below refer to the pre-squash history. The QA was performed by Claude Opus 5.5 (Anthropic) in Claude Code.

QA Round 1

Contract Verification

Contract: #2835 + PR acceptance criteria (cost-proportional cooldown only).

#	Criterion	Verdict	Evidence
1	After a successful re-index, the next poll waits at least 3x the index duration, and never less than the poll interval	⚠️ PARTIAL	cbm_watcher_success_delay_ms (watcher.c:217-229) correctly returns max(interval, 3×elapsed). But poll_project adds the delay to ctx->now (watcher.c:1761), which is the cycle timestamp taken once in cbm_watcher_poll_once (watcher.c:1811), not to the time this index started. See Finding 1.
2	The delay is capped at 30 min	✅ PASS	INDEX_COOLDOWN_CEILING_MS 1800000. success_delay_clamped_and_degenerate checks 600000, 600001 and INT64_MAX, and all three give 1800000.
3	Small/fast repos keep their poll cadence	✅ PASS	When 3×elapsed <= interval the function returns interval. Tested at the 1666/1667 boundary, and the wire-up test asserts fast_hits == 0.
4	Failure backoff and busy-skip behaviour are unchanged	✅ PASS	delay_ms stays at -1 for rc>0, rc<0 and a missing index_fn, so those cases fall through to the unchanged cbm_watcher_index_backoff_ms(...).
5	The delay computation cannot overflow and handles degenerate inputs	✅ PASS	The ceiling/factor comparison runs before the multiply, a negative interval clamps to 0, and elapsed <= 0 returns the interval. Tested with INT64_MAX, INT64_MIN, -1, 0 and INT_MAX.
6	Existing suites stay green, and the new tests are red without the change	⚠️ PARTIAL / not verified	The suites were not run in this round (the lenses are read-only). Without the change the pure-function tests do not link. The wire-up test stays green if only the next_poll_ns scheduling hunk is reverted. See Finding 2.
7	Trust boundary: no weakening of authn/authz, input validation or secret handling	✅ PASS	The diff changes scheduling arithmetic and adds one info log line (project name plus an integer). It adds no new input surface.

Finding 1 — [claude] Cooldown is anchored to the stale poll-cycle timestamp, so a project indexed after another in the same cycle can be rebuilt back-to-back

• Severity: major · Status: confirmed · Relevance: contract (AC 1) · Category: logic-error
• Location: src/watcher/watcher.c:1722-1761 (and :1811)

poll_project measures the index from index_start = now_ns() (L1722) and computes delay_ms = 3 × elapsed (L1729). It then sets s->next_poll_ns = ctx->now + delay_ms (L1761). ctx->now is taken once per cycle in cbm_watcher_poll_once (L1811), before any project is checked or indexed. Projects are polled one after another, so time spent earlier in the cycle comes out of the cooldown. That includes git checks and the index runs of projects polled earlier. The idle time left after the index finishes is roughly 2×elapsed minus that earlier time, and it can reach zero.

• Two watched projects A and B, both changed. A indexes for 10 min and then B for 5 min. B's next_poll is T0 + 15 min, which is exactly when B's index finishes, so B is due again on the next cycle.
• A single project whose index takes 30 min or more hits the cap. Its next_poll is T0 + 30 min, which is at or before the end of its own index, so it can rebuild back-to-back. That contradicts the comment that the ceiling prevents this.

Expected: after a successful index, the next poll is at least index_start + max(interval, 3×elapsed), however long other projects took earlier in the cycle. On the success path, use index_start (or the index-end time) as the base instead of ctx->now. In the common case of one project indexing in under 10 min the effect is small, but this is the scenario #2835 describes.

Finding 2 — [claude] Wire-up test checks the cooldown log line, not that the next poll is actually deferred

• Severity: minor · Status: confirmed · Relevance: regression · Category: test-gap
• Location: tests/test_watcher.c:2326-2378

watcher_success_cooldown_measures_index_issue2835 calls cbm_watcher_touch before every poll, and touch resets next_poll_ns = 0, which bypasses the gate this change sets. The only assertion is cooldown_log_hits == 1, and that line is logged before next_poll_ns is assigned. If the final scheduling hunk were reverted to the plain backoff, the test would still pass. A 1000× unit error in the elapsed conversion would also pass. Compare watcher_index_failure_backoff_gates_repolling_issue2015, which runs a poll without touch and checks that the gate holds. The production code is correct today, so this is a mutation gap rather than a defect. It is acceptable to defer it to a follow-up.

Summary

Severity	Confirmed
Critical	0
Major	1
Minor	1

Panel: contract-security, regression-edges and test-quality all landed on claude-opus-5-5 with navigation cmm+ctx. The working tree was unchanged after the panel. No schema change.

Observations

None.

---

SAST review not applicable

Target default has security_stage: false in the qa-cycle config. No CI
security stage is wired for this target, so no SAST/SCA delta is computed.

---

QA performed by Claude Code (claude-opus-5-5), manager + 3-lens panel

Dispositions (orchestrator, Step 3B)

• Fixed (major): the cooldown was anchored to the poll cycle's start time (ctx->now), so time spent earlier in the cycle, or an index longer than the 30-minute cap, could cancel it. After a successful index the next poll is now index end + max(interval, 3 × duration). Failure and busy-skip paths still anchor to ctx->now, as before. Comments updated: indexing now uses at most about 1/(K+1) of wall time.
• Not fixed (carried forward, minor): the wiring test asserts the watcher.cooldown log line, not next_poll_ns. The watcher has no injectable clock or next-poll accessor, so asserting the schedule needs new test infrastructure, which is bigger than the fix. Consequently, this round's fix has no red-first test; it was verified by reading only.

Verification

CC=/usr/bin/clang CXX=/usr/bin/clang++ SDKROOT=… scripts/test.sh --suites "watcher daemon daemon_runtime daemon_application" → 248 passed, 0 failed (exit 0). This is narrower than CI's default leg; the local default scripts/test.sh stops at contract Step 0t, which unmodified main also fails in this environment.

QA-Fix-Commit: a83e75b

---

Posted by the developer account (QA-agent token unavailable). Review by Claude Opus 5.5 (Anthropic) in Claude Code via qa-panel.

[halindrome]

QA round 2 for this change. It ran on the fork PR (halindrome#6) before the commits were squashed into 5c54201, so any commit SHAs below refer to the pre-squash history. The QA was performed by Claude Opus 5.5 (Anthropic) in Claude Code.

QA Round 2

Contract Verification

Contract: #2835 + PR acceptance criteria (cost-proportional cooldown only). Reviewed at the head that includes round 1's fix a83e75b7.

#	Criterion	Verdict	Evidence
1	After a successful re-index, the next poll waits at least 3x the index duration, and never less than the poll interval	✅ PASS	poll_project now sets anchor_ns = now_ns() after index_fn and next_poll_ns = anchor_ns + cbm_watcher_success_delay_ms(interval, elapsed) × US_PER_MS. The function returns max(interval, 3×elapsed). Round 1's Finding 1 (stale ctx->now anchor) is resolved.
2	The delay is capped at 30 min	✅ PASS	INDEX_COOLDOWN_CEILING_MS 1800000 (watcher.c:165). success_delay_clamped_and_degenerate checks 600000, 600001 and INT64_MAX, and all three give 1800000.
3	Small/fast repos keep their poll cadence	✅ PASS	When 3×elapsed <= interval the function returns interval (success_delay_fast_index_keeps_interval). The wire-up test asserts that an instant index logs no cooldown.
4	Failure backoff and busy-skip behaviour are unchanged	✅ PASS	On rc>0, rc<0 or a missing index_fn, delay_ms stays at -1 and anchor_ns stays at ctx->now, so those paths compute the same ctx->now + cbm_watcher_index_backoff_ms(...) expression as the base branch.
5	The delay computation cannot overflow and handles degenerate inputs	✅ PASS	The ceiling/factor comparison runs before the multiply, a negative interval clamps to 0, and elapsed <= 0 returns the interval. Tested with INT64_MAX, INT64_MIN, -1, (-1,0) and INT_MAX.
6	Existing suites stay green, and the new tests are red without the change	⚠️ PARTIAL	The lenses are read-only, so they did not run the suites. The orchestrator ran scripts/test.sh --suites "watcher daemon daemon_runtime daemon_application": 248 passed, 0 failed. Without the change the new tests do not link. Round 1's anchor fix has no red-first test; that is the deferred item recorded under Observations.

Finding 1 — [claude] Header doc and test comment say the cooldown is measured from the index start; the code measures from the index finish

• Severity: minor · Status: confirmed · Relevance: regression · Category: documentation
• Location: src/watcher/watcher.h:198-204, plus the comment in tests/test_watcher.c success_delay_slow_index_scales_by_factor
• Lenses: contract-security, regression-edges and test-quality (3 of 3 raised it)

Round 1's fix (a83e75b7) moved the cooldown anchor to the index finish and updated the .c comment at watcher.c:157-163, which now reads "counted from when the index finished … ~1/(K+1) of wall time". Two comments were not updated:

• The public doc comment for cbm_watcher_success_delay_ms still says "Measured from the index start, so the watcher spends at most ~1/3 of wall time indexing a project".
• The test comment still says "5-minute rebuild → next poll 15 min after its start, ~10 min idle".

The header now contradicts both the code and the .c comment.

Expected: both comments say the delay counts from the index finish: indexing takes about 1/(K+1) ≈ 1/4 of wall time, and a 5-minute rebuild idles for 15 minutes. There is no runtime effect, but a maintainer who trusts the header could move the anchor back, and no test would catch it (see Observations). The fix is a two-comment edit.

Summary

Severity	Confirmed
Critical	0
Major	0
Minor	1

Panel: contract-security, regression-edges and test-quality all ran on claude-opus-5-5 with navigation cmm+ctx. The working tree was unchanged after the panel. No schema change. Attribution: the finding is on lines round 1's fix commit did not write, so qa_introduced=false. It is a comment that round 1 left stale.

Observations

• [claude] The cooldown's scheduling (next_poll_ns) and round 1's finish-time anchor are not pinned by any test (minor, tests/test_watcher.c:2328-2378). This is round 1's deferred item, raised again by test-quality. The extra point this round: a83e75b7 changed only src/watcher/watcher.c, so reverting that anchor would also pass the suite. The finding stays deferred because the watcher has no injectable clock or next-poll accessor.

---

SAST review not applicable

Target default has security_stage: false in the qa-cycle config. No CI
security stage is wired for this target, so no SAST/SCA delta is computed.

---

QA performed by Claude Code (claude-opus-5-5), manager + 3-lens panel

Dispositions (orchestrator, Step 3B)

• Fixed (minor, documentation): the watcher.h doc for cbm_watcher_success_delay_ms and a test comment still described the round-1 behaviour (counted from the index start, about 1/3 duty cycle). Both now say the cooldown counts from the index finish (about 1/4 duty cycle; a 5-minute rebuild is followed by 15 minutes idle). Comment-only; clang-format --dry-run --Werror is clean. No test changed, so no red-first run applies.
• Fix review (Step 3B.5): clean, reviewed_sha 40966a20237f75d1cd69371578589c304e34d81b. Q1: the diff does what the finding asked. Q2: no sibling comment in watcher.c, watcher.h or test_watcher.c still asserts the old anchor.
• Carried forward: next_poll_ns and the finish-time anchor are not pinned by any test (no injectable clock). This is the same item as round 1.

Cycle outcome: no critical or major findings remain after round 2. The round-1 major (cooldown anchor) is fixed and verified. Not approved: no QA-agent token is available, and approval is the human reviewer's call.

QA-Fix-Commit: 40966a2

---

Posted by the developer account (QA-agent token unavailable). Review by Claude Opus 5.5 (Anthropic) in Claude Code via qa-panel.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Thank you, Shane, for digging into #2835 so thoroughly: the analysis of why agent sessions force full rebuilds is very useful, and pacing the watcher by the cost of the last successful run is a clean, well-bounded idea. The helper is overflow-safe, failure backoff and busy-skip are untouched, and anchoring the cooldown to the end of the index is the right call.

Two changes before we merge, please:

1. Please bound the success cooldown more tightly: cap it at 5 minutes (the existing INDEX_FAIL_CEILING_MS), or use 2x the last index duration as the issue proposed, instead of 3x with a 30-minute cap. With continuous writes, 3x / 30 minutes lets a project fall up to roughly 20-30 minutes behind after a long rebuild; a tighter bound keeps most of the CPU relief while keeping the index reasonably fresh. Either variant is fine with us - pick whichever reads cleaner in the code.
2. watcher_success_cooldown_measures_index_issue2835 sleeps 1.8 s inside the index callback to simulate a slow index, and it checks the log line rather than the schedule. We keep fixed delays out of the suite so tests stay fast and deterministic. watcher.c already has a CBM_ENABLE_TEST_SEAMS section; a small seam there (one that reports a project's next_poll_ns, plus a way to feed the measured index duration or the clock) would let the test assert the actual scheduled poll time with no sleep, and would also pin the end-of-index anchor that is currently untested.

Could you also change "Closes #2835" to "Refs #2835" in the description? The issue lists follow-ups worth keeping open. Thanks again, this is careful work.

[halindrome]

Thanks for the review! Both changes are in cbf3794. The work was done by Claude Opus 5.5 (Anthropic) in Claude Code.

1. Cooldown bound: the success cooldown is now capped at INDEX_FAIL_CEILING_MS (5 minutes); the separate 30-minute constant is gone. I kept the 3× factor rather than switching to 2×, because reusing the existing ceiling meant one constant fewer and the smallest diff. Below 100 s of indexing nothing changes; from there up the wait is 5 minutes, so under continuous writes a project falls at most one rebuild plus 5 minutes behind.
2. Deterministic test: two small seams in the existing CBM_ENABLE_TEST_SEAMS section of watcher.c. cbm_watcher_test_set_clock_ns freezes the watcher's scheduling clock, and cbm_watcher_test_next_poll_ns reports a project's next_poll_ns. watcher_success_cooldown_measures_index_issue2835 no longer sleeps or reads the log: the index callback advances the frozen clock, and the test asserts the scheduled poll time exactly. An instant index keeps the plain interval, a 60 s index gives 180 s, and a 200 s index is capped at 300 s. The 60 s case pins the end-of-index anchor, since anchoring at the start would give 120 s.

The description now says "Refs #2835" and reflects the new cap and test. scripts/test.sh --suites watcher passes (96 tests).