Skip to content

feat(hooks): SessionStart instinct primer and research-dispatch hook - #180

Open
vraspar wants to merge 4 commits into
mainfrom
vraspar/session-primer-dispatch-hook
Open

feat(hooks): SessionStart instinct primer and research-dispatch hook#180
vraspar wants to merge 4 commits into
mainfrom
vraspar/session-primer-dispatch-hook

Conversation

@vraspar

@vraspar vraspar commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Widens the hook trigger surface so the marketplace check no longer depends on the agent using WebSearch, and puts a one-paragraph Tenjin instinct primer at the top of every session.

Closes #173. Implements items 1 and 3 of #174 (WebFetch arm, dispatch hook); the shell-research decision and check-visibility items stay tracked there.

What

  • SessionStart primer (tenjin-sessionstart.mjs, matcher startup|clear|compact): injects one fixed paragraph teaching what a Tenjin-shaped question is and to run tenjin search before spending on research, including when enumerating sources in a subagent prompt. No network, no ledger state, no update line: the primer stays pure and brief. Runtime toggle hooks.sessionPrimer (default on).
  • Research-dispatch hook (tenjin-dispatch.mjs, matcher Agent|Task|WebFetch): the dispatch of a research subagent is the one moment the full question exists as text, before the tokens are spent. The hook forwards the task description plus the first 400 characters of the prompt to /api/agent/search, injects at most 2 candidate lines on a hit (same format and attribution guard as the WebSearch hook), and records HIT/MISS under source: 'dispatch-hook'. Skips prompts under 80 chars and dedupes per session by normalized question.
  • WebFetch arm: logged to the ledger under source: 'webfetch-hook', never injected into (prior dogfooding measured 45% duplicate noise on fetch-time hints). Demand data only.
  • The Stop nag ignores both new sources: dispatch and fetch entries never become nags. outcome --last keeps targeting deliberate searches only.
  • Shared request/validation/store core extracted into one generated-source block used by both PreToolUse scripts; HOOK_SCRIPT_VERSION 17 → 18; install disclosure and command reference updated.

Privacy note (decision, flagged deliberately)

This is the first hook that sends non-search text off-box: up to 400 characters of a subagent prompt head, plus up to 100 of the task description. Bounded by constants, disclosed in the install prompt and docs, and gated by the existing hooks.searchMode toggle (off silences it). If we want a separate toggle or a smaller slice, say so on this PR.

Verification

  • Full vitest suite: 2178 passed, 10 skipped, exit 0 (isolated HOME).
  • scripts/pack-smoke.sh run explicitly outside vitest after build: PASS.
  • Real install --harness claude into a temp home verified the wiring shape: two PreToolUse entries (WebSearch → websearch script, Agent|Task|WebFetch → dispatch script), SessionStart → primer, Stop unchanged; running the installed primer emits exactly the specified paragraph.
  • Lead spot-re-ran the three core test files independently: 202 passed, exit 0.

🤖 Generated with Claude Code

Register two more Claude Code hooks so the retrieval reflex fires before the
research does. A SessionStart hook (matcher startup|clear|compact) prints one
paragraph on when to search first, with no network call and nothing else riding
along; hooks.sessionPrimer off silences it at run time. A PreToolUse hook on
Agent|Task|WebFetch asks the marketplace what a subagent was dispatched to find
out, sending the description plus at most 400 characters of the prompt, and
mentions at most two tested answers in the WebSearch hook's format. WebFetch is
logged and never injected into.

Both demand arms record into searches.json under new sources, dispatch-hook and
webfetch-hook, which the Stop hook skips unnagged: its strong arm stays cli-only
and its weak arm websearch-hook-only. The WebSearch hook's store, request and
response boundary are now shared source rather than duplicated a third time.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator A1igator left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: the primer is clean, the dispatch hook needs its own consent before it ships

Reviewed against main at 0c92f5b, verified at df94055. Read at the head checkout rather than off the diff. The three Majors are all one question asked three ways: this PR moves the hooks from "ask the marketplace what you were already asking the web" to "tell the marketplace what you are about to do internally", and the consent, the defaults, and the local ledger were all sized for the first one.

