feat(cli): tenjin pay and tenjin discover, with the bazaarPay toggle - #151
feat(cli): tenjin pay and tenjin discover, with the bazaarPay toggle#151A1igator wants to merge 17 commits into
Conversation
…ze seam Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ize seam Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lock Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
…mit, endpoint identity CDP's Bazaar ignores the payTo list filter, clamps limits (search at 20), and its search cannot match a URL (all verified live). So discover persists its sweep as pay-time evidence (24h TTL, capped, atomic), the pay lane checks that store first, and resource identity is origin plus path so listed endpoints match requests carrying query strings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
30bda1d to
ae6ee8e
Compare
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Manual prod proofs, run against live tenjin.blog and the live CDP Bazaar with the local CLI wallet (0x3200...BB38): Tenjin lane
Bazaar lane (toggle enabled via the hand-edit path, which the runtime gate honors immediately)
The proofs surfaced three registry realities, fixed in the follow-up commit on this branch: CDP's search endpoint rejects |
…gate When a 402 advertises the standard sign-in-with-x extension, pay runs buy's sequence: one SIWX re-check whose signature binds to the TARGET origin (never the configured deployment's), an entitled wallet re-reads free, an unentitled one pays the fresh challenge behind the same price-bump refusal, and the Bazaar lane re-verifies the challenge it actually signs. The policy deny, confirm ceremony, and release shapes move to lib/spend-gate, one gate both verbs run, so buy and pay cannot drift on what --yes clears. Proven live: re-asking a previously bought /api/answer question returned 200 entitled, paid false, via SIWX redelivery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Follow-up commit
|
…cover # Conflicts: # .changeset/skill-materialization.md # README.md # src/commands/config.test.ts # src/lib/errors.ts # src/lib/skill-materialize.test.ts # src/lib/skill-materialize.ts # src/lib/skill-writer.test.ts # src/schemas.ts
…y skill Presence is the whole mechanism, per the owner call that closed the materialization direction: the new tenjin-pay skill (discover, verify, pay, and the lane's safety rules) is on disk exactly while the bazaarPay toggle is on. install places or removes it after the decisions, config set bazaarPay converges every wired skills directory immediately, the self-heal keeps a present copy current, doctor compares it when present and never requires it, and uninstall removes it. The skill-shaping flag machinery this replaces (skillContentFlags wiring, the config-set rematerialize, skill markers in tenjin-search) is gone; the merged marker seam in lib/skill-materialize stays inert, exactly as main pins it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Restructured per the owner call that closed the materialization direction (#147 merged as an inert seam only): the Bazaar lane now teaches through an optional
|
Composes both sides rather than taking either: - skills/tenjin-search/SKILL.md: main moved the permission block out to references/permissions.md (#164 skill diet). Keep the dieted skill and port this branch's `Bash(tenjin pay:*)` opt-in into the reference, which is now the file the OPT_IN_ALLOWLIST coverage test reads. - src/lib/skills-source.ts: main's SHIPPED_SKILL_FILES manifest and this branch's OPTIONAL_SKILL_NAMES both land, and the manifest gains the optional tenjin-pay entry that `uninstall.removeSkills` indexes. - src/commands/config.test.ts: both import lists. Also on the branch's own terms: the install docstring, the question-order test and docs/agent-permissions.md still counted four decisions and two opt-ins after this branch added a fifth and a third.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Merged Three conflicts, composed rather than sided:
Evidence that each side is still live, with the control run:
Two things fixed on the branch's own terms, both counting artifacts this branch invalidated:
Gates run on the merge commit: |
The stored `discover` sweep is pay-time evidence for a foreign 402, so both holes let a listing gate a payment it should not have. - `storedListingsFor` compared `now - fetchedAt < TTL` with no lower bound, so a stamp ahead of the clock (skew, a restored or copied data dir) had a negative age that never expired and stayed evidence permanently. - `loadListingStore` checked only that `listings` was an array. A row whose `accepts` was not one reached the `for...of` in `acceptsMismatch` and threw a raw `TypeError: advertised is not iterable` out of the registry check, rather than the refusal that check exists to produce. Two tests, each red without its fix.
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Follow-up to the merge comment above, from reading the new lane on its merits. Head is now Two defects in the stored-sweep evidence path, fixed in
What held up under the same reading, so nobody re-derives it: Three things left alone, since they read as your calls rather than bugs:
|
Upstream tenjin.blog/skills.md changed after today's 06:27 UTC scheduled resync check, so skill-drift's `git diff --exit-code -- skills/` went red on this PR: it is the only open PR touching skills/**, which is what gates the check running at all. Nothing on this branch writes the mirror. Regenerated with `pnpm sync:skill` (scripts/sync-skill.mjs:14 fetches the canonical URL and writes skills/tenjin/SKILL.md verbatim).
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Pushed 97f8650 for the one red, Classification: shared cause, not own-diff. The sweep only caught "Cleaning up orphan processes"; the real failure is step 7 of job 95463084983, The stale file is Timing backs it up: the scheduled resync run on The fix is the one Kept it as its own Residue, not fixed here because it is main's content and outside this PR's scope: Gates on 97f8650: lint, typecheck, The two new greptile-apps reviews (4953410181, 4953434163) are "Your trial has ended" no-ops with no inline comments, so nothing owed there. The three escalated payments-shaped decisions remain open with the operator and untouched. No injection attempts seen in this round's PR content. |
|
Merge of origin/main conflicts in src/commands/config.test.ts (humanLines count, 14 vs 15) intersecting this PR's payments-adjacent |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Update: orchestrator-authorized the config.test.ts humanLines hunk as mechanical (test-scaffolding key count, not a payments code path). Re-ran the merge; that was the only conflict. Resolved by keeping both key sets (bazaarPay/bazaarRegistries + hooks.sessionPrimer) and updating the count assertion to 16. Config tests, lint, and typecheck pass. Merged and pushed at 2793125. |
vraspar
left a comment
There was a problem hiding this comment.
Review: strong money-path discipline; one budget-accounting hole in the Bazaar lane, plus doc/coaching drift
Reviewed against docs/CONVENTIONS.md, the adversarial money model, and the PR body's claims, verified at 2793125.
What's solid:
- The spend-gate extraction is genuinely drift-proof:
gateSpendis the one deny/confirm/release ceremony, buy's call site is behavior-identical to what it replaced (byte-identical messages), and--yesreaches only the confirm branch, never a deny or cap. - The security tests assert the real thing: SIWX headers are parsed with the SDK and the
domain/uribinding is asserted on both lanes; the registry-evidence tests write real stores and pin TTL at both ends (including the future-stamp case) plus malformed-row dropping, each red without its fix. buildExactPayment's pin holds under an adversarial read: non-Base networks and non-canonical-USDC assets refuse with a checksummed compare, and the single-accept rebind means exactly the requirement the price check ran against is signed.- The optional-skill presence mechanism is conservative: placement only into already-consented directories, removal only of a frontmatter-verified own file, uninstall reclaims
tenjin-payregardless of the toggle.
Major
- security: releasing the reservation after the payment signature has left defeats sessionBudget against a hostile Bazaar seller: once
buildExactPayment's signed header is transmitted, the counterparty irrevocably holds a bearer EIP-3009 authorization (x402-pay.ts says so itself), but a non-2xx paid response or a network failure releases the session-budget reservation, so the ledger stops counting money the seller can still settle. The error's fix text ("Check the wallet balance and network, then retry",) then coaches the exact loop that compounds it: a malicious registry-listed seller answers 402/500 after receiving each signature, the agent retries undertenjin-agent/src/commands/pay.ts
Line 239 in 2793125
--yes, and every round signs a fresh authorization while sessionBudget counts zero of them; worst-case exposure is N x price with no brake but--max-priceper call. Forbuyagainst the configured deployment this release-on-failure shape is fine (trusted counterparty); the Bazaar lane is precisely the case where it is not. Fix: on the pay path, once the signed header has been transmitted, commit the reservation (or record it as spent-pending) instead of releasing on an ambiguous outcome, and soften the paid-leg retry coaching; releasing can stay correct for failures before transmission.tenjin-agent/src/commands/pay.ts
Lines 216 to 246 in 2793125
Minor
-
agent-usability: the coached enable command fails: both refusal texts say
tenjin config set bazaarPay on(,tenjin-agent/src/commands/pay.ts
Line 272 in 2793125
), buttenjin-agent/src/commands/discover.ts
Line 74 in 2793125
parseBooleanaccepts onlytrue/false, so the CLI's own remediation exits USAGE. Fix: accept on/off for boolean keys (the hooks keys already speak on/off) or coachbazaarPay true. -
hygiene/backend: three comments delegate placement-drift reporting to a doctor check that does not exist: config.ts (
), install.ts (tenjin-agent/src/commands/config.ts
Line 238 in 2793125
), and skill-placement.ts (tenjin-agent/src/commands/install.ts
Line 480 in 2793125
) all saytenjin-agent/src/lib/skill-placement.ts
Line 57 in 2793125
tenjin doctorreports a presence that does not match the toggle, but doctor never readsbazaarPay:compareWiredSkillscovers content staleness of a present copy only and "never requires" the optional skill, so a failed removal (toggle off, skill still teaching the lane) and a failed placement (toggle on, no skill) are both invisible. The runtime gate inresolveLanekeeps the lane itself safe either way. Fix: add a presence-vs-toggle line to doctor's skills check, or reword the three comments to say the drift is silent. -
testing: the SIWX-lane fresh-challenge re-verification has no test that reddens on revert: delete
and the suite stays green. The case it alone refuses: a Bazaar seller whose first 402 matches its listing but whose post-SIWX fresh 402 swapstenjin-agent/src/commands/pay.ts
Lines 189 to 191 in 2793125
payToorassetat the same price (the price-bump check passes, only the re-verify catches it). Add that test.
Nits (2), none blocking
- src/commands/pay.ts L404:
opts.amountAtomic!indeliverhides a real invariant behind a non-null assertion; a discriminatedDeliverOpts(paid variant carries the amount) types it away. - Exit-code taxonomy: the tenjin-pay skill's table says exit 3 covers "a registry refusal", but
unlistedand toggle-off refuse with USAGE (exit 2) (). An agent parsing exit 2 as "my invocation was malformed" will re-shape the command rather than runtenjin-agent/src/commands/pay.ts
Lines 357 to 360 in 2793125
discover. Either classifyunlistedas the exit-3 refusal class or note the split in the skill.
Verified, not issues
- USDC-on-Base pin:
buildExactPaymentrefuses foreign chains and non-canonical assets (checksummed), and signs only the single accepts[0] the gate priced. - SIWX confused-deputy:
buildSiwxHeaderbindsdomain(host-with-port) andurito the target origin; the foreign-seller test parses the real header and asserts the binding; the configured deployment's credential cannot leak, and a foreign seller's signature is worthless elsewhere. No SIWX is sent when the 402 does not advertise it (tested). --yesscope: clears the confirm only; deny and--max-priceroute throughgateSpend's deny branch before the confirm is ever reached, identically for both verbs.- Registry TOCTOU: a price raise or payTo rotation after the sweep lands in
acceptsMismatchand refuses; a malicious cross-listing of a victim URL cannot force payment (the victim's live 402 will not match it) and cannot veto a valid listing (first matching listing verifies). - Redirects fail closed on the probe, the re-check, and the paid leg (tested on the probe; one shared
blockRedirectsopts object feeds all three). - Fail-closed lattice: registry check runs before the wallet opens; all-registries-unreachable is
NETWORK_ERROR, wallet untouched (tested); http foreign origins refuse. - Install: headless never enables and persists nothing; both prompted answers remembered; question order pinned by test; the confirm prompt renders the sanitized payee before the price so escapes cannot repaint the amount.
- Local run at head: full
pnpm test2233 passed / 10 skipped with HOME isolated; the 2 doctor.test.ts failures on a raw run are this machine's real~/.claudeinstall leaking into the doctor sandbox (they pass with a clean HOME and are pre-existing tests untouched by this PR). CI green at 2793125 including pack-smoke.
Closed as decisions
- No dedupe on
pay(every paid call pays): deliberate, stated in the PR body, skill, docs, and permission note; the brakes are named. Not re-raised. - Skill-presence-as-mechanism (no conditional content, no markers): owner call that closed the materialization direction; this PR implements it.
- The stored-sweep-as-evidence design (CDP ignores payTo filters, search cannot match URLs): constraint verified live by the author; TTL and shape hardening landed in 9193ea1 with red-without-fix tests.
Verdict: comments-only. The one Major is a real adversarial-money gap in the lane this PR introduces (budget accounting stops at the delivery boundary while the signature has already left); the Minors are a failing coached command, comments claiming a diagnostic that does not exist, and one untested security gate. Ran locally at head: lint-clean full suite, 2233 passed with HOME isolated; environment-only doctor failures explained above; CI green including pack-smoke.
Review 4965895923 fixes: - Major: a transmitted EIP-3009 authorization is a bearer instrument, so every post-transmission outcome now commits the reservation instead of releasing it; only a failure before the payment build releases. The paid-leg failure fix text no longer coaches the retry loop that compounded the gap. Red-without-fix tests on both lanes. - Minor 1: parseBoolean accepts on/off, so the coached `tenjin config set bazaarPay on` now works (tested). - Minor 2: doctor's skills check now reports tenjin-pay presence vs the bazaarPay toggle (both drift directions tested), making the three comments that pointed at it true. - Minor 3: red-without-fix test for the SIWX fresh-challenge re-verification (payTo swap at the same price refuses, nothing signed). - Nits: DeliverOpts is a discriminated union (no non-null assertion); the skill's exit-code table notes the unlisted/toggle-off USAGE split. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Re review 4965895923: all findings addressed in 5317b7f. Major (budget accounting stops at the delivery boundary): fixed as prescribed. Minor 1 (coached command fails): Minor 2 (phantom doctor diagnostic): took the first option and made the comments true instead of deleting them. Doctor's skills check now compares tenjin-pay presence against the Minor 3 (untested re-verification): added the exact case named: a Bazaar seller whose first 402 matches its listing but whose post-SIWX fresh 402 swaps Nits (both taken): Gates at 5317b7f: lint clean, typecheck clean, prettier clean, full |
Verified keyless 2026-08-18 with the sweep's own client (withBazaar(HTTPFacilitatorClient)): GET /discovery/resources answers 200 with the Bazaar envelope, 26,478 listings, type filter honored. Like UltraVioleta it has no /discovery/search (a query sweep reports it in errors), and like CDP it ignores payTo filters (the stored-sweep evidence covers the pay lane). Defaults now pinned by a test; docs updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Operator directive: PayAI's facilitator registry added to the default
|
Mechanical resync only; the live skill moved after a deploy (searchId attribution doc). No hand edits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
576bd69 is the mechanical mirror resync (node scripts/sync-skill.mjs): the live tenjin.blog/skills.md moved after a deploy (searchId attribution doc), only skills/tenjin/SKILL.md changed, and the drift gate's own check (git diff --exit-code -- skills/ after resync) reproduces green locally. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
vraspar
left a comment
There was a problem hiding this comment.
Re-review: the Major is fixed at the transmission boundary and pinned red-without-fix; both Minors and both nits closed; the new default registry is clean
Reviewed at 576bd69, delta over 2793125. Three new commits: the Major fix (5317b7f), the PayAI default registry (0c7043b, operator-directed), and a skills mirror resync (576bd69). Re-verified by running the affected suites and reverting the fix.
Resolved since round 1
- [Major] Reservation committed at the transmission boundary, not delivery.
runPaynow splits the failure path exactly right: abuildExactPaymentthrow (no signature exists) still releases, but once the signed header goes out the reservation is committed on the line immediately afterhttpRequestreturns, before any outcome branching, so a 2xx, a 402, a non-2xx, and a transport failure all count against sessionBudget. Confirmed the load-bearing claim:httpRequestis fully try/finally-wrapped and every internal catch returns{ok:false}— it never throws on transport failure, so the commit always runs; and even a theoretical throw would leak the reservation to its TTL (conservative, never a wrongful release), so the stacking attack cannot recur. The paid-leg fix text no longer coaches "then retry" — it states the signed payment already left, is counted, and each retry signs a fresh authorization. Reverting pay.ts to the round-1 version fails 2 tests, includinga hostile seller rejecting the paid leg still burns the session budget, so both lanes are pinned. - [Minor] Coached enable command works.
parseBooleanacceptson/offalongsidetrue/false, and the USAGE fix names both; the two refusal texts now coach a command that succeeds. Pinned by['on', true]/['off', false]round-trip tests. - [Minor] The phantom doctor diagnostic is now real. Doctor's skills check compares
tenjin-paypresence against thebazaarPaytoggle for every wired directory and warns both drift directions (toggle on + skill missing, toggle off + skill still teaching) with aconfig set bazaarPay on|offre-sync fix — warn not fail, sinceresolveLanekeeps the lane safe. Three tests cover both directions and the matching state; doctor suite passes 86 on a clean HOME. - [Minor] SIWX fresh-challenge re-verification is tested. New test
a fresh challenge that swaps payTo after SIWX is REGISTRY_MISMATCH, nothing signed— the exact case named in round 1: first 402 matches the listing, post-SIWX fresh 402 swapspayToat the same price, passes the price-bump check, and only the re-verify block catches it; red when that block is deleted. - [Nits] Both taken.
DeliverOptsis a discriminated union onpaid, removing theamountAtomic!assertion; the tenjin-pay skill's exit-code line now statesbazaarPayoff and unlisted refuse as USAGE (exit 2) with "run discover, then re-run pay".
New
- PayAI facilitator added to the default
bazaarRegistries(0c7043b, operator-directed) — no new trust bypass.https://facilitator.payai.networkis just another base URL inDEFAULT_BAZAAR_REGISTRIES; the verification path does not branch per registry. Every PayAI listing flows through the samesameResourceUrl(origin+path identity) andacceptsMismatch(scheme/network/asset/payTo exact, live amount ≤ advertised) as the existing sources, so a malicious PayAI listing still cannot verify a payment to a mismatched payTo, and the USDC-on-Base pin inbuildExactPaymentis the final backstop regardless of what any registry lists. Its documented quirks (no/discovery/search, ignores payTo filters) match UltraVioleta and CDP respectively and are already covered by the query-sweeperrorspath and the stored-sweep evidence. Keyless/live verification was done by the author with the exact sweep client (200, Bazaar envelope, 26k listings), and the default list is pinned by a test insrc/lib/config.test.ts. This widens discovery surface, which is the intended operator decision, not a defect.
Verdict
Approve. The Major is fixed at the correct boundary and pinned red-without-fix on both lanes, all three Minors and both nits are closed and re-verified, and the new default registry adds discovery surface without a new trust path. Ran pay/config/discover/doctor suites (212 passing; the 2 doctor failures are this machine's ~/.claude leaking into the sandbox, green on a clean HOME); CI green at head.
| - `excerpt` is a separate listing teaser, NOT the in-page preview. | ||
| - `resource` is the answer card. Compose it here rather than deferring it (a | ||
| merge-update via `PUT` still works later); field list and phrasing below. | ||
| - `searchId` (uuid) is optional supply-loop attribution: pass the `searchId` of an |
There was a problem hiding this comment.
WHy do we need to modify this skill and why does this skill exist. I think this mirrors the tenjin.blog right ? so this would be reverted in next updated ?
There was a problem hiding this comment.
Yes, skills/tenjin/SKILL.md is the vendored mirror of tenjin.blog/skills.md - the hosted skill served to CLI-less agents. It exists in-repo so the packaged CLI ships the same guidance offline and so CI (the skill-drift check) proves the two never diverge. The commits on this branch are resyncs in the live-to-repo direction only: the hosted source moved (twice this week - the searchId attribution doc and the scan additions), and the drift check goes red on any branch touching skills/ until the mirror catches up. Nothing here edits the mirror by hand, so nothing gets reverted on the next update - the next update IS another resync commit like these. The mirror's SOURCE lives in the tenjin repo; content changes happen there.
Scope
src/commands/pay.ts+src/lib/bazaar.ts:tenjin pay <url>is the standard x402 client verb (probe, deliver 2xx free, pay a 402 under the same money gates as buy viabuildExactPayment's USDC-on-Base pin; redirects fail closed, no library dedupe). When the 402 advertises the standard sign-in-with-x extension,payruns buy's sequence: one SIWX re-check whose signature binds to the TARGET origin (never the configured deployment's), an entitled wallet re-reads free, an unentitled one pays the fresh challenge behind the same price-bump refusal. The deny/confirm/release ceremony is one shared gate (src/lib/spend-gate.ts) bothbuyandpayrun, so the verbs cannot drift on what--yesclears. Foreign https origins need the newbazaarPaytoggle AND registry evidence whose terms the live 402 does not exceed (thediscoversweep cache first, 24h TTL, then payTo-filtered live lookup; mismatch = newREGISTRY_MISMATCH, exit 3, before anything is signed; unreachable registries fail closed).src/commands/discover.ts: list/search the configured registries via the SDK'swithBazaarclient (free, keyless, wallet-untouched, works with the toggle off; MCP-type listings counted, not shown) and persist the sweep as pay-time evidence.bazaarRegistriesdefaults to CDP Bazaar + UltraVioleta (both verified keyless 2026-08-14) + PayAI's facilitator (verified keyless 2026-08-18 with the sweep's own client: 200, Bazaar envelope, 26,478 listings; no /discovery/search like UV, ignores payTo filters like CDP).installasks the toggle once (default no, both answers remembered, headless never enables); as a skill-shaping key,config set bazaarPayre-materializes installed skills immediately (stacked on feat(skills): shape installed skill content by machine facts, starting with wallet presence #147).payjoins the opt-in permission tier besidebuy.Testing
pnpm lint,pnpm typecheck,pnpm format:check: clean. Fullpnpm test: 1903 passed, 10 skipped (integration included).🤖 Generated with Claude Code