Skip to content

feat(hooks): give the advisory event log a reader - #1758

Open
anikinsasha wants to merge 1 commit into
danielmiessler:mainfrom
anikinsasha:pr-a/advisory-event-loop-v2
Open

feat(hooks): give the advisory event log a reader#1758
anikinsasha wants to merge 1 commit into
danielmiessler:mainfrom
anikinsasha:pr-a/advisory-event-loop-v2

Conversation

@anikinsasha

Copy link
Copy Markdown

Implements #1756.

Closes the loop the Unified Event System describes but has never had a reader
for. Emission stays where it is; delivery moves to SessionStart, which is the
only end of that pair where context injection works.

What changes

File Change
hooks/lib/events.ts NEWappendEvent() (the API HookSystem.md:1555 documents), emitFindingSet(), and a backwards per-type-complete reader
hooks/lib/advisory-readback.ts NEW — changed-set digest with a slow re-announce
hooks/handlers/MemoryDirIntegrity.ts inline emit → the lib; findings gain stable keys
hooks/handlers/DocCrossRefIntegrity.ts the summary emission its docstring has claimed since v5.0.0 — emission only, apply semantics untouched
hooks/LoadContext.hook.ts fourth loader + a notification-channel guard at main()
settings.system.json (both copies) dynamicContext.advisoryReadback, ships true
LIFEOS/PULSE/Observability/observability.ts STATE/events.jsonl constant + merged into /api/events/recent
HookSystem.md, MemorySystem.md, NotificationSystem.md, hooks/README.md doc repair

Three decisions worth your attention

The emission contract is normative, not illustrative. One event per
completed check carrying the full finding set including the empty set; no event
on a skipped check. Both directions matter: an emitter that speaks only when it
has findings can never announce that a problem was fixed, so the digest shows it
forever; an emitter that emits an empty set from a skip path claims a clean bill
of health it never checked. DocCrossRefIntegrity's early return is commented for
exactly this reason.

Finding keys carry no counts or timestamps. The readback compares key sets.
A count-bearing key re-fires the digest every run; a timestamp-bearing one never
settles.

The channel guard is at main(), not per loader. Putting it inside the new
loader only would leave the existing loaders — including activeWorkSummary, the
one routinely non-empty on a real install — injecting the principal's context into
a remote turn. One check covers all of them and every future one. It sits
immediately after your isSubagentContext() check, which is the same class of
gate.

What the doc repair covers

The event-type table at HookSystem.md:1575-1589 mapped 13 categories to 11
named emitters. One of them, RebuildSkill (DocRebuild handler), has no file
anywhere in the payload — git grep returns exactly that one table row. None of
the 11 writes STATE/events.jsonl, and the two handlers that do
(MemoryDirIntegrity, and KnowledgeConformance new this release) were not in
the table. It is now a table of what writes this log, with a note listing the
other observability streams that write their own files.

### Components at :1550 was a heading followed by two blank lines; it now has
the component table it was going to have. MemorySystem.md:647 asserted that
every hook in its integration table emits here; it now names the three that do.

Wire-format note

doc.integrity.memory_dir events change shape: driftfindings (each with a
stable key), drift_countfinding_count. There are no programmatic readers
of this file in any release, so the only consumers are humans tailing it, but it
is a change and I would rather name it.

timestamp stays UTC via Date#toISOString, matching every event ever written to
this log. lib/time.ts's getISOTimestamp() is local-with-offset; adopting it
would have silently changed the format, and it drags identity.ts and a YAML
parse onto a path inside every hook run.

Shared files and rebase expectations

Every row below is the result of merging the two branches against this base, not
a reading of the diffs.

File Also touched by Expectation
hooks/handlers/MemoryDirIntegrity.ts PR-B Conflicts, and the resolution decides whether this PR still works. See below.
hooks/LoadContext.hook.ts PR-D, PR-E Conflicts with each of them. The regions differ in intent but sit close together: this PR's loader and main() guard against D's edit inside the learning block and E's deletion of the relationship block. Small, and both sides are kept each time. D and E merge clean with each other.
hooks/handlers/DocCrossRefIntegrity.ts PR-C Conflicts. Both touch the SIDE EFFECTS docstring and the emit block — C's proposal event and this PR's typed emitter want the same lines. Small; take both sides.
settings.system.json ×2 PR-C, PR-E Merges clean in every combination, including with E's removal of a sibling key inside dynamicContext.
hooks/README.md PR-C, PR-E Different rows of the same tables; merges clean.
LIFEOS/PULSE/Observability/observability.ts Sole toucher in this set.
LIFEOS/DOCUMENTATION/Memory/MemorySystem.md PR-B Merges clean — this PR edits the note at :647 in the Hook Integration section, B rewrites the inventory table ~380 lines above it.

Resolving the PR-B conflict. Two hunks, and neither is either/or. In the type
block, this PR adds the DriftItem docstring and key field while B adds the
status vocabulary sets: keep both. In the handler, B extracts the drift loops into
a pure computeDrift() and adds a third direction: keep B's structure, and carry
this PR's key fields into its pushes — unknown_on_disk:<dir>,
missing_active:<NAME>, unrecognised_status:<NAME>,
inventory_unparseable:<class>.

Taking B's side verbatim silently removes every key. Keys are the finding identity
the SessionStart readback compares to decide whether anything changed, so without
them the digest re-announces on every run or falls silent depending on ordering,
and the change-detection this PR is about stops working. Both test suites stay
green either way — B's does not know about keys, and mine tests the readback
rather than this handler — so a typechecker is what catches it, not bun test.

Tests

hooks/lib/events.test.ts and hooks/lib/advisory-readback.test.ts — 30 cases,
bun test, green against this base. They cover the contract claims specifically:
an empty set is still emitted and flagged ok; a chatty emitter cannot evict a
quiet one from the reader's window; multi-byte characters survive a read-window
boundary; a torn trailing line is skipped; the digest is silent on an unchanged
set, re-announces after 20 sessions, never re-announces an empty one, and renders
one finding as exactly one line whatever the finding's text contains.

Two boundaries stated rather than left implied. The anti-eviction property holds
within the reader's maxBytes window and not outside it: on an append-only log a
quiet type whose last emission is further back than 8 MiB is not found, which
costs an absent advisory line, never a wrong one. And detail is flattened before
it is rendered, because MemoryDirIntegrity puts on-disk directory names in it
and the digest lands inside the SessionStart block.

This repository ships no tests today, so these are additive and entirely
separable — the fix stands without them if you would rather not carry them. They
are colocated *.test.ts because that is the convention hooks/lib/work-events.ts:9
already names.

Deliberately not included

`MEMORY/STATE/events.jsonl` is documented as the Unified Event System. Two
shipped handlers write it — MemoryDirIntegrity and, new this release,
KnowledgeConformance — and nothing in any release has ever read it. Findings
produced at SessionEnd reach neither a human nor the model: SessionEnd stderr
is a transient teardown print, and the `appendEvent()` emitter the docs
describe has never shipped.

- `hooks/lib/events.ts` (new): `appendEvent()` — the API HookSystem.md
  documents — plus `emitFindingSet()` and a backwards per-type-complete
  reader.
- `hooks/lib/advisory-readback.ts` (new): a changed-set digest with a slow
  re-announce. Steady state is zero characters.
- `LoadContext.hook.ts`: a fourth loader, plus a notification-channel guard
  at `main()` rather than inside the new loader — the three existing loaders
  would otherwise inject the principal's context into a remote turn too.
- `MemoryDirIntegrity.ts`: emits through the lib; findings gain stable keys.
  Keys carry no counts and no timestamps: the readback compares key sets, so
  a count-bearing key re-fires every run and a timestamp-bearing one never
  settles.
- `DocCrossRefIntegrity.ts`: the summary emission its docstring has claimed
  since v5.0.0. Emission only — apply semantics untouched, that is a
  separate PR.
- `observability.ts`: the missing `STATE/events.jsonl` constant, merged into
  `/api/events/recent`.
- Doc repair: `### Components` was an empty heading; the event-type table
  named eleven emitters, of which one (`RebuildSkill`) has no file anywhere
  in the payload and none writes this log; `MemorySystem.md` asserted that
  every hook in its integration table emits here.

The emission contract is normative, not illustrative: one event per COMPLETED
check carrying the full finding set including the empty set, and no event on
a skipped check. An emitter that speaks only when it has findings can never
announce that a problem was fixed, so the digest shows it forever; one that
emits an empty set from a skip path claims a clean bill of health it never
checked.

Wire-format note: `doc.integrity.memory_dir` changes shape — `drift` →
`findings` (each with a stable `key`), `drift_count` → `finding_count`. There
are no programmatic readers of this file in any release, so the only
consumers are humans tailing it, but it is a change and I would rather name
it.

30 tests, bun test, green.
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