What's solid:

  • The primer is inert by construction: PRIMER_TEXT is a compile-time constant serialized into the script through JSON.stringify, and the SessionStart script reads nothing but stdin, which it drains and discards, and the one config key. No stored entry and no marketplace response can reach it, which is the property that matters most for a hook that speaks first in every session.
  • Writing it to fd 1 directly rather than through emit() so the update signal cannot join it, with a test that pins exactly that.
  • nextHooks[spec.event] replacing hooks[spec.event] in the wiring loop: with two PreToolUse specs the old read would have had the second spec overwrite the first, and the note() dedupe keeps the reported events honest.
  • latestSearch narrowed to undefined || 'cli' rather than excluding one source by name, so neither new source can re-target outcome --last.
  • Uninstall now claims and removes all four scripts from one list, in both directions.

Major

  1. [security] the dispatch hook sends internal prompt text off-machine under a key the user granted for something else: dispatchQuestion builds description + ': ' + prompt.slice(0, 400) and posts it to tenjin.blog (hook-scripts.ts:775-784), gated only by hooks.searchMode, which defaults to auto. The WebSearch hook's whole justification is that the query was leaving the machine anyway; a Task or Agent prompt was not. Subagent prompts routinely carry repo and branch names, file paths, SHAs, ticket text, and pasted code, and 400 characters is most of a brief rather than a fragment of one. The consent moment understates the surface twice over: the auto choice hint says "before a WebSearch or a subagent dispatch" and never mentions WebFetch at all (install.ts:1326), and a non-interactive re-install takes stored ?? DEFAULT_HOOK_MODE without prompting, so an existing auto user is upgraded into the wider egress with no question asked, which is the path an agent-driven upgrade takes. Fix: give the dispatch arm its own key, hooks.dispatchMode, defaulting to off or remind, so hooks.searchMode keeps meaning what it meant when it was answered. If it must ride one key, treat an existing hooks.searchMode in the config file as consent for the old surface only and re-ask once on upgrade. Either way the choice hint should name every tool the key turns on.

  2. [security] the WebFetch arm is a network call with no return to the person running it: recordSearch runs before the isFetch mute (hook-scripts.ts:816-828), so every WebFetch posts its prompt and host to the marketplace and writes a store entry, and the comment is explicit that nothing is ever injected back because a hint there measured as noise. The user pays the egress, the latency, and a store slot; the marketplace gets demand data. That may well be a trade worth offering, but it is not one hooks.searchMode: auto was ever asked about, and WebFetch is the highest-frequency of the three triggers. Fix: make the fetch arm opt-in on its own, or hold it until it returns something to the person whose machine it runs on. If it ships as is, it belongs in the consent choice rather than only in the post-install summary line.

  3. [data-integrity] three writers, one drain, fifty slots: STORE_MAX_ENTRIES is 50 (hook-scripts.ts:91) and the Stop hook's new three-way deliberately never raises dispatch-hook or webfetch-hook (hook-scripts.ts:1053-1062), so those entries are never nagged, never closed, and never leave except by eviction. They still take slots from the two things the store exists for: buy <resourceId> resolves a candidate's payable read URL out of it, and latestSearch and outcome --last need the cli entries. One research turn with a ten-way fan-out and twenty fetches evicts every deliberate search the user might still have wanted to buy from or report on. Fix: budget the demand-only sources separately inside the 50, or keep them in their own ledger, since they are telemetry rather than searches anyone can act on.

Minor

  1. [security] marketplace-authored text now lands at the moment a subagent is briefed: the mitigations carry over unchanged and they are the right ones (control bytes stripped, " folded to ', framed as a listing, and the disclaimer line), but the injection point moved. On Agent/Task the hint sits directly beside a prompt about to be handed to another agent, and the marketplace is permissionless, so the title corpus is attacker-authored by design. Worth stating in the code whether the disclaimer travels into the subagent's own context or stays in the parent's, since a crafted title that reaches the child without its framing is the case this defense is for.

  2. [performance] up to two seconds in front of every dispatch and every fetch: SEARCH_TIMEOUT_MS is 2000 with a 5s harness kill, and alreadyAsked skips only exact fingerprint repeats within a session, so a fan-out of ten distinct prompts can add up to twenty seconds spread across the dispatches, plus that many calls against the agent-search rate budget. The WebSearch hook paid this once per web search a human-scale flow produced; a fan-out produces them in bursts. Worth either a shorter budget on the dispatch arm or a per-session call ceiling.

  3. [hygiene] SESSION_START_MATCHER omits resume: it is 'startup|clear|compact' (harness-hooks.ts:58) and the comment calls that "a new session, and the two ways a running one loses its context". A resumed session is a fresh process with an empty context too, so either it belongs in the set or the comment should say why it does not.

Nits (2), none blocking
  1. [hygiene] one event can now land in two result lists: with the WebSearch entry already present and the dispatch entry new, PreToolUse is pushed onto both added and alreadyPresent. The reported line is right because wrote > 0 wins, but the result object says two contradictory things about one event.
  2. [hygiene] the primer states a price: "a hit costs cents" ships in every session's context and pins a number the marketplace controls. "A hit is priced per piece" costs nothing and cannot go stale.
Verified, not issues
  • Nothing stored or remote can steer the primer: static constant, no store read, no network, and the sessionPrimer key is the only input. writeFileSync is imported in the shared prelude, so the fd-1 path really writes rather than throwing into main().catch(quiet) and silently printing nothing.
  • No hook emits permissionDecision, so none of the three PreToolUse arms can block or alter a tool call, and a test pins it for the new one.
  • The question built from tool input passes through clean() and is sent as a JSON body only, with no shell and no interpolation into the script.
  • HOOK_SCRIPT_VERSION moving 17 to 18 collides textually with #177 but not functionally: writeScripts compares the full script text (onDisk === spec.script), so a stale script is rewritten whatever the version line says.
  • The Agent|Task alternation as one matcher with one entry per script is consistent with the ownership-by-filename rule the module documents.

Heads up on merge order, not a review finding: #113 overlaps on seven of these files including harness-hooks.ts, hook-scripts.ts, install.ts, uninstall.ts and config.ts, and it also adds hook specs and script files to the same two lists, so a careless merge there can drop one side's script from specs() or from the uninstall set and leave orphans behind. Your own #177 and #179 both touch hook-scripts.ts, and #177 rewrites the same Stop hook source branch this PR turns three-way.

Verdict: comments-only, three Majors, all in the consent and accounting around the new arms rather than in the mechanism. The hook code itself is the same careful shape as the existing two, the primer is the cleanest part of the PR, and the test coverage on behavior is genuinely thorough. What is missing is a separate gate for a materially different kind of egress, and a home for demand entries that is not the ledger a purchase depends on. Ran locally: head checkout, read of the generated script bodies and the wiring, and a pass over the prelude to confirm the primer's write path resolves.

Review round on #180. The fetch arm is gone: nothing fires on a WebFetch, the
matcher is Agent|Task, and the webfetch-hook source (which shipped only in this
PR) is out of the enum. The install disclosure and the search-hooks choice hint
now name the subagent prompt and its 400-character bound rather than only "the
query text".

Two bounds on a fan-out. A session gets at most 10 dispatch lookups, counted from
the store before the request, so a ten-way research turn cannot put the fetch
budget in front of every subagent. Demand entries hold at most 15 of the store's
50 slots and the oldest go first, so a flood cannot evict a cli entry that buy or
outcome --last still depends on.

Also: one event now reports its strongest outcome only, instead of landing in
both added and alreadyPresent; the injection site states that the hint reaches
the parent's context and never the subagent's; and SESSION_START_MATCHER says why
resume is omitted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vraspar

vraspar commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 8f2b887 addressing the review. Disposition per finding, with the author's decisions marked as such:

Major 1 (dispatch egress consent): resolved by author decision. The dispatch arm stays under hooks.searchMode and auto-enables for existing users on re-install; no separate key and no re-ask step. The consent hint and the install disclosure now name the dispatch arm and the 400-character bound explicitly (wording author-approved verbatim).

Major 2 (WebFetch arm): dropped entirely, by author decision. Matcher is now Agent|Task, the webfetch-hook source is gone from the enum, and no fetch ever triggers a marketplace call.

Major 3 (store accounting): fixed. Demand entries are budgeted to 15 of the 50 slots (budgeted() on the shared save path); a dispatch flood evicts its own oldest entries and can no longer drain cli or websearch-hook entries. Tests pin that a cli entry with candidates survives a 60-entry dispatch flood.

Minor 1 (injection point): comment added at the injection site: the hint lands only in the parent's context, tool_input is already formed, so the subagent never sees a title without its disclaimer framing.

Minor 2 (burst cost): fixed with a per-session ceiling of 10 dispatch lookups, counted before the API call. Counted from the bounded store, so it rate-limits bursts rather than capping a session for life; stated in the code.

Minor 3 (resume): declined. A resumed session restores its transcript, so the primer from the original SessionStart is already in context; the matcher comment now says exactly that.

Nit 1: fixed; result lists are exclusive per event (added > updated > alreadyPresent).

Nit 2 ("costs cents"): declined by author decision; pricing is staying in that range and the cheapness claim is doing work in the primer.

Thanks for the merge-order heads-up on #113/#177/#179; noted for whoever lands second.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator A1igator left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review round 2: the two code Majors are closed, the consent one is not yours to close

Delta only, df94055..8f2b887. Verified at 8f2b887, CI green at post time.

Resolved since round 1

  1. [closed] Major 2, the WebFetch arm: gone rather than gated, which is the cleaner answer. DISPATCH_MATCHER is now 'Agent|Task' (harness-hooks.ts:54), fetchQuestion and the isFetch branching are deleted, webfetch-hook is out of SearchSourceSchema, and the in-script defense in depth is back to two tool names. Checked for leftovers across src/ and docs/: every remaining mention of WebFetch is a negative statement, and a settings test asserts the serialized wiring contains the string nowhere at all.

  2. [closed] Major 3, the store drain: budgeted() sits on the shared save path in marketplaceSource (hook-scripts.ts:537-547), so both generated scripts enforce it, and iterating newest-first means the entries it drops are the oldest demand ones. Three tests pin the shape that matters: a flood cannot evict a cli entry, the newest demand entries survive, and a websearch-hook entry is deliberately not budgeted because that source is nagged and closable. Fifteen of fifty is a defensible line for something nothing ever closes.

  3. [closed] Minor 1, the injection site: the comment answers the question I actually asked, and the answer is right: tool_input is already formed when PreToolUse fires and the hook emits no permissionDecision and no modified input, so the hint reaches the parent only and the titles can never arrive somewhere their disclaimer did not (hook-scripts.ts:854-855).

  4. [closed] Minor 3, resume declined: accepted on merits. A resumed session restores its transcript, so the primer the original SessionStart printed is still in context, and compact is in the set precisely because compaction is the case where it is not. The rationale now sits at the matcher where the next reader will find it.

  5. [closed] round-1 nit, one event in two lists: replaced with a rank map that reports each event once by its strongest outcome, in HOOK_EVENTS order.

Still open

  1. [security] Major 1, the dispatch-egress consent: the disclosure wording now matches what you described. The auto choice hint names the dispatch arm and the 400-character bound (install.ts:1326), and the install summary says the same in its own words (install.ts:683). The remaining question is whether the arm ships default-on under hooks.searchMode with no re-ask on upgrade, and that is the operator's product-consent call rather than an author decision or a review finding. It stays open awaiting the operator, and I am not re-arguing the merits here.

New

  1. [performance] the burst ceiling does not count the calls that cost the most: spentThisSession counts recorded dispatch-hook entries (hook-scripts.ts:817-822), and a record is only written after askTenjin returns non-null, so a lookup that times out costs its full budget and does not count toward the ten. During an outage, which is exactly when the bound should bite, a sixty-way fan-out pays the fetch budget sixty times. The second gap is if (sessionId === null) return 0: on a harness that names no session the ceiling never binds at all, and alreadyAsked only dedupes identical questions, so distinct prompts stay unbounded there. Fix: count attempts rather than records, or stop the arm for the session after a couple of consecutive failures, the way an unhealthy server already stops a batch elsewhere in this codebase.

  2. [hygiene] the budget belongs to the store, but only one of its two writers knows about it: lib/search-store.ts's recordSearch still trims with a plain .slice(0, MAX_ENTRIES) (search-store.ts:148-151), so a tenjin search write drops the oldest entry whatever its source while fifteen demand entries sit untouched. The bounded-share invariant survives, since any hook write re-applies it, but the property the budget was added for, that a demand entry never costs a deliberate one its slot, holds only on the hook path. Mirroring budgeted() in the TS writer makes it a property of the store rather than of one writer.

Nits (2), none blocking
  1. [hygiene] an unconsumed changeset on main now contradicts this one: .changeset/adoption-loop.md describes "Two harness hooks" and "A PreToolUse hook matched to WebSearch". It is unreleased, so it will be concatenated into the same CHANGELOG entry as this PR's changeset, which describes four hooks. This PR is what makes it false, so a one-line edit here keeps the released notes internally consistent.
  2. [hygiene] the primer still states a price: "a hit costs cents" is unchanged. Noting it stands rather than re-raising it; your call.
Verified, not issues
  • The demand budget counts only dispatch-hook, leaving websearch-hook unbudgeted on purpose, which is right: that source is nagged, closable, and drained.
  • latestSearch and the Stop hook's three-way still exclude the demand source, and a test round-trips it through both.
  • The ceiling at ten sits below the store budget of fifteen, so within a session the ceiling binds first and the budget is the backstop rather than the mechanism.

Verdict: comments-only. Both code Majors are closed and closed well, the WebFetch removal in particular. Two new Minors, both narrow, both in the bound that was added this round rather than in what it replaced. Major 1 is not a defect and not yours to settle; it sits with the operator.

… both writers

Review round 2 on #180.

The burst ceiling counted recorded lookups, so the case it should bound hardest
was the one it could not see: a lookup that times out records nothing, and during
an outage a wide fan-out paid the full fetch budget every time. The dispatch arm
now keeps a consecutive-failure count in hook-health.json and goes quiet for ten
minutes after two, self-healing when the window expires. An answer clears the
count, and a MISS is an answer. The ceiling also binds on a harness that names no
session: the unstamped '' bucket is a real session here, exactly as it is for the
dedupe.

The demand budget moves into the store's other writer. lib/search-store.ts's
recordSearch trimmed with a plain slice, so a `tenjin search` write dropped the
oldest entry whatever its source while demand entries sat untouched; both writers
now apply the same rule, and both bounds are exported from one definition and
baked into the generated scripts.

Also: .changeset/adoption-loop.md said two hooks and two scripts, which this PR
makes false, and both changesets concatenate into one CHANGELOG entry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vraspar

vraspar commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Pushed bc7ce9e for round 2.

Major 1 (consent): closed by the operator, not by an author on their behalf. The auto-enable-on-re-install call, the single-key model, and the final disclosure wording were each decided directly by the operator during this PR's rounds; the wording you verified at install.ts:683 and install.ts:1326 is their approved text verbatim. There is no party left to await, so treating it as open would be re-litigating a settled operator decision.

New Minor 1 (ceiling gaps): fixed, with one correction to the review. Two consecutive failed lookups (a throw and an error-null count the same; a clean MISS is an answer) now stop the dispatch arm for 10 minutes, time-bounded so an outage self-heals without intervention, tracked in a new hook-health.json following the hook-nags conventions (unlocked, cheap to lose, corrupt-reads-healthy). The null-session gap is closed the same way alreadyAsked handles it: unstamped entries count when no session_id arrives. The correction: the review says an unhealthy-server batch stop already exists elsewhere in this codebase; it does not. We searched by concept and by mechanism across src/, evals/ and scripts/, and the only failure-shaped signal is the 429 RATE_LIMITED error, which stops nothing on its own. The state-file and TTL conventions were mirrored from hook-nags.json and update-check.ts instead.

New Minor 2 (store writer): fixed. budgeted() now lives in both writers; MAX_ENTRIES and DEMAND_MAX_ENTRIES are exported from search-store.ts and baked into the generated scripts, so both bounds have one definition, with a mirrored-must-update-together note at each copy.

Nit 1 (changeset): fixed minimally to state current truth. One pre-existing staleness in the same file was left alone deliberately (its closing "uninstalling the hooks is still manual" sentence, made false by an earlier PR, not this one).

Nit 2: stands, operator's call, as you noted.

Every new gate was verified by reverting it (failure-stop threshold, null-session count, store budget) and confirming the tests fail, then restoring. Full suite 2192 passed, pack-smoke green, and a live check against a dead port ends at failures: 2 with the third dispatch exiting before any attempt.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator A1igator left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review round 3: everything closes, and one correction is mine

Delta only, 8f2b887..bc7ce9e. Verified at bc7ce9e, checks green. Final round from me.

Correction I owe you first. My round-2 fix line said to stop the arm "the way an unhealthy server already stops a batch elsewhere in this codebase". You are right that no such thing exists here. I checked origin/main: there is no HALTING_FAILURES and no batch-stop anywhere in src/, only a 429 mapped to RATE_LIMITED, which stops nothing. I was describing an unmerged PR of yours as if it were the codebase, which is exactly the kind of unverified claim this loop is supposed to catch rather than produce. The suggestion happened to be right on its own merits, but the precedent I cited for it was not there.

Resolved since round 2

  1. [closed] Major 1, the dispatch-egress consent: settled by the operator, who accepts the default-on arm under hooks.searchMode with the disclosure verified in round 2. Not re-argued, and it does not come back.

  2. [closed] Minor 1, the ceiling's two gaps: both closed, and the harder one closed properly. A throw and a null are now the same outcome to the counter, an answer of any kind clears the run, and a MISS counts as an answer because it is one (hook-scripts.ts:123-131). Two details worth naming because they are easy to get wrong and are right here: the run restarts rather than increments once the window has passed, so a failure from months ago cannot combine with one today to stop the arm, and stopped() is a time window rather than a latch, so a recovered server needs nothing done to it. The null-session gap closed the way I would have asked, by making the unstamped bucket a real session on the same terms as alreadyAsked (hook-scripts.ts:829-838), since a harness that scopes nothing is the last place a bound should go missing. hook-health.json is its own file on the documented hook-nags.json terms, unreadable reads as healthy, and readJsonFile is in the prelude so the read path resolves rather than throwing into quiet(). Seven tests, including a real timeout and the unreadable-file case.

  3. [closed] Minor 2, one writer knowing the budget: the bounds are exported from the module that owns them and baked into the generated body (search-store.ts:15-23), and budgeted now runs in the TS writer too, so the bound belongs to the store rather than to whichever process wrote last. Baking the imported constants is the better half of this: the two copies can no longer disagree about the numbers, only about the algorithm.

  4. [closed] nit, the contradicting changeset: fixed by dropping the count rather than by rewriting the paragraph, which is the right size of edit for a changeset that is not this PR's subject.

Nits (3), none blocking, none needing a reply
  1. [hygiene] the failure counter undercounts a parallel burst: hook-health.json is unlocked by design, so subagents dispatched in one message all read the count before any of them writes, and an outage-time burst lands at one failure rather than N. It self-corrects on the next dispatch and the trade matches the Stop hook's documented one for hook-nags.json, so this is a note for the next reader rather than a change to make.
  2. [hygiene] the mirrored budgeted is now pinned by value but not by behavior: the constants cannot drift, the algorithm still can. A test that runs both copies over one fixture would close it cheaply, in the spirit of the existing test that pins the script's lock protocol against the module's.
  3. [hygiene] the deliberately-left staleness: leaving "Uninstalling the hooks is still manual" out of scope is a defensible call, and it still ships as a false sentence in a release note whoever fixes it. Worth an issue so it has an owner rather than a comment thread.

Verdict: comments-only, and nothing is open on my side. Three rounds, eight findings, all closed in the code except the one that was never a defect. The two bounds added under review, the failure stop and the demand budget, are both better than what I asked for: a self-healing window instead of a session latch, and one definition compiled into both writers instead of a second copy to keep in step. Ran locally: head checkout, a read of both generated script bodies, and a check of origin/main for the batch-stop I wrongly cited.

… current uninstall

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@A1igator A1igator left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: the last two nits, closed

Delta only, bc7ce9e..e7d9474. Verified at e7d9474, CI green. Nothing here touches the settled surfaces: the consent wiring, the demand budget itself, and the health file are all untouched, so the standing verdict holds.

  1. [closed] the mirrored budgeted is now pinned by behavior, not just by value: one fixture through both writers, compared searchId for searchId after a sixty-entry flood. That is the guard I described and it is the right shape for it, since the constants were already compiled from one definition and only the algorithm could still drift.

  2. [closed] the stale uninstall sentence: replaced with what tenjin uninstall actually does, which matches the code I read in round 1: it unwires all four entries by filename and deletes the scripts.

Verdict: unchanged, comments-only, nothing open on my side. Three rounds plus this one, all findings closed in the code except the consent question, which the operator settled.

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

Labels

critical Drop everything: data-corrupting or user-facing breakage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

WebSearch hook works, but agent research bypasses it: 2.07M tokens, zero triggers

2 participants