Repository navigation
feat(agent): report hook liveness and aggregated hook errors - #220
Open
graywolf336 wants to merge 2 commits into
Open
graywolf336 wants to merge 2 commits into
graywolf336 wants to merge 2 commits into
Conversation
`atomic agent status` answered "are the hooks installed?" and stopped. It could not answer "are they still working?", and the difference is the expensive one: the shell guard in atomic-agent::hooks ends in `|| true`, so a hook that dies is indistinguishable from one that was never installed, and a failing dispatch drops the turn silently. In this repository, .atomic/hook-errors.log held 1784 lines over six days and 21 sessions — 546 of them the same failure, "Provenance turn N is not running" — with no command that would ever read it. - Record every hook dispatch to .atomic/hook-health.json: per-agent, per-verb last_fired plus outcome. last_ok is preserved across a later failure, so "worked then broke" is distinguishable from "never worked". Written on both the dispatch and the parse-failure paths. - Read a bounded tail of hook-errors.log and aggregate it by a signature that collapses session ids and turn numbers, turning 546 near-identical lines into one actionable row. Continuation lines are folded into their entry: the plugin appends messages unescaped, and dropping the tail was losing the actual error. - Surface both in `atomic agent status`, human and --json, plus a silent_agents list naming hooks that are installed but have never recorded. That list stays empty when no health file exists at all, so a freshly-upgraded repository is not accused. The two signals are printed together because either alone misleads: a clean error log proves nothing about whether the recorder is running, and a live recorder proves nothing about what it failed to do earlier.
Several agents record into one repository at once, and the record is a read-modify-write, so two things broke. - The temp file had a fixed name. Concurrent writers shared it, so the loser's rename failed outright and the winner could publish bytes the other had interleaved into the same path. HookHealth::read treats an unparseable file as "no data", so either failure discarded the whole record rather than one entry. The temp name now carries pid and a counter. - Nothing serialised the read-modify-write, so the last writer won and silently dropped what the others had just recorded. For a first-ever fire that leaves an agent missing from the record, which reads as "hooks installed but never recorded" — the false alarm this feature exists to raise. Guarded now with an fs2 flock on .atomic/, retried for up to 50ms and then proceeding unlocked, because a hook that hangs is worse than one that loses a timestamp. flock is released by the kernel on descriptor close, so a process dying mid-write cannot wedge the file. Verified with 30 concurrent hook processes across 6 verbs: all 6 survive, no corruption, no stray temp files.
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
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.
Problem
atomic agent statusanswered "are the hooks installed?" and stopped. It could not answer "are they still working?" — and that's the expensive question. The shell guard inatomic-agent::hooksends in|| true, so a hook that dies is indistinguishable from one that was never installed, and a failing dispatch drops the turn silently.In this repository,
.atomic/hook-errors.logheld 1,784 lines over 6 days across 21 sessions — 546 of them the same failure (Provenance turn N is not running) — and no command would ever read it.Changes
.atomic/hook-health.json: per-agent, per-verblast_fired+ outcome.last_okis preserved across a later failure, so "worked then broke" is distinguishable from "never worked". Written on the dispatch path and the parse-failure path.hook-errors.log, aggregated by a signature that collapses session ids and turn numbers, turning 546 near-identical lines into one row. Continuation lines fold into their entry (the plugin appends messages unescaped; dropping the tail was losing the actual error).atomic agent status(human +--json), plus asilent_agentslist naming hooks that are installed but have never recorded. Empty when no health file exists, so a freshly-upgraded repo isn't accused.The two signals print together because either alone misleads: a clean error log says nothing about whether the recorder is running, and a live recorder says nothing about what it failed to do earlier.
Verification
cargo test -p atomic-cli— 1952 passed, 0 failed (20 new)fmt --checkcleandev— pre-existing (draft view 'agent-3' created), verified by stashing and re-running.outcome: errorrecorded withlast_okretained.Self-review findings
Both surfaced only when the feature was run against real input, not by unit tests — the parser takes strings in memory, so the file-reading path and the early-return path were unexercised. Both were introduced in this branch, are not present on
dev, and both now carry regression tests:start == 0and no seek had happened. Because the log is append-only, that silently dropped the oldest error — and a repo with exactly one error reported zero. The unit tests fed the parser strings, so only the scratch-repo run caught it: a 5-entry log parsed as 4.