docs: check in CLAUDE.md, refreshed against the current tree - #100
Merged
Conversation
CLAUDE.md was untracked, so every contributor had their own copy or none. Checks it in, and corrects the parts that had gone stale — its named defects were the ones most likely to be trusted verbatim, and all three were fixed: - R3.7's `consentRequiredFrom` example was fixed (it now matches `details.reason`). Replaced with the rule that actually bites now: a Response body reads once, so an already-parsed body must use `errorFromBody`, not a re-read of the spent Response (#99). - R1.5's "one 2s retry from onClose" applied to the worker inbound path (fixed in #88) and then to the approver inbox (fixed in #97). Replaced with the invariant — cap the delay not the attempt count, re-arm on every failure including first-connect — and a pointer to `ReconnectScheduler`. - R1.2's `handleApiGet`/`handleApiPost` example was fixed in #88 and is now the compliant reference. Replaced with the thing that actually hides these: fetch is injected, so `grep "fetch("` finds almost nothing and the timeout belongs at the injection point (#98). Promotes the one genuinely open defect to its own section: R1.6 persist-before-ack, which is not fixable from this repo — vti-didcomm-js acks before dispatching to `onMessage`, and the wallet persists only the message id, so an offscreen teardown mid-prompt loses a task-consent request for good. Adds a repo-mechanics section for the traps that cost time this week: build `core` before typechecking dependents, lint is `tsc -b` (never `-b --noEmit`, TS6310), cross-workspace imports need a `references` entry, what CI asserts, stub with real `Response` objects, and Node unreffing the `AbortSignal.timeout` timer (passes locally, fails in CI). Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CLAUDE.mdwas untracked — every contributor had their own copy, or none. This checks it in and corrects what had gone stale.Why it needed correcting
Its named defects are the parts most likely to be trusted verbatim, and all three had been fixed:
consentRequiredFrommatches a code the VTA never emitsdetails.reasononClosehandleApiGet/handleApiPosthangs the page for minutesA doc that points at already-fixed bugs sends the next person hunting for them, and — worse — implies the remaining problems are covered.
Each stale example is replaced with the invariant that actually bites now:
Responsebody reads once; an already-parsed body must useerrorFromBody, never a re-read of the spentResponse(that throws into a swallowingcatchand degrades to a status-only guess) — fix: parse error bodies before throwing on status (R3.7) #99onClosecannot fire for a session that never opened; useReconnectScheduler— fix: re-arm the approver inbox with exponential backoff (R1.5) #97fetchis injected, because a literalgrep "fetch("finds almost nothing here — fix: bound every outbound fetch with a timeout (R1.2) #98The one genuinely open defect
Promoted R1.6 persist-before-ack to its own section, since it is the only live one and is not fixable from this repo:
vti-didcomm-jsacks before dispatching toonMessage, and the wallet persists only the message id — so an offscreen teardown mid-prompt loses atask-consent/requestpermanently. Needs a library change or disabling auto-ack, and affects pnm-relay too (R4.1).New: repo mechanics
The traps that actually cost time this week, so they cost the next person nothing:
corebefore typechecking dependents — each workspace typechecks against gitignoreddist, and a stale one produces phantom errors in correct sourcetsc -b, nevertsc -b --noEmit(TS6310 when a referenced composite project must emit)referencesentryimport()invariantResponseobjects, not{ ok, json }literalsAbortSignal.timeouttimer — passes locally, fails in CI as "Promise resolution is still pending"Verification
Docs only — no code changed. CI runs anyway; lint/build/test are unaffected.
Worth a read rather than a rubber-stamp: it now asserts things about the codebase, and I'd rather you catch anything I've overstated.