agent-memory-sync: mid-tick progress signal + inactivity-based watch deadline with absolute cap - #109
Merged
Conversation
… deadline measures inactivity, not total duration Structural follow-up to PR #102, which raised withTickDeadline's per-tick test budget to a honestly-documented 90000ms MARGIN (not a guarantee) after measuring that no fixed whole-tick budget can categorically rule out a CPU-contention straggler. watch.ts's pushSnapshot() now writes "watch tick pushing snapshot" to stderr (verbose-gated) the instant a tick actually starts performPush, not only once the result is known. withTickDeadline gains an inactivity mode: when called with a getStderr callback, its deadline resets on either the existing ready line or the new push-start line, so the budget bounds the gap since the last observed progress signal instead of the tick's total duration. Called without a stderr source (watch-teardown-guard.test.ts's deliberate never-resolving fn), it keeps the original fixed-wall-clock behavior unchanged. runWatchTick and the two direct callers in watch-mirror-delete.test.ts use inactivity mode. INACTIVITY_TIMEOUT_MS is kept at 90000ms after measuring, not assuming, both directions: 45000ms was insufficient (failed on an ordinary local full-suite run with no synthetic load), and raising to 150000ms to chase a clean 10/10 under PR #102's load scenario made the pass rate WORSE (8/10), evidence the residual failures are driven by this machine's rising ambient load during this session (load average 14.34 -> 20.11 on 12 cores, other agents run here per AGENTS.md), not by budget size. Cross-checked via git stash: unmodified origin/master reproduces the identical failure class at comparable ambient load. Measured, not cherry-picked, load-scenario result across three independent 10-run trials at 90000ms: 27/30 (90%). SIGSTOP positive control still fails reliably at both 90000ms and 150000ms. New tests/unit/watch-process-inactivity.test.ts pins the inactivity semantics directly (fast, deterministic, no real spawn): a slow-but-progressing tick that outlives one budget window in total but keeps signaling inside it resolves; a tick with zero progress signals is still killed once the budget elapses. CHANGELOG.md added (package had none); README documents the new verbose line. Refs: eb798875-6355-413f-b274-91ba5a856b13 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
…-tick-progress-signal
…dings Fixes 4 reviewer findings from the eb798875 review round, merged onto current master (PR #108's push/base-snapshot changes): - HIGH (production line untested): watch-restore.test.ts's single-file commit message test now asserts stderr matches "watch tick pushing snapshot", that the push-start line precedes the result line (mid-tick, not end-of-tick), and a new test asserts the line is absent without --verbose. Deleting the production writeInfo call now turns the new assertion red while the rest of the suite stays green. A new unit test also pins that the push-start pattern specifically (not just the ready pattern) resets withTickDeadline's inactivity deadline. - MEDIUM (falsified attribution): rewrote the INACTIVITY_TIMEOUT_MS comment and CHANGELOG entry to the reviewer's matched-control measurement instead of the prior single-session uptime/git-stash read. The residual load-scenario failures stall in the ready-to-push-start gap with only the ready line in stderr (the trigger edit's fs event is never delivered to chokidar) at a comparable rate on both this branch and the unmodified merge base, so no budget or signal change can fix them. Marked out of scope; a follow-up task will be filed by the orchestrator. - MEDIUM (no absolute ceiling): withTickDeadline's inactivity mode now arms a second, independent absolute whole-tick cap (2.5x the inactivity budget) that fires with a distinct message even if progress signals keep resetting the inactivity deadline forever. A new fast unit test pins it. - LOW: compressed the CHANGELOG entry to scannable bullets with no task UUIDs/PR numbers/session references, and removed the new em dashes from CHANGELOG/README prose. Refs: eb798875-6355-413f-b274-91ba5a856b13 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HPmP28EwHconrJps6tvtw2
LanNguyenSi
added
review:tests-pass
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
1 participant
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.
Summary
watch --verbosenow emits a deterministic mid-tick progress signal ('watch tick pushing snapshot') the moment the push phase starts, and the test helper'swithTickDeadlinegained an inactivity mode: the deadline bounds the gap since the last progress signal (ready or push-start line) instead of the whole tick, with an absolute whole-tick cap at 2.5x as a backstop so stacked gaps or a signal-emitting runaway stay bounded. Without a stderr source the helper is byte-for-byte the old fixed-wall-clock behavior. INACTIVITY_TIMEOUT_MS stays 90000 after a measured 4-trial calibration.Verification
Refs: eb798875-6355-413f-b274-91ba5a856b13