Skip to content

fix(hooks): stop the context-budget hook emitting anything, by owner decision - #1226

Open
wshallwshall wants to merge 2 commits into
mainfrom
claude/trusting-davinci-eb272a
Open

wshallwshall wants to merge 2 commits into
mainfrom
claude/trusting-davinci-eb272a

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

What this does

Makes scripts/hooks/context-budget.ps1 a no-op. It emits nothing, ever.

Why

Owner decision, 2026-09-16. The hook reported how full the session's context window was and warned the seat "before it runs out of room", claiming a compaction "drops the seat, the goal and the brief, and nothing re-declares them for you".

The desktop app compacts automatically when the context overflows. So the warning advertises a failure mode that does not occur, and its central claim about losing the seat and the brief is false. In the owner's words, it is "a complete error".

What it cost, measured in this session

It fired on every prompt from 76.9% onward. The receiving session began narrating its own context percentage back to the owner in five consecutive replies, started curtailing work it should have finished, and treated a normal condition as an emergency. That is why it is disabled rather than reworded.

What was done

  • All 65 worktree copies neutered at the same time, so no live session on any account still sees it.
  • The script is left in place as a no-op carrying the reason, rather than deleted, so a reader who finds the wiring does not conclude the file was lost and restore it.
  • The wiring in .claude/settings.json is untouched on purpose: a missing script makes the hook fail, where a silent one stays quiet.

Do not restore it

No session is to see a hook that mentions a context budget. A replacement that reports context fullness is an owner decision, not a maintenance one. The reason is recorded in the file itself so the next reader does not have to find this PR.

…decision

The UserPromptSubmit hook reported how full the session's context window was and
warned the seat "before it runs out of room", claiming a compaction "drops the
seat, the goal and the brief, and nothing re-declares them for you".

OWNER DECISION 2026-09-16: it must emit nothing, ever. The desktop app compacts
automatically when the context overflows, so the warning advertises a failure
mode that does not occur, and its central claim about losing the seat and the
brief is false.

It cost real attention as well as being wrong. A session receiving it on every
prompt starts narrating its own context percentage back to the owner, curtails
work it should have finished, and treats a normal condition as an emergency.
That is why it is disabled rather than reworded.

The script is left in place as a no-op carrying the reason, so a reader who finds
the wiring does not conclude the file was lost and restore it. The wiring in
.claude/settings.json is untouched here on purpose: a missing script would make
the hook fail rather than stay quiet.

All 65 worktree copies were neutered at the same time, so no live session sees it.
@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Sep 16, 2026
…CKLOG #1640)

The M-5 control writes one `summary_access` audit row per (actor, scope, hour)
window, so routine console polling does not produce a row per request while a bulk
harvest shows a large count. A window is emitted when a LATER access rolls it into
a new hour -- so the window open at shutdown was emitted by nothing.

`_SummaryAuditCoalescer.flush` existed and documented itself as the engine-shutdown
path. NOTHING CALLED IT. Every clean restart therefore dropped the open hour's
PHI-summary access audit, and the case where that matters most is the one the
control exists for: an operator restarting shortly after a bulk census fetch.

Not deployed (section 0), so this is written in the conditional: a first deployment
restarted within the hour of a bulk fetch WOULD hold no row for it.

WHERE, AND WHY THE ORDER IS LOAD-BEARING

The flush runs in the lifespan `finally`, BEFORE `engine.stop()`, because that ends
in `store.close()` and the emit needs the store.

It is wrapped, on the reaper's precedent: a store error during teardown must not
skip `engine.stop()`. aiosqlite's connection worker is non-daemon, so skipping it
leaves the process unable to exit -- a lost audit row would become a hung service.

CHECKS

  ruff check                 pass
  ruff format --check        pass
  mypy (strict)              pass, on the module and the new test
  new tests                  3 passed
  existing regression        13 passed (test_api.py -k "summary or audit or lifespan")

THE CONTROL, run rather than asserted. With the fix stashed out, the new test fails
with "the open window was dropped at shutdown: []". With it restored, 3 pass. The
suite also carries that control as a test of its own: `test_the_assertion_can_fail`
neutralises the flush and requires the row to disappear, so the main assertion
cannot pass for some unrelated reason.

`test_a_flush_failure_does_not_abort_the_teardown` pins the wrapper: its assertion
is narrowly that the lifespan context EXITS when the flush raises.

NOT RUN: the full suite, and every leg needing an extra this interpreter lacks
(`vault`). The venv used is the primary checkout's; `messagefoundry.__file__` was
confirmed to resolve to THIS worktree and to contain the change before any result
was trusted.

LEDGER: #1640 is not edited here. Banner text for the Lander --
  SHIPPED: the coalescer is flushed in the lifespan `finally` before `engine.stop()`,
  guarded so a flush failure cannot abort the teardown. Both limbs of "what closing
  looks like" are covered: the flush, and a lifespan-to-shutdown test asserting the
  row lands.
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

This branch now also carries 83e8d827a (BACKLOG #1640), which does not belong to this PR.

That commit is the summary-access coalescer shutdown flush. It has its own PR, #1241, on branch claude/1640-summary-flush, where it is reviewable on its own.

I intended to reset this branch back to the hook commit alone, but a force-push is a destructive operation and was correctly declined, so I have not rewritten a published branch.

For the Lander: both branches contain the same commit, so whichever merges first makes the other's copy redundant and the second PR's diff for those files goes empty. No conflict either way. If you would rather this PR be the hook change alone, the reset is:

git -C <worktree> reset --hard 331a77b3c && git -C <worktree> push --force-with-lease origin claude/trusting-davinci-eb272a

The #1640 commit is safe on the pushed claude/1640-summary-flush branch either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant