feat(pwa-demo): standalone local-first PWA demo with guarded release tooling - #35
Open
johnnyrobot wants to merge 61 commits into
Open
feat(pwa-demo): standalone local-first PWA demo with guarded release tooling#35johnnyrobot wants to merge 61 commits into
johnnyrobot wants to merge 61 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Worker sets allow_fallbacks: true; freeness is enforced by the :free model list and zero max_price, not by disabling fallback. Only the ai:evaluate script sets allow_fallbacks: false. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rule that no secret reaches a child process was written seven times across the release scripts: four byte-identical 20-key lists, plus three 17-key variants that deliberately retain Cloudflare credentials so Wrangler can authenticate. Nothing recorded that the shorter lists were intentional, so each copy read as a possible oversight. scripts/child-environment.mjs now owns the key list and names the one axis that varies: childEnvironment() denies every secret, and wranglerChildEnvironment() keeps exactly the three Cloudflare credentials Wrangler needs. The functions that add to the scrub (releaseGateEnvironment, postdeployEnvironment, wranglerReadOnlyEnvironment) delegate the scrub half and keep their own augmentation; the pure pass-throughs are deleted. No behaviour change. The two inline lists omitted WRANGLER_OUTPUT_FILE_PATH and assigned it on the next line, so scrubbing it first is inert. Secret scrubbing is now asserted once, in child-environment.test.mjs; the four test files that duplicated that assertion narrow to what their own function adds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The build validator, the Worker-bundle validator, and the production verifier each carried their own copy of the three secret shapes. The copies had already drifted: only the build validator's patterns were global, forcing a `pattern.lastIndex` reset in the shared file scanner so a stateful regex could not skip a later file. `scripts/secret-scan.mjs` now owns `SECRET_PATTERNS` and the single verb that answers whether a text carries a secret. All three checkpoints feed it their bytes; `scanFilesForSecretPatterns` no longer accepts caller patterns, so no caller can hand it a global regex and the reset deletes itself. Its test now scans for a real private key block rather than a stand-in marker. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The guarantee that the demo can only ever reach free, zero-retention models was asserted in four separate strata of worker/index.ts: the model-list parser, the provider block built per request, and two response validators several hundred lines away. Nothing tied them together, and the model list was parsed twice per request. worker/free-routing.ts now owns both directions. buildFreeRouting names the chain and the provider constraints; assertFreeRoutingHonoured re-checks the model and the reported cost of what came back. The routing is built once, before the daily quota is reserved, so a misconfigured model list still fails closed without spending an attempt. ApiError and the JSON record helpers move to their own modules so the policy module can throw the same errors without importing the Worker entry point back into itself. Provider fallback stays permitted; freeness is enforced by the :free model list plus a zero max_price. The single-attempt stance was already guarded by the upstream-error tests and is now stated in the module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The wire protocol between the Worker and the DailyAiQuota Durable Object was written four times: the caller built the request body, the object re-stated the same field constraints, the caller re-validated the response shape, and the in-memory test double reimplemented the reservation decision with the five-per-day limit hardcoded. worker/quota-protocol.ts now owns the request shape, the response predicate, and the decision itself. The Durable Object applies that decision inside a storage transaction; the test double applies it over a Map. Only the storage differs, so the double can no longer drift from the object it stands in for. Making the double faithful surfaced a gap it had been hiding: it never validated expiresAtMs, so no test exercised the expiry rule the object actually enforces. The clock is now an explicit parameter of the double and each suite pins it to the same instant its caller uses. The object still stores only an HMAC-derived install ID, the day, an attempt count, request IDs, and an expiry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three AI surfaces each re-derived what a caller must do next. Both panels read the session marker directly and repeated the rule that a 401 or 403 discards the session; the consent gate derived its stage from the disclosure flag alone. src/lib/ai/session-readiness.ts now answers that question once, as ready / needs-disclosure / needs-challenge, and owns the re-sync rule. An established session deliberately outranks a cleared acknowledgement, which preserves the previous behaviour of every caller exactly. Scoped deliberately short of the architecture review's proposal. That called for readiness to ask the server rather than remember, but the authority is an HttpOnly __Host- cookie the browser cannot read and the only session route requires a Turnstile token, so asking would mean a new unauthenticated endpoint reporting cookie validity. That is a security-surface change, not a refactor, and the benefit it buys is one avoided challenge in a second tab. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The courses/ area already held this seam: the route screen reads and writes, and the views below it render from props. Three components elsewhere reached past it. ApprovalActionPanel wrote a workflow transition from inside the queue; ProgramCourseBuilder, a grandchild of the program editor, both read the course list and wrote requirement reordering; ProgramView read its own aggregate and references. Each now takes what it needs as props, and the screen above it owns the repository call. The cost of the old shape was visible in the tests: all three mocked ../../lib/data to render at all. ApprovalActionPanel and ProgramCourseBuilder no longer mock it, and ProgramView is driven entirely by props. The wiring that moved into ProgramViewRoute gained tests for the department lookup, the error pass-through, and the combined loading state. RegistrarDashboard was listed in the architecture review but needs no change: it is rendered directly by src/app/dashboard/page.tsx, so it is the route screen, already at the seam. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every record a release attempt reads or writes had its path baked from process.cwd() at import time, so the thirteen functions that decide whether a half-finished publish can be recovered could not be exercised against anything but a real checkout. Twelve of them had no test at all. releaseRecordPaths(cwd) now resolves the whole set from one directory, and each record function takes an optional cwd. Callers that pass nothing behave exactly as before. The new suite covers what was previously unverified: a pending attempt can be claimed only once, an upload records its version against the same attempt and refuses a changed one, a reconciled attempt archives read-only, a cancellation refuses to discard an attempt that already uploaded a version, ownership is sealed at 0400 while the current deployment stays replaceable, and the history appends each version once and refuses a malformed file. This is the safety net the wider release-target facade would need. The facade itself is not attempted here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scanFilesForSecretPatterns kept "Patterns" from a signature that once took them as a parameter. The patterns now live in findSecrets, so the name described an argument the function no longer has. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
docs/agents/domain.md told the engineering skills to read CONTEXT.md and docs/adr/ and then to proceed silently when neither existed, while illustrating the layout with two ADR filenames that were never written. Both now exist and the example names the real one. CONTEXT.md fixes the vocabulary the other prose files assume rather than restating them: the curriculum terms of art, the workflow nouns, the local-first runtime terms, and the AI and release terms that security review depends on. ADR-0001 records why AI session readiness is remembered client-side. The architecture review asked for it to be derived by asking the server; the authority is an HttpOnly __Host- cookie the browser cannot read, and the only session route is the Turnstile challenge itself, so asking would mean a new unauthenticated endpoint reporting cookie validity. That reasoning lived only in a commit message and would have been re-litigated by the next review. Neither file is a release input: RELEASE_INPUT_ROOT_FILES allowlists root files explicitly and docs/ is not a release input directory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Clicking a breadcrumb, an internal link, or a Navigation API intercept were covered. The paths no click reaches were not: beforeunload, pagehide, legacy popstate, unmount, and the shell-wide flush that Settings and the PWA updater call before destroying local data. Those are the paths that lose a draft when they regress, and they are the ones a change to where the draft session lives is most likely to drop. Pinned against current behaviour, including that a clean editor neither saves nor warns, that a failed save reports false so the shell refuses to reset, and that nothing saves after unmount. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CourseEditor held the working copy, the autosave debounce, the in-flight save coalescing, the crash-recovery mirror, the cross-tab conflict detector, and six ways to save without a click, interleaved with the tab strip, the comment form, and the submit button. Reading any one of them meant reading all of them. useCourseDraftSession now owns that, and the editor asks it for a value to render and four things it can do: change, flush, navigate, and export. The component drops from 1065 lines to 614 and no longer names a single ref. Two things the split made visible rather than changed: The conflict check is broader than the exit paths'. It treats an in-flight save as pending work, because a save that has not returned may still be about to write; the exit paths ask the narrower question, because an in-flight save is already doing the work they would start. That difference was previously two similar expressions 200 lines apart and is now a comment at the one place it matters. A failed submission and a failed save shared one state variable. They are now separate, rendered through one alert, because the submit path cleared the save message at a point where a successful flush had already cleared it. The submission failure had no test at all; it has one now. course-draft-session.test.tsx covers the session directly, which the extraction is what makes possible: recovery entries written for another course or unreadable, the saved-to-idle window, save coalescing, the conflict refusing to save, routing withheld when the save fails, and the unsaved-draft export. 517 -> 531 unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
publishWorker and promoteUploadedVersion could not be exercised without a Cloudflare account. They were not, as an earlier note claimed, calling spawn directly — they call runPausedPublisher, which has taken a spawnProcess parameter all along. What they did not do is accept one to forward, and they also called the two preconditions, the child environment builder, and readFile as module-level bindings. All of that is now one WRANGLER_PUBLISHER value, substituted as a single argument rather than six threaded parameters, because it is one concept: everything the publish steps do outside this module. The tests that become possible are about order, which is what the sequence exists to get right. Neither command runs before both preconditions pass. The deploy does not run until the uploaded version id has been handed back for the pending record, so a publish interrupted between the two is recoverable rather than lost. A non-zero upload stops before the deploy; a non-zero deploy still leaves the upload recorded. A promote deploys the version it was given and never uploads. This is the prerequisite for collapsing the release target's surface, not that collapse. 136 -> 148 script tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pending record is the only thing that makes a half-finished publish recoverable, so who writes it and in what order is the safety property that matters. Both the publish path and the recovery path built and mutated it inline: an eighteen-field literal in one, and four places that assigned versionId, versionTag, and origin and then called update, which a reader had to match up by eye to see they were the same operation. ReleaseAttempt owns it. open builds the record and makes it durable before the first mutating Wrangler command; recordUpload binds the attempt to the version Wrangler returned and persists immediately, because it is called from inside the publish between the upload and the deploy, and that write is what turns a lost publish into a recoverable one; finalize promotes and archives. Recovery resumes the same object, so the reads that follow still see what was written, as they did when the mutations were inline. No behaviour changes. The record layer's own guards are unchanged and still reject a second attempt over an unreconciled one, a rebind to a different version, and a move to a different origin - now demonstrated rather than assumed, since ReleaseAttempt takes cwd and can be driven in a temporary tree. The attempt lifecycle had no tests. It has ten. 148 -> 158 script tests, 541 -> 553 overall. Not done, and not on the report's terms: the 36 exports of cloudflare-release-target.mjs stay. Thirteen of them are pure parsers and validators with direct tests, and hiding them behind publishRelease / recoverAttempt / cancelIfUnchanged would trade that coverage for a shorter name list on a path that still cannot be executed once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Both documents this work started from live at OS temp paths, which do not survive a reboot. The one candidate still open is C5, and its problem statement existed only inside a generated HTML report there. The handoff carries C5 forward with its sites, its proposed shape, and the two numbers in the report that no longer match the code, plus the findings and decisions that would otherwise have to be rediscovered. Everything already recorded in a commit message or ADR is referenced rather than repeated. docs/ is not a release input: RELEASE_INPUT_ROOT_FILES allowlists root files explicitly and docs/ is not a release input directory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A call-site census over src/ and e2e/ found fourteen of the interface's forty-seven methods with no production caller. They were not a uniform kind of dead weight: updateCourse, replaceCourseSLOs, replaceCourseContent, replaceCourseRequisites, setCourseCCNJustification reimplemented, line for line, work saveCourseAggregate already does for the same fields in one transaction. Keeping both meant two copies of the TOP/CB03 canonicalization, the requisite cycle check, and the CCN non-match validation, either of which could drift from the other. getRevision, subscribe forwarded to the invalidation bus the caller can reach directly. useRepositoryRevision now subscribes to that bus, which is the same shared instance the repository was handing it back. listComments, deleteComment, deleteProgram, getAIConversation, deleteAIConversation, listAIArtifacts, deleteAIArtifact had no caller at all. Comments already arrive on the course aggregate, so listComments duplicated a read the screens get for free. No behaviour is dropped that a caller could observe. The tests that drove the removed course verbs now drive saveCourseAggregate through named helpers, so post-approval immutability, TOP inference, CB03 agreement, requisite cycles, and CCN adoption stay pinned to the same assertions — on the one implementation that survives. Note for anyone reading the numbers: an earlier census missed getRevision/subscribe because hooks.ts calls them as `curriculumRepository.subscribe.bind(...)`, with no paren after the name. Grep for the bare member, not the call. repository.ts 2291 -> 1900 lines; the interface 47 -> 33 methods. 553 unit tests pass; repository.ts coverage 96.2% lines / 88.67% branches against its 90/85 floor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`database` was a public field on DexieCurriculumRepository, so the test file reached tables directly in 21 places while the interface it was meant to be testing sat unused beside it. The field is now private and one documented accessor, unsafeDatabaseForTests, hands it back. The name is the point: reaching a table is not a normal thing to do, and a reader can tell at the call site that a test chose to go around the interface. Eleven of those call sites had to keep the seam. They arrange states no public verb can produce — a schemaVersion 1 record to migrate from, a newer schemaVersion the repository must refuse, a course row forced into each of the five statuses so the workflow matrix can try every edge from it, and bulk fixtures that exceed the conversation and artifact prune caps. Adding read verbs to serve those would have widened the interface to narrow the test surface. The accessor is a method rather than the free function first sketched for it: a function outside the class cannot read a private field without a cast, and the method needs none. Production is unaffected either way, because `curriculumRepository` is exported as CurriculumRepository and that interface does not declare it. 553 unit tests pass; lint and typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Writes were parsed; reads were not. Every row the repository returned was trusted because some earlier version of this code had parsed it on the way in — which is exactly the assumption a local-first demo cannot make. A browser holds records seeded by an older build, restored from a backup, or left behind by a migration, and a field that has since changed shape would surface as undefined somewhere down a screen rather than as an error at the boundary. CourseAggregateSchema and ProgramAggregateSchema compose the part schemas that already existed, and the two private aggregate funnels parse through validateOnRead. Both getCourse and getProgram go through those funnels, and so do the aggregates returned from saves, so a write that produced a record the schema rejects cannot report success either. getDashboard validates only recentActivity. Its counts are derived in the method from rows it just read; recentActivity is the one part it hands a screen unchanged. List and count paths are deliberately left alone. listCourses reads every course row on each query and re-runs on every revision bump, so parsing there would put a Zod pass on the hot path to catch a record that getCourse will catch when the user opens it. A failure is reported as invalid-backup with the parse issues attached and a message naming the recovery — reset the demo data or restore a backup — because that is what a corrupt local record actually needs. 554 unit tests (one new: a units field stored as a number, caught through getCourse, through the program join, and a non-UUID actor key caught in the dashboard's activity). repository.ts coverage 96.22% lines / 88.72% branches against its 90/85 floor. The AppShell coverage-run failure noted in the 2026-08-02 handoff happened once more here and passed on rerun, unchanged. Third occurrence, still environmental. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CurriculumRepository was one flat list. Reading it told you the table layout, not what any caller does with it, and the deep verbs — the one transactional course save, the workflow transition, the backup round-trip — sat between listNotifications and getReferences with nothing marking them as the ones that carry weight. It is now composed from six roles named for the caller that needs them: CurriculumReads, CourseAuthoring, ProgramAuthoring, NotificationInbox, AIPersistence, DemoAdministration. The union is unchanged, so curriculumRepository and every existing call site keep working; what changes is that a module can now ask for the role it uses. Two already do. chat-persistence.ts had hand-rolled `Pick<CurriculumRepository, "listAIConversations" | ...>` before the roles existed, which is this refactor done ad hoc for one module; ai/persistence.ts now names getActivePersona plus artifact writing as what it depends on, instead of importing the whole repository to use two verbs. AIPersistence was one role until the chat-persistence doubles rejected it: those tests build a three-method conversation double, and demanding saveAIArtifact of them would have meant padding a double with a method its subject never calls. Splitting into AIConversationPersistence and AIArtifactPersistence, with AIPersistence extending both, is the seam the callers actually wanted, and the doubles stay faithful. Comment verbs sit in CourseAuthoring because this demo only comments on courses. setActivePersona sits in DemoAdministration because there is no account here — choosing who you are is a demo control, like reset and backup, not authentication. Route screens still import the union singleton. Splitting them across two or three role imports each would fragment the one entry point they have for no gain the roles do not already give a reader. 554 unit tests pass; repository.ts coverage 96.22% lines / 88.72% branches; lint and typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The review that produced C5 asked for the interface to collapse into six aggregate verbs. A call-site census showed that premise was wrong, and the work went a different way. Without a record, the next review reads the same 33-method interface and proposes the same collapse. ADR-0002 carries the census result, the four changes that were made instead, and the six alternatives rejected along the way — including the two that look like obvious improvements until you check what the tests need to arrange. It also records two things found by doing the work rather than planning it: a test double refusing a role is evidence about the role, not an obstacle to route around; and saveAIArtifact is write-only, which the deletion exposed rather than caused. npm run verify exit 0 at this commit: 554 unit tests, 174 worker tests, build, worker dry-run, and the chromium smoke suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 2026-08-02 handoff carried three errors, each re-verified against the code before being corrected here. The scope-invariant command is the one that mattered. Both its pathspecs are cwd-relative, so run from `calricula_pwa_demo/` — the working root that same document declares — the exclude matches nothing and it lists all 267 demo files. It reads as a catastrophic boundary violation when nothing is wrong. From the repository root it is correctly empty. CLAUDE.md carried the same defect and would have misled every future agent; AGENTS.md already said "run from the repository root" and was always correct. Both now explain why the cwd matters rather than just asserting the location. The `repository.database` assertion count was understated seven-fold: the handoff said three, citing 79/266/313, because those were the lines its source report happened to name. There were 21, counted from f451f02:src/lib/data/repository.test.ts. That understated the C5 estimate. Loose end #1 was already closed when written — `docs/handoffs/` does appear in docs/agents/domain.md:27, added by the same commit that added the handoff. Corrections are marked in place rather than rewritten away, and an errata section sits above the body so a reader hits it before trusting anything. The C5 sections are marked superseded and pointed at ADR-0002, since C5 was not built in the shape that document proposes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Records the work between the current green tree and a public beta, so the sequence is not re-derived next session. Three gaps this names, each verified against the code rather than carried from a prior handoff: Live-model qualification covers 2 of 7 AI task routes. ai-evaluate.mjs runs one plain and one structured fixture per candidate (FIXTURES_PER_CANDIDATE = 2), while worker/index.ts enforces seven per-task validators, three of them demanding: content-outline requires generated contact hours to sum exactly to the supplied total, top-code requires the title to match the server-owned catalog character-for-character, compliance-explanation requires citations from a server-owned pack. A model can pass both current fixtures and fail those on every request. The stubbed-provider contract for all seven routes already exists in tests/worker/ and worker/index.test.ts; what has never been tested is whether a real free model can satisfy it. The AI artifact table is write-only by construction, not by oversight. persistence.ts is the only caller of saveAIArtifact, and 6834d95 removed listAIArtifacts and deleteAIArtifact from the interface as unreachable, so nothing can read or delete what a beta would accumulate. Nothing turns a user report into a reproduction. The privacy design forbids logging prompts or responses, so triage has only the error code, the status, the requestId already carried to AIRequestError, and what the user was doing. The design constraint that shapes the rest: MAX_DAILY_ATTEMPTS = 5 per install per UTC day makes a seven-route canary through the deployed Worker impossible. So qualification calls OpenRouter directly with the maintainer key, bypassing the Worker and its quota, and the deployed canary stays at the two requests AGENTS.md fixes it at. Each tier answers the question it can afford to. The plan is proposed, not approved or started. It changes tracked src/lib/** from 46 to 47, which AGENTS.md and HANDOFF.md publish as a provenance check; the task that adds the file updates both in the same commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Defines the seven AI task fixtures the Worker exposes, each with a
deterministic rubric. Every check is tagged workerEnforced:
- true mirrors a validator in worker/index.ts, so failing output would be
rejected in production as UPSTREAM_INVALID_RESPONSE
- false is a hallucination or quality heuristic deliberately stricter than
the Worker, which cannot enforce a prompt-level expectation
scoreFixture stays the conjunction of all checks, so pass.rate === 1 remains
the strict deployment bar; workerEnforcedVerdict exposes the provable subset
for the Worker parity test.
Shapes are mirrored from worker/index.ts rather than from the plan sketch:
content-outline topics are {sequence, topic, contactHours, relatedSloNumbers}
with consecutive one-based sequencing, and compliance citations are
{sourceId, supports} with required recommendations and humanReviewRequired.
A sibling .d.mts is required because tsconfig sets allowJs: false and the
parity tests import this module from TypeScript.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ai:evaluate qualified a candidate on two fixtures: plain chat and a two-item SLO array. A free model could pass both and still fail the other five Worker output validators on every request, reaching users as UPSTREAM_INVALID_RESPONSE. The evaluator now loops the fixture registry, so FIXTURES_PER_CANDIDATE rises 2 -> 7 and MAX_GENERATION_REQUESTS 8 -> 28. Requests are paced by REQUEST_SPACING_MS so a four-candidate run cannot trip a free-tier rate limit and score a model as failing for a reason unrelated to its output. Routing policy is unchanged and still byte-identical: allow_fallbacks false, data_collection deny, zdr true, zero max_price, temperature 0, no retries. Results now carry pass.byTask and per-task check IDs. runCli takes an options bag so the no-content-on-stdout guarantee is testable without a live network call or a real 18-second pacing delay. ai-eval-samples.mjs holds one accepted and one rejected output per task, shared with the Worker parity test so the evaluator's idea of passing output and the Worker's cannot drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The rubric decides whether a candidate model is fit to deploy. If it accepted output worker/index.ts rejects, a model would pass qualification and then fail every real request as UPSTREAM_INVALID_RESPONSE. Two independent drift risks, two tests. Constant drift: the .mjs fixtures duplicate AI_TOP_CODE_CATALOG and AI_COMPLIANCE_SOURCE_PACK because a plain Node script cannot import the Zod-bearing module. A jsdom test now deep-equals both. The eval pack gains url and checksum so that comparison is total rather than a subset. Verdict drift: a Workers-pool test pushes one accepted and one rejected sample per task through the real handleRequest and compares verdicts. Only the checks tagged workerEnforced are compared -- the rubric-only checks are deliberately stricter than any validator can be, so comparing the whole rubric would fail by construction. Verified to have teeth: loosening hours-sum-exact makes it fail. The Worker keeps a third, module-private copy of both catalogs. It is pinned behaviourally: every eval catalog code and every source ID must survive the Worker's own comparison, and a code outside the catalog must not. Tracked src/lib/** rises 46 -> 47; AGENTS.md and HANDOFF.md publish that count as a provenance check, so they move in the same commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… budget release-evidence hard-asserted pass.plain and pass.structured, so it would have rejected every result the new evaluator produces. It now requires a boolean verdict for all seven task routes and refuses a release unless each one is true -- a rate of 1 alone can no longer stand in for the per-route verdicts, and the superseded two-fixture shape is rejected outright. AGENTS.md now states both budgets and which one may grow. ai:evaluate qualifies each candidate on all seven routes at 28 requests maximum; the deployed canary stays at exactly one text and one structured request, because MAX_DAILY_ATTEMPTS = 5 per install per UTC day makes a seven-route canary impossible and rotating sessions to evade that is prohibited. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The aiArtifacts table had a write path and, since 6834d95 removed listAIArtifacts and deleteAIArtifact as unreachable, no read or delete path. MAX_AI_ARTIFACTS_PER_ACTOR silently discarded the oldest rows. A beta would have accumulated real curriculum text a user could neither see nor remove. That is not an audit trail. Removed end to end rather than at the call site alone: both component callers, the persistence module itself, saveAIArtifact and pruneAIArtifacts on the repository, and the AIArtifactPersistence and AIArtifactQuery contracts. The panel's onAccepted prop goes too -- it existed only to feed this write, and leaving a prop with no caller would recreate the same unreachable interface this commit is removing. The Dexie table stays declared. Dropping it is a schema migration and existing installs may hold rows; it simply stops growing, and the backup round trip still carries it. Re-adding a read path later is a feature with a UI and its own accessibility and e2e coverage, and should be planned as one. Tracked src/lib/** falls 47 -> 45; AGENTS.md and HANDOFF.md move with it. The vitest threshold entry for the deleted file is removed -- no surviving file's floor is weakened, and coverage still exits 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The server logs no prompt, response, or curriculum text by design, and the quota Durable Object stores only an HMAC-derived install ID, a day, an attempt count, request IDs, and an expiry. Triage therefore runs on the error code, the HTTP status, the request ID, and what the user was doing -- and the first and third only reach a report if the failure state puts them on screen. The alert region rendered the message and the retry hint but neither of those values. Both reachable AI surfaces now render the pair: AISuggestionPanel and AIChatPanel, via one shared ErrorDiagnostics component rather than two copies. A browser-side rejection reports AI_OUTPUT_REJECTED so a report points at the right side of the client/Worker boundary, and the request ID line is omitted when the failure carries none. The raw upstream message is still not surfaced. Rendered inside the existing role=alert region, so a failure is announced once, in gold-ink (#7E6018) because this is small text on parchment. axe passes on all six scanned routes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Twenty-seven error codes existed and a requestId already flowed Worker -> envelope -> AIRequestError, but there was no documented path from a beta user's report to a reproducible local failure. The runbook is superpowers:systematic-debugging specialised to this system's one hard constraint: the server cannot tell you what happened, because it is designed not to know. Triage therefore runs on four signals only -- code, status, request ID, and what the user was doing. Contains the intake block (and what never to ask for), the complete 27-code hypothesis table, a four-rung reproduction ladder ordered cheapest first, the model-delisted procedure written out as steps because it is the most likely real beta incident, and the rule this project keeps re-learning: "it passes on rerun" is evidence for a timing defect, not against one. Verified rather than asserted: every referenced path exists, every quoted command was run and behaves as described, and the code table was diffed against worker/index.ts -- it covers every Worker code with no extras. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cker Observed while verifying the public-beta work: test:e2e:full fails about once per 84-test run and the failing test moves between runs and browsers. It reproduces at cebba8a in a clean worktree, before any of this work, so it is pre-existing rather than a regression. Recorded rather than absorbed, because test:e2e:full is step 11 of 14 in RELEASE_GATE_STEPS and the gate seal is only written after every step passes -- so this will intermittently block Phases 2 and 4. Deliberately not called environmental. One observed failure is a plausible product behaviour: after "Close editor" the URL was the approved source course rather than the draft just created. Treating "it passes on rerun" as proof of absence is what let the AppShell defect survive three handoffs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test:e2e:full failed about once per 84-test run with a different test and browser each time, which made it read as environmental. It was two real defects, both reproduced at cebba8a before the public-beta work. Root cause 1 -- worker oversubscription. run-release-e2e.mjs used Playwright's default worker count, ceil(cores/2) = 6 on a 12-core host. That heuristic assumes one light browser; this suite drives three projects per group, each worker running a full browser that spawns several processes. Measured load average hit 21.1 on 12 cores and the slowest test stretched from 7.8s serial to 24.1s, against a 30s default timeout. With ~6s of headroom, ordinary scheduling jitter pushed some test past the deadline about half the time. browserProjectWorkers() now divides cores by three, which returns the slowest test to about 10s. No timeout was raised and no retry was added: the deadline is unchanged and the tests do the same work -- the contention blowing through it is what is gone. Root cause 2 -- navigating before an async write committed. resilience-a11y.spec.ts switched persona with a bare selectOption and immediately called page.goto("/approvals/"). setActivePersona is an async IndexedDB write, so the docket could render under the previous persona and the awaited approval card would never appear -- a 30s hang independent of load. workflow-depth.spec.ts already had the correct helper, which waits for the AppShell live-region announcement; it moves to e2e/helpers.ts as switchPersona and both specs now use it. Verified: 5 of 5 consecutive full runs pass on a settled machine, from 2 of 3 failing before. The worker cap is unit-tested. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AISessionDataSchema is strict and declared only expiresAt, but the Worker's
session response also carries remainingDailyAttempts. validateAISessionData
runs unconditionally on every session response (src/lib/ai/client.ts), so it
threw AIOutputValidationError every time and establishAISession could never
resolve. No AI feature could start in a real deployment.
Nothing caught it. The Worker suite asserted the response shape but never ran
the browser validator over it, the schema unit test only passed { expiresAt },
and the E2E suite forces AI_ENABLED=false so the session path never executes.
It fails closed, so it was a broken-demo bug rather than a security hole -- but
it would have surfaced as a dead AI surface on the first real beta request.
Adds the field, bounded by the quota protocol's own invariant and optional so
an older deployed Worker still validates, and closes the seam that hid it: a
parity test now pushes the real session response and all seven real task
responses through the browser validators. Verified to have teeth -- removing
the field again fails it.
Found during the D1 security review of b446c76..4751437.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two defence-in-depth gaps found during the D1 security review. Neither was exploitable in the guarded release path, so neither was filed as a finding, but both contradict invariants AGENTS.md states plainly. 1. Three spawn sites bypassed the shared scrubber and forwarded raw process.env to third-party children -- verify-fresh-checkout.mjs (including npm ci, which runs package install scripts), run-release-e2e.mjs (browsers and wrangler dev), and local-production.mjs. The gate already hands them a scrubbed environment, so only a manual invocation with secrets exported would have leaked, but childEnvironment exists so that one edit protects every spawn site and these were the three that opted out. There are now no raw process.env spawn sites left. 2. release-deploy.mjs sealed the plaintext secrets copy and only opened the try/finally that removed it several awaits later, so a throw from the lifecycle lease, evidence invalidation, or attempt open left all three Worker secrets on disk. Rather than reorder the statements and rely on the next editor preserving that order, withSealedReleaseSecrets binds the lifetime to a callback, which makes the mistake unrepresentable. The cleanup-on-throw guarantee is directly tested; the environment scrubbing is asserted against a payload containing all three Worker secrets plus a Cloudflare token, and confirms an ordinary PATH still reaches the child. npm run verify exits 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The diff scan started on 2026-08-04 over 43 commits. Records the range actually under scan, marks the row open until a report exists, and clears the stale claim that the scan never started. Adds an "Automated pre-scan review" section recording what an automated review of the same range found, so the scan has a prior and any discrepancy between the two is visible rather than silent. It closes no finding and does not substitute for the scan. It names the three defects fixed inside the scanned range and the two hardening items deliberately left open, which the scan may reasonably disagree with. This commit carries documentation only; f0c5854 remains the last commit with code in the scanned range. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 54-Hour Rule had three implementations. src/lib/compliance/hours.ts is the one golden-parity.test.ts pins to the parent Python service; the other two were literals in component files, and the one that won was the one the parity test could not reach. course-draft-session.ts computed totalStudentHours as a sum times a literal 18 and wrote it through saveCourseAggregate on every hours edit. It used Number(x || 0), so an unreadable value from a recovered or imported draft persisted the string "NaN". It now calls calculateTotalStudentLearningHours, whose toFiniteNumber yields 0. The field names differ between CourseViewModel and WeeklyHoursInput, so the call site maps them rather than widening the compliance interface to suit one caller. CourseEditorSections.tsx divided by a literal 54 and rendered a ComplianceMark whenever |total/54 - units| exceeded 0.25. The parent service explicitly warns against exactly this (compliance_service.py:72-76): 54 is a district convention for an 18-week term, not the regulatory minimum, and "compliance must be evaluated against the 48-hour minimum, not against an exact 54-hour equality". The 0.25 tolerance was borrowed from the 48-hour minimum check, where the parent applies it one-sided. The banner is now a neutral reference readout with no verdict; UNIT-005 and the Section VI audit, which do mirror the parent, are unchanged. hours.ts is not modified. It was already deep and already correct; only the call sites needed to stop working around it. Tests: the NaN case is pinned at the useCourseDraftSession seam; the readout is pinned to carry no pass/warn verdict; golden-parity now asserts calculateTotalStudentLearningHours by name, so a change to its delegation cannot silently unpin the editor's path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ADR-0003 captures the two decisions behind 460aeeb: compliance arithmetic has exactly one implementation, and the 54-hour figure is a reference that must never drive a pass/fail verdict. The second is the load-bearing one. The editor had been rendering a ComplianceMark against a two-sided 54-hour equality test, which the parent Python service explicitly warns against — 54 is a district convention for an 18-week term, not the regulatory minimum. On a regulated public tool that presented a local convention to faculty as a Title 5 result. Recorded as an ADR rather than enforced by a lint rule or a grep-style test: both were considered and rejected as brittle, and neither can explain the reasoning to the person who would otherwise re-add the check. The rejected alternatives are written down for the same reason ADR-0002 wrote down its own — so the next architecture review does not re-propose them. CONTEXT.md's 54-Hour Rule entry gains the distinction it was missing. Its claim that hours.ts is the only implementation was false when written and is now true. Committed separately from the code so that reverting the change does not silently revert the reasoning that says not to re-add it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
f0c5854 claimed every spawned child's environment goes through childEnvironment(). Two sites never did, and the security diff scan over b446c76..f0c5854 reported clean without catching either. run-lighthouse.mjs passed {...process.env, CHROME_PATH, CI} to the Lighthouse child, which launches headless Chrome — the widest third-party surface in the release path. The file did not import child-environment.mjs at all. release-inputs.mjs called execFile('git', ...) with no env key, so Node inherited the parent environment wholesale. assertTrackedReleaseInputs is the first thing release-deploy, release-gate, release-evidence and verify-fresh-checkout each do. Neither is exploitable: git and Chrome are not the threat model. Both contradicted a stated invariant, which is the same reasoning f0c5854 applied to the three sites it did fix. Both sites were also untestable, which is why the gap survived. Each now has the seam the other four wrappers already have: lighthouseChildEnvironment is a named export asserted to deny every CHILD_SECRET_KEYS entry while keeping PATH, CHROME_PATH and CI; assertTrackedReleaseInputs takes an injectable execute on its existing options bag, so a test can observe the environment its git children actually receive. grep for '...process.env' and 'env: process.env' under scripts/ and worker/ now returns nothing. Not done here: the nine duplicated stdout/stderr capture blocks with five different overflow policies, and the consolidation of all fourteen sites behind one spawn module. That touches release-deploy.mjs, whose core is unreachable by tests and only runs against real Cloudflare, and is queued as its own candidate rather than bundled with a security correction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The security diff scan over b446c76..f0c5854 reported clean. That is recorded in the executive status, the status table, and the scan-state section, which until now all described it as still running. Three corrections go with it. The claim that every spawned child's environment goes through childEnvironment(), with no env: process.env spawn site remaining, was false when written and stayed false through f0c5854. It is marked as a dated correction rather than quietly rewritten, because the previous text is what the scan was given as context. The clean result is qualified. The scanned range contained two unscrubbed spawn sites and the scan did not report them, so a clean diff scan is evidence about the findings it looked for and not proof that a stated invariant holds. Invariants get checked against the code; e683986 now checks these two with tests. A second diff scan is owed over f0c5854..HEAD and is recorded as an open row rather than left implicit, with the attention list started. One batch scan at a stable head, not one per commit. Deferred: the handoff is still stale in other ways found during the architecture review — the header date, a git checkpoint sixteen commits behind, a 44/45 file-count contradiction, unreferenced docs/plans and docs/runbooks, and verify evidence pinned to c83084f. Those are a single reconcile pass once the architecture work stops moving HEAD. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CourseEditRouteScreen held a 132-line save() behind ten early returns. It threw RepositoryError five times — the data layer's error class, with a 'validation' code, raised by a component about rules the repository never enforced. Reaching any of it in a test meant satisfying every guard and mocking the whole @/lib/data barrel, including a hand-rolled RepositoryError class so the screen's throws still worked. planCourseSave is pure: a draft plus its context in, a saveCourseAggregate command or the reasons it cannot be built out. No repository, no React, no guards. The course id stays out of it — a plan says what to save, not where. The screen keeps the guards and the wiring, and converts a failed plan into the plain Error the draft session already renders; it never inspected the class, only error.message. Validation stays fail-fast so this diff is about where the logic lives, not about changing what a user sees. issues is an array so collecting every reason later needs no interface change. Four tests move to the module, where they assert a returned object instead of driving a screen: the temporary-client-ID projection, both CCN non-match rules, and the CB03/TOP reconciliation. The atomic-commit test stays on the screen as the wiring guard. The CCN tests now run the real planCCNAdoption against a standard fixture rather than a mocked plan, so they cover the integration the screen test could only simulate. Extracting found a live defect on the way: childId tests for the 'tmp-' prefix, and a first pass at the module wrote 'temp-'. That would have sent a draft-only child to the repository as a persisted row. The migrated test caught it; it is pinned now. Not done, and deliberately: makeCCNMatches and aggregateAudit have the same "logic in a screen" shape but are read-path derivations, and the two-pass confidence fallback in makeCCNMatches deserves its own argument. Considered and rejected: moving the save into the repository. It would make the repository speak CourseViewModel, which ADR-0002 deliberately kept out of its vocabulary, and would trade a pure function for one that needs a database to test. CourseRouteScreens.tsx 680 -> 546 lines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
worker/index.ts held the TOP code catalog, the compliance source pack, their membership predicates and the two system prompts built from them. Reaching any of it meant a Turnstile exchange, an HMAC round trip and a quota reservation, so the data the AI is allowed to cite was only ever asserted through HTTP. worker/catalog.ts now owns them, following the seam free-routing.ts and quota-protocol.ts already established: a pure module with a sibling test that never touches the network. The one caller that reached into the code->entry Map now asks topCodeTitle() instead, so the Map stays private and the module exposes the question rather than the structure. Behaviour is unchanged. The new tests pin what the HTTP path could not assert cheaply: the four source ids and their checksums, the twenty TOP codes, that inherited object keys are not catalogue members, and that both prompts still carry their prompt-injection guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
worker/index.ts was 2520 lines exporting one function, so the seven task definitions could only be reached through it. Asserting that the chat validator trims its output cost a Turnstile exchange, an HMAC round trip and a quota reservation, which is why most validators had no direct test at all and the eval-parity suite minted a fresh session per case. worker/tasks.ts now owns the schemas, the TASKS registry, the route table and the output-validation primitives. OutputValidationError moves with them rather than joining ApiError: it deliberately carries no status or code, because handleRequest is the single place that turns it into a 502 UPSTREAM_INVALID_RESPONSE. index.ts drops to 1706 lines and its diff is a pure deletion plus one import — every one of the 682 moved lines is byte-identical, the only edits being the six declarations that gained an export keyword. Tests follow the seam. worker/tasks.test.ts asserts all seven validators directly, including the cases that were never worth their HTTP cost: non-consecutive topic sequences, a catalogued TOP code wearing a title the catalog does not give it, a compliance citation replaced by the server's own source metadata. worker.test.ts keeps exactly one HTTP case, now also asserting the 502 names neither the failing field nor the rejected value. ai-eval-parity compares the rubric synchronously and keeps one HTTP case on content-outline — the only task whose validator reads the request input, so it is the case that proves handleRequest still threads input through to the validator the rubric is pinned to. Behaviour is unchanged; the worker suite goes 216 -> 225. Follow-up: the catalog-parity block in ai-eval-parity still runs nine HTTP round trips, justified by a comment claiming the Worker's catalog is module-private. That stopped being true in 500095d; the comment is corrected here, but the block itself could now compare data directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The TOP catalog and the compliance source pack were written out three times, in three shapes: an ordered array in the Worker, a code-to-title map in the browser, a frozen map in the eval fixtures. The compliance pack additionally used two key spellings — the Worker said title/section and renamed them to sourceTitle/sourceSection on the way out. shared/ai-catalog.ts now holds one copy. worker/catalog.ts keeps only what is Worker-only (the membership predicates and the two system prompts); src/lib/ai/schemas.ts re-exports under the names the UI already imports, so no component or test changed. sourceTitle/sourceSection won because that is what crosses the wire and what CourseAIControls renders, which turns the Worker's five-field rename into a spread. The premise this was planned against turned out to be wrong and is worth recording. Both the handoff and the comment at eval-catalog-parity.test.ts said the Worker's copy could not be imported from the default vitest project because it only runs under the Workers pool. It can: exclude ['worker/**'] stops worker tests from running there, not from being imported. A probe test in the jsdom project imported worker/catalog.ts and worker/tasks.ts and ran a validator. Nothing ever required a plain-data lowest common denominator on the TypeScript side, so the third copy could simply be deleted rather than parity-tested. What remains is one duplicate, for a reason that has not gone away: ai:evaluate runs scripts/ai-eval-fixtures.mjs under plain Node, which cannot import TypeScript. eval-catalog-parity.test.ts pins it to the shared module directly now, and its claim about a Worker third copy is corrected. shared/ is added to both the vitest include and the coverage include, so moving code out of src/ does not quietly shrink what the coverage floor measures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every AI output bound was written out three times — once in the Worker's JSON schema, once in the Worker's validator, once in the browser's Zod schema — plus two or three more times in the evaluation rubric. Around forty-five literals for nineteen bounds. This is not a tidiness problem. The browser re-validates the Worker's response, so a bound that differs by one between the two does not fail loudly: it fails as AI_OUTPUT_REJECTED on a response the Worker considered perfectly valid. That exact failure has happened before, which is why the parity test exists at all. shared/ai-limits.ts makes the agreement structural rather than coincidental. All nineteen agreed before this change — it is preserving a property, not fixing a break. Only bounds both sides express moved. The Worker's 7-character TOP code, its 80-character sourceId, the browser's bounds on the citation metadata the Worker itself injects, and confidence 0..1 each have one enforcer, so routing them through a shared module would add a hop without adding an agreement. scripts/ai-eval-fixtures.mjs keeps its copy, because ai:evaluate runs it under plain Node and Node cannot import TypeScript. It now declares the bounds once as EVAL_OUTPUT_LIMITS instead of inlining them, and eval-catalog-parity.test.ts compares the two objects by value. That closes the drift that mattered most: the rubric qualifying a model on output the Worker rejects, so the model passes the gate and then fails every real request. Rewiring the rubric is a change to the deployment gate, so it was checked harder than by running tests. Every fixture's schema, check id and pass/fail verdict for the good and bad sample of all seven tasks was captured before the edit and diffed after: identical. The new parity assertion passes by construction, so it was mutated (1600 -> 1599) to confirm it fails on drift. EVAL_OUTPUT_LIMITS is typed loosely in ai-eval-fixtures.d.mts on purpose. Mirroring the real shape in a hand-written declaration would be a fourth copy of the contract, free to drift in silence; the value comparison is the check that carries the weight. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The repository could only be asked for a page, so five callers each invented an answer to "give me all of them". One hand-rolled a fan-out over pageCount-1 parallel reads and rebuilt a fake single-page result; four passed a page size they hoped was big enough — 250, 250, 100, 50. The guesses do not work. page() clamps pageSize to 100, so the registrar dashboard and the approval queue have been asking for 250 and silently receiving 100, with no error and nothing to indicate rows were dropped. The dashboard made it visible without making it obvious: "Outlines in progress — X of N" took N from the uncapped total while X counted only the hundred rows it got back. The seeded fixture has 22 courses so it does not show today, but the demo lets a user create courses, so it is reachable. A test now pins that truncation in place as the reason this verb exists. The fan-out was worse than it looked. listCourses already reads the whole table into memory, filters and sorts it, and only then slices — so fetching pageCount pages ran a full table scan per page to reassemble data the first call had already computed and thrown away. listAllCourses is that method without the final page(), and listCourses is now a slice of it. Also removed useRepositoryReady. Nothing outside its own test called it, and the initialization path that ships is DemoProvider's, which adds retry and an error state the hook never had — keeping a second, thinner one only invited someone to use it. Two claims about this area that did not survive checking, recorded so nobody re-derives them. useRepositoryRevision is not caller-less: it is used at hooks.ts:49 inside useRepositoryQuery. And DemoProvider does not duplicate useRepositoryReady; it does strictly more. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`export * from "./session"` put all nine of that module's exports into @/lib/ai, while session-readiness — the module ADR-0001 decided callers should ask — was not in the barrel at all. So the three AI surfaces each issued two imports, one of them a deep path, to reach the thing the ADR points them at, and the raw disclosure and session-marker accessors sat in the package surface next to it. Only two of the nine had a production caller outside the module: acknowledgeAIDisclosure and establishAISession, both from AIConsentGate. Five exist so session-readiness can reach them across a file boundary. getOrCreateInstallationId had no caller anywhere, and markAISessionReady had only test callers. The barrel now exports those two plus session-readiness: nine down to five, with the readiness module reachable by name for the first time. Each AI surface is one import, and AIConsentGate's test is one mock instead of two. The rule this settles, now written in index.ts: module-level exports are the file's surface, the barrel is the package's. The five accessors stay exported because session-readiness has to reach them; they are simply not @/lib/ai's business. A test that wants the raw marker imports "../../lib/ai/session", and that path is the marking. session.test.ts re-declared the three storage keys as literals and asserted on raw window storage — the same bypass as reaching around the repository, but with nothing to grep for. It now reads unsafeSessionStorageKeysForTests, named after ADR-0002's unsafeDatabaseForTests(), where the name is the decision. Not done, and deliberately: merging session-readiness into session, which would make those five private. ADR-0001 is Accepted and names the module as its implementation, and the merge would force AIConsentGate's test to mock one coarser module supplying both the readiness answers and the two verbs the component really calls. The ADR's decision — readiness derived client-side, no server oracle — is untouched here, and its references still resolve. getOrCreateInstallationId stays exported despite having no caller: session.test.ts exercises it directly and its storage-fallback behaviour feeds the Worker's HMAC install identity and the daily quota. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Five of the six repository roles have no importer outside contracts.ts. That reads like a failed decision, so the amendment says plainly that it is not one: the benefit ADR-0002 claimed is that a reader learns the role they need instead of a flat list of names, and that is delivered by the interface being segmented, not by anyone importing the segments. The roles stay. Alternative 6 turns out to be narrower than it looked. It rejected splitting a screen across two or three role imports, fragmenting the call site — not a caller naming one role. ContextualAssistant was picking two verbs out of the whole union; it now picks them out of CurriculumReads, where both live. Still one import, so the objection does not reach it, and a second role now has a caller. Three corrections. The "33 methods" figure was 32 when last checked and is 34 now — 2e7809f added listAllCourses and listAllPrograms — so it is marked as that day's census rather than a live number. AIArtifactPersistence never shipped; 0c3972c settled the role as AIConversationPersistence, with AIPersistence surviving as an unused alias. And the entry calling saveAIArtifact write-only and asking for an explicit decision has had one, in the direction it pointed: no artifact read, write or delete verb exists, repository.test.ts asserts all three are absent, and the Dexie table stays declared because dropping it is a schema migration. Amendment rather than an edit of the original text, so what was decided in August and what was corrected later stay separable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RELEASE_INPUT_DIRECTORIES did not list `shared`, so the two runtime modules added by f1e0535 were outside the canonical release enumeration. That is not only a fingerprint gap. verify-fresh-checkout copies exactly these files into a canonical checkout and rebuilds there, so the rebuild would have hit unresolved imports from both the Worker and the browser bundles: release:verify:fresh-checkout has been broken since f1e0535. Nothing shipped wrong — no Worker has been bootstrapped and the gate has never run — but eight green `npm run verify` runs passed straight over it, which is the hole HANDOFF.md already describes: verify does not exercise the release scripts, so their unit tests are the only guard. So the guard is the point of this commit, not the one-line list change. release-inputs.test.mjs now walks the demo root and fails if a directory holding source is not enumerated, which is what would have caught this the moment shared/ was created. Confirmed by removing 'shared' again and watching it fail. `tests/` is recorded there as a deliberate exclusion rather than being added: Worker unit tests are not a build input. It does mean a change under tests/ leaves the source fingerprint unchanged, which is worth an explicit decision rather than a silent widening of what the fingerprint covers. The assertion is a subset check, not equality — `public/` is a release input that ships static assets and no source, so it never appears in the walked set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The reconcile was deliberately deferred while the architecture work kept moving HEAD. HEAD has stopped, so this brings the document back to the code. Recorded for the first time: an automated pre-scan review over f0c5854..HEAD, the range whose Codex Security diff scan is still owed. It found nothing, and it lists the checks it ran rather than reporting a bare "clean" — the previous clean scan covered a range containing two unscrubbed spawn sites, so scope is the part worth writing down. It is a prior for the scan and closes no finding. One sub-threshold observation is noted as belonging to the owed standard scan instead: worker/index.ts reflects a client-supplied field name into a 400 JSON response, which predates the baseline and is not in this diff. Corrected against the code: the reconcile date; the current head and its 29-commit range; the 44-vs-45 runtime-file contradiction, where the prose and the status row disagreed and 45 was right; and every figure in "Verification already performed", which was pinned to c83084f and is now d650b92 — 617 tests in 79 files, 222 Worker tests, coverage 85.31/74.70/82.07/87.13, Worker bundle 71.33 KiB. docs/plans/ and docs/runbooks/ existed unreferenced by any top-level document. They are now in the file map, along with docs/adr/ and docs/handoffs/, each with a line saying what it is for and how far to trust it — the plan predates the work it describes, so its line references are as-of its own date. The eight-candidate review now has a handoff of its own, following the precedent f451f02 set for the nine-candidate one. It records what each candidate found, the three defects that were not shape problems, and the five prior claims that did not survive being read against the code. AGENTS.md gains the rule the shared/ break exposed: runtime source is no longer confined to src/lib/**, and any directory that ships source has to be a release input or fresh-checkout rebuilds without it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A review pass read every checkable claim in the 2026-08-07 handoff back against the tree. The substance held — the range, the candidate table, the truncation finding, the release-input break and the worker/index.ts line all verified exactly — but five entries did not, and four of them were in the sections whose whole purpose is to stop a later reader re-deriving something. Those are the ones worth fixing. The 60-unit program limit does not exist in this demo. It was offered as the verdict sweep's next target, and there is nothing there to look at: compliance/rules.ts mentions programs nowhere, totalUnits is a UnitValueSchema decimal string with no ceiling of any kind, and the string appears nowhere under src/, worker/, shared/ or scripts/. It is a rule of the parent Next.js + FastAPI application that crossed the stack boundary this demo's CLAUDE.md draws. The claim originates in ADR-0003, so that is amended too, rather than edited — following ed1de9a, so what was decided and what was corrected stay separable. The sweep still has a target; it is restated without the bad example. There is no spawnChild helper to consolidate, which is the actual finding: a reader greps and gets nothing. Replaced with the survey. scripts/ has 13 spawn sites, each with its own wrapper — 12 call spawn directly and release-deploy.mjs:470 goes through an injectable spawnProcess, which is why a `spawn(` grep finds only 12 and reconciles this with HANDOFF.md's count. Seven accumulate child output under five overflow policies, now named with file and line rather than tallied. The original counts were off on two of three: seven capture blocks, not nine, and "two unbounded" does not survive any consistent reading of the code, so the numbers are replaced by anchors. The owed Codex Security scan no longer carries a range here. It said f0c5854..d650b92; HANDOFF.md says f0c5854..HEAD and explicitly resolves at scan time rather than pinning a SHA. HANDOFF.md is authoritative by this document's own closing paragraph, so the entry defers to it and there is no second copy to drift. The hooks census was right and unverifiable at the same time. "Five are" is correct as of 9b4b2ab, where hooks.ts exported exactly 12 and exactly five delegated to one repository method and nothing else — but the document named neither the five nor the commit, and the tree has since moved to 13 hooks of which four are pass-throughs. Both figures are now pinned to that commit with the drift recorded, the same treatment ed1de9a gave ADR-0002's method count. hooks.ts:49 is 48. Inherited from 2e7809f's commit message, so the error had already propagated once. Left alone deliberately: HANDOFF.md:66 reads "14 demo commits, 9b4b2ab..d650b92", but that range is 13 and 14 is the count for f0c5854..d650b92 — the label and the range disagree by one. It is a real error, but HANDOFF.md is release-authoritative and its commit counts are not something to adjust as a drive-by. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The keys cannot be pasted in one sitting, which is the thing a next session most needs told. Three of them do not exist until earlier steps produce them: the Turnstile site and secret keys bind to a hostname that deploy:bootstrap has not yet created, APP_ORIGIN is that same hostname, and OPENROUTER_FREE_MODELS comes out of ai:evaluate passing two models under human review. Only the Cloudflare account ID and token, the OpenRouter key, and the generated HMAC secret can be prepared up front. The nine stages are that dependency order, not a suggested sequence. Four destinations, recorded because they are not interchangeable and three of the four are outside the repository. NEXT_PUBLIC_TURNSTILE_SITE_KEY is the only key that belongs in the tree, in gitignored .env.local, and the gate proves that exact value rather than the placeholder was compiled into the sealed export. The three Worker secrets go in an external 0600 file passed by path, never through `wrangler secret put`, because each such invocation is its own version change and trips the ownership lock. Cloudflare and canary credentials stay in the shell for one command. wrangler.jsonc holds configuration that ships fail-closed and is not a key at all. Two constraints written down because neither is visible from the command list. The recorded discovery and evaluation evidence expires in 24 hours and is bound to the source fingerprint and the OpenRouter key fingerprint, so stages E through H are one working window and stage D should not start outside one. And gate step 10, release:fresh-checkout, has never run against the current source shape: HANDOFF.md records it passing at 274d428, which predates shared/ existing, and f1e0535 then broke it for four commits. d650b92 fixed the enumeration and added the guard, but the repair is proven by its unit test and not by a rebuild, so the gate will be its first real exercise. It is also the only blocking item needing no credentials, which makes it the cheapest thing to de-risk first. The decision to drop the Codex Security scans is recorded here rather than applied to HANDOFF.md's status rows, and the handoff says so explicitly in both places. Those rows still read as blocking, and a fresh session that trusts them — correctly, since AGENTS.md and HANDOFF.md outrank everything else for release work — will stop on them. Restating them is named as the first task, ahead of any key work, so the contradiction is visible rather than silently resolved by whoever reads it next. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedToo many files! This PR contains 283 files, which is 183 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (283)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
Introduces
calricula_pwa_demo/— a standalone, local-first PWA demo that livesas a tracked subtree in this repository. 61 commits, 288 files, all additive.
This is source only. Nothing here is deployed, and this PR does not deploy
anything.
What it is
It shares the product domain and visual language with the main app but none of
its stack. No Next.js server, no FastAPI, no PostgreSQL, no Firebase, no
Gemini, no Docker, no auth, no server database. The root
CLAUDE.mddoes notapply inside that directory;
calricula_pwa_demo/CLAUDE.mdgoverns it.output: 'export'produces a finite routeset;
run_worker_first: ["/api/*"]means the Worker only ever sees APItraffic. Entity IDs live in query parameters, which keeps the route set finite
and precacheable.
curriculumRepositorywrite path. Nothing leaves the device except the AI calls.
golden-parity.test.tsto a captured fixture of the parent Python complianceservice — that parity test is the contract.
cookie, rate limits, and an authoritative five-per-UTC-day quota in a SQLite
Durable Object. Free-only OpenRouter routing (ZDR,
data_collection: deny,zero price caps). The quota DO stores an HMAC-derived install ID, day, attempt
count, request IDs, and expiry — never prompts, responses, or curriculum text.
scripts/(54 files), most with a sibling*.test.mjs. Ships fail-closed:AI_ENABLED=false, an invalidAPP_ORIGIN,and an empty model list, so an accidental first deploy cannot serve AI.
Verification status
Per
calricula_pwa_demo/HANDOFF.md,npm run verifypassed atd650b92—617 UI/repository/release-script tests in 79 files, 222 Worker tests, 7 Chromium
smoke tests. I did not re-run the suite for this PR; that figure is quoted
from the handoff, not freshly measured. The two most recent commits are
documentation only.
Release status — not ready, deliberately
.release-evidence/local-gate.jsondoes not exist — the 14-step gate hasnever been run to a seal
AI canaries all still outstanding
docs/handoffs/2026-08-07-release-key-provisioning.mdis the starting point forthat work. It maps every key to one of four destinations and stages them in
dependency order, because three of them cannot exist until earlier steps produce
them.
Reviewer notes
longer using Codex).
HANDOFF.mdstill carries two scan rows reading asblocking; restating them is the first task recorded in the key-provisioning
handoff, and is intentionally not done in this PR.
release:fresh-checkout, has never run against the currentsource shape.
f1e0535addedshared/without enumerating it as a releaseinput, breaking that step for four commits;
d650b92fixed the enumerationand added a directory-coverage guard. The repair is proven by its unit test,
not by an actual rebuild.
git diff --name-only main...HEAD -- . ':(exclude)calricula_pwa_demo/**'is empty. No file outside the demo directory is touched.
scripts/**andtests/**are synthetic fixtures, each named to announce it(
sk-or-v1-not-a-real-key,must-not-reach-a-browser).🤖 Generated with Claude Code