feat: consent-first, role-aware Bedrock tutoring uplift (#8–#11)#13
Merged
Conversation
…cret from .env The GitHub-App client id (technically public) is no longer hardcoded in the repo: [vars] entry removed; it becomes a wrangler secret sourced from the repo-root .env GITHUB_APP_CLIENT_ID, alongside GITHUB_CLIENT_SECRET. README runbook updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
Add a consents table (github_user_id, terms_version, granted_at; PK on the first two) to schema.sql, idempotent and FK-light so consent can be recorded before a learners row exists (the pending-consent flow, t5). Add getConsent/recordConsent/deleteLearnerData to db.js, following the existing prepared-statement/ON CONFLICT/ISO-8601 style, with unit tests against an extended D1Stub (consents storage + batch()). No route wiring — index.js/auth.js/session.js untouched, that's t5/t7. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…deleteLearnerData)
…ERMS_VERSION Adds /learn/terms/ and /learn/privacy/ (site-astro), both rendering a visible version + effective date and linked from the /learn footer. The Privacy Policy names the three processors (GitHub OAuth, Cloudflare hosting, AWS Bedrock Nova Sonic 2 / Nova Pro for the approved tutoring tier — with an explicit speech/text-leaves-to-the-model-provider callout), states what's collected (GitHub id + display name, the learning ledger, a session cookie; no email, no password), why, retention (kept while the account exists, hard-deleted on delete/withdraw), and the learner's rights (access/export/delete/withdraw). Terms of Use covers GitHub-based accounts, acceptable use, "as is" service, the approved AI-tutoring tier, data ownership, termination/suspension, changes-to-terms -> version bump -> re-consent, and a simple contact/governing-law section (GitHub issues, no published email). shared/terms-version.mjs is the single TERMS_VERSION/TERMS_EFFECTIVE_DATE source of truth, started at 1.0.0 / 2026-07-11, re-exported (not copied) by both site-astro/src/lib/terms.ts and workers/learn-api/src/terms.js — proven importable under both Vite/astro build and Node's own --test runner. tests/test_policy_pages.py cross-checks the pages' data claims against workers/learn-api/schema.sql (no email/password column) and proves the rendered version can't drift from the shared source. check-export-pages.mjs and check-static-auth.mjs are updated to treat terms/ and privacy/ as known non-subject, non-learner-panel pages so the new routes don't trip the orphan-page or signed-out-invitation checks.
…source (shared/terms-version.mjs)
Extends the existing `cloze` exercise type (unchanged since contract 1.0)
with two OPTIONAL fields — `text` (passage with `{{blank_id}}` placeholders)
and `blanks` ({id, options, answer}) — for a multi-blank, closed-option
variant a reader can check without a driver or model call. The legacy
single-blank free-text cloze shape (prompt + answer) is untouched.
- learn/contract/schemas/{practice,story}.json: additive `cloze_blank` $def
+ exercise.text/blanks, open payloads so nothing predating them breaks.
- learn/subjects/conformance.py: new `cloze-items` doctor check — reads
every declared story and verifies well-formed blanks (options include the
answer, blank ids unique + match text placeholders 1:1, item_id present),
passing trivially for subjects that declare none.
- No changes needed to record.json or workers/learn-api/src/validate.js:
`recorded` carries no exercise-shape field, so a cloze result (its
multi-blank tally lands in the pre-existing correct/total counters) was
already contract-valid — proven by new tests, not just asserted.
- site-astro: renders a pick-the-right-word cloze passage as inline button
groups per blank with instant right/wrong feedback, entirely client-side
(wireClozeExercises() in learner.js, zero fetch calls, works signed-out).
- tests/test_site_export.py locks the export byte-stable for a subject
declaring no cloze items (acceptance criterion 5).
docs/specs/subject-plugin-contract.md §3.6.1 documents the shape and the
least-invasive design decisions for the subject-CLI follow-up tasks
(french-cli, spanish-cli, culture-guide) to author against.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
… check, interactive export rendering
…c 2 over API GW WebSocket (t4) Spike verdict (infra/SPIKE.md, run live against us-east-1, read-only): InvokeModelWithBidirectionalStream is SigV4-only — Bedrock API keys get 403 "This operation does not support API Keys" — but a plain Python 3.12 process holding the stream via the experimental aws_sdk_bedrock_runtime SDK completed a FULL audio round trip (Polly speech in, transcript + 19 audioOutput events back), so the Lambda relay authenticates with its execution role and python3.12/arm64 stays (league's convention, no Node). Bonus t15 intel: the bearer key serves Nova Pro on /converse (200 "Pong.") but NOT on /openai/v1/chat/completions (model_not_found in us-east-1). infra/template.yaml mirrors league-of-agents-platform's conventions: SAM, one arm64 Lambda (router + self-invoked session holder), long-form intrinsics parseable as plain YAML, capacity caps as Parameters cross-checked against voice_bridge/config.py by test, an AWS Budgets alarm pinned to a 20 USD/month ceiling with every sizing choice commented against it, zero idle cost. h7 groundwork: the bridge accepts ONLY short-lived voice tokens (session.js-symmetric HMAC format, scope=voice, approved=true) minted by learn-api (t16 mints; verification implemented + unit-tested here). No token, no upstream Bedrock connection — enforced structurally at $connect and proven by test. 65 new tests (tokens 21, handler 12, relay 6, template 26, incl. sam validate when the CLI is present); suite 420 passed. TDD: tests written red first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…SigV4 GO, API-key bidirectional NO-GO)
…ssion, accept/decline endpoints, zero D1 writes pre-consent (t5)
Implements spec c9/h1 + decision c19: an unconsented sign-in (web
/api/auth/callback AND device-flow poll) no longer writes anything to D1.
It mints a short-lived (10 min) pending-consent session — the marker lives
in the stateless signed token, so nothing is persisted — and lands the
user on the consent surface: the web callback 302s to /learn/consent/
with a pending cookie; the device poll returns status "consent_required"
plus a pending Bearer token and the consent requirement so the CLI can
prompt.
Pending sessions can only GET /api/me (reports pending_consent +
consent_required incl. TERMS_VERSION, never sliding-refreshes), use the
consent endpoints, and log out; progress/record/tutor reject them with a
structured 403 consent_required before touching D1 or inference —
extending the resource-gate invariant one level (signed-out AND
pending-consent traffic can never trigger a model call).
New endpoints:
- GET /api/consent public: terms_version, effective_date, policy links
- POST /api/consent/accept records consent (exact shared TERMS_VERSION)
FIRST, THEN upserts the learner (the consents
table's missing FK makes that order possible),
revokes the pending token, upgrades to a full
session (Set-Cookie + body Bearer, symmetric
across faces)
- POST /api/consent/decline pending only: revokes the session, clears the
cookie, zero rows ever written; a full session
gets 409 already_consented (withdrawal =
deletion is t7)
The consent-satisfaction check is factored into
consent.js#consentSatisfiesCurrentTerms for t6 to extend with the
version-mismatch (re-consent) rule; re-consent itself is deliberately NOT
implemented here.
TDD: test/consent.test.js was written red-first — the h1 pair (fresh
sign-in on BOTH paths asserts zero D1 writes) failed against the previous
Worker with a logged learners insert, then the gate made it green. The D1
test stub now logs every write statement so "zero writes" and write ORDER
(consents before learners before records) are asserted literally. The two
pre-existing sign-in tests now seed a consented user, preserving their
intent (post-consent sign-in still upserts + issues a full session).
Tests: workers/learn-api 74/74 (was 58); pytest 458 passed, 12 skipped;
markdownlint clean on the README (route table + t10/t12 endpoint shapes
documented).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…ons, consent routes, requireConsented wall
…panish 0.7.0, culture-guide 0.10.0 (cloze in, dev- excluded) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…ck, structured 403, /api/me additive field (t6)
Extends t5's consentSatisfiesCurrentTerms (t5 scope: consent != null) to
require an exact terms_version match against the currently published
version (consent.js#currentTermsVersion, an env.TERMS_VERSION_OVERRIDE test
seam mirroring util.js#outboundFetch — production never sets it).
- requireConsented (auth.js) adds a getConsent D1 read for full sessions and
rejects a stale match with 403 consent_required { reason: "stale_version",
consent_required }, alongside the existing pending-session 403 (now
{ reason: "pending", ... } for the same shape). HttpError gained an
additive `extra` body-fields param to carry this.
- Both sign-in paths (handleCallback, handleDevice) and handleConsentAccept
now resolve the version through currentTermsVersion(env) instead of a bare
TERMS_VERSION import, so a bump reroutes a previously-consented learner's
next sign-in to pending-consent (zero new D1 writes) and re-accept always
stamps the current version.
- /api/me never 403s on stale consent — it reports an additive
reconsent_required field (+ consent_required when true), keeping identity
checks reachable while other routes reject.
Design: D1-read-per-request over a claims-based (session-payload) cache —
simple and always correct now; documented tradeoff in README "Storage".
Tests: new test/reconsent.test.js (10 tests: predicate/seam, h2 stamping,
both sign-in paths re-routing on a simulated bump, the live-token 403 wall
with zero inference calls, the full v1->bump->403->re-accept->200 cycle,
/api/me's additive shape). 5 existing worker.test.js tests updated to seed a
matching consent row now that requireConsented checks D1, not just the
token's pending marker. 74 baseline + 10 new = 84/84 green; 458 Python tests
green; README route table + endpoint shapes updated additively.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
… check, additive 403/me shapes
Add site-astro/src/pages/consent/ (spec c9/c19) presenting the WHAT/WHY/ RETENTION/WHO-CAN-SEE-IT notice and a dedicated src/scripts/consent.js that drives it against workers/learn-api's pending-consent contract: GET /api/me + GET /api/consent to render live state, POST /api/consent/accept to upgrade into the signed-in hub, POST /api/consent/decline to confirm nothing was stored. Handles pending, expired (401, 10-min TTL), already-signed-in, declined, and error states. Extend check-export-pages.mjs's KNOWN_NON_SUBJECT_DIRS and check-static-auth.mjs's NOT_A_LEARNER_PANEL_PAGE for the new /consent/ route, and add a precise (not blanket) CONSENT_ALLOWED_SUFFIX_RE fetch whitelist for consent.js, separate from learner.js's own. Fix the built-bundle whitelist check to collect API_BASE aliases globally across dist/ instead of per-file, since Vite now code-splits src/lib/api.ts into its own shared chunk now that two scripts import it. tests/test_consent_page.py locks the page's key claims (no email/no password, imports TERMS_VERSION rather than hardcoding it, links both policy pages, all five states present) the same way test_policy_pages.py does for Terms/Privacy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…reak (flaky reconsent full-cycle test) Two consent rows inserted in the same millisecond made ORDER BY granted_at DESC ambiguous; the newest insertion now wins (rowid DESC), stub mirrors it, regression test pins the tie case. Found by the t6 merge gate (2/5 runs red). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…-learner erasure (t7)
GET /api/export (requireConsented) returns the learner's complete data as
self-describing JSON: identity row, every recorded result across every
subject (db.js#listAllRecords), and full consent history (db.js#listConsents).
POST /api/delete (requireAuth — deliberately not requireConsented, so a
stale-consent learner can still erase without re-accepting terms first)
requires a { confirm: "<github_user_id>" } body, calls the existing
deleteLearnerData batch, revokes the current session (KV tombstone), and
clears the cookie — h3 verbatim: no row survives in learners/records/consents
and the old token 401s on the next call. c18 holds: the ledger stays
append-only for everyone else (isolation-tested), whole-learner erasure is
the only deletion path, and a deleted learner signing in again is
indistinguishable from a brand-new one (full-cycle tested).
No site-astro change: no existing account/profile surface exists to hang a
destructive, confirmation-guarded control on without inventing new UI — both
routes are CLI/agent-ready today; the web affordance is deferred to t8
(roles + visibility), which already needs a real account-scoped surface.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…lete, erasure invariant
…rner list, self-serve visibility Adds spec c12/h4 to the consent-first uplift: server-side-only admin enforcement (src/admin.js#isAdmin/requireAdmin, backed by the new ADMIN_GITHUB_IDS Worker var), GET /api/admin/learners (a three-query, no-N+1 roster with per-subject record counts), default-private learner visibility living in the existing state blob (no schema change) with a POST /api/me/visibility toggle, and additive is_admin/ visibility fields on GET /api/me. h4 is proven with a forged client-side admin claim test and a cross-learner isolation audit. The site-astro account panel (LearnerPanelOverview's signed-in hub) gains the visibility toggle plus the "Export my data"/"Delete my data" web affordance t7 deferred here, and an admin-only learner list gated on is_admin. The CLI gets `learn admin learners` following the existing learn.profile authenticated-API pattern (learn auth), with an explain-catalog entry and a green agent-first rubric. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…st surface, default-private visibility, account panel
…PROVED The tutoring tier is now admin-granted per learner (spec c13/h5, decision c20 enforced in code): - Worker: `approved` flag in learners.state (no schema change, absent == not approved), POST /api/admin/approve + POST /api/admin/revoke behind t8's requireAdmin; handleTutor 403s approval_required BEFORE the INFERENCE_URL check with zero outbound inference (h5, proven by extending the ordering test one level in test/approval.test.js); approve 409s consent_stale unless the target's consent covers the CURRENT terms (c20); a terms bump blocks tutoring even for an approved learner (both gates independent); deletion erases the flag — a re-signup is not approved. Additive: learner.approved on GET /api/me, approved on the admin roster. - CLI: learn admin approve/revoke <github_user_id> (profile API client + catalog entries + rubric green); admin learners renders tutoring status. - Site: account panel shows the caller's own tier; the admin list gains per-learner approve/revoke buttons; check-static-auth's admin whitelist is now exactly enumerated (learners|approve|revoke). Worker tests 136 -> 151; pytest 500 green; teken doctor --strict green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…level tutor gate, c20 enforced
…hly voice budget, site voice UI (c15/h7) The learn-api half of the voice approval gate plus the web face; the live bridge deploy stays a supervised launch-gate step (t4 handoff #4). Worker (POST /api/voice/token): - Same learner-side gates as /api/tutor, same order: requireConsented -> the SAME approvedOf check (reused in place per t9's hook-point note, not duplicated) -> config 503 not_configured (VOICE_BRIDGE_URL + VOICE_TOKEN_SECRET, mirroring INFERENCE_URL) -> budget 429. h7: no token in any non-200 body, ever — signed-out/pending/stale/unapproved proven 401/403/403/403 with the approval 403 firing before the config 503. - src/voice.js mints the session.js-shaped token infra/voice_bridge/tokens.py verifies: {v, scope:"voice", uid, approved:true, iat, exp, sid}, unpadded base64url, HMAC-SHA256 over the encoded body, TTL 120s (mint->connect window; session length is the bridge's own cap). - Cross-language pin: tests/fixtures/voice_token_cross_language.json is a committed JS-minted token — test/voice.test.js asserts the minter still reproduces it byte-for-byte, tests/test_voice_token_cross_language.py asserts tokens.py still verifies it. Neither side can drift alone. - Per-learner monthly budget (t4 handoff #3): each mint books the full bridge session cap (300s) against learners.state.voice_usage; the mint that would exceed VOICE_MONTHLY_SECONDS_CAP (default 1800s = 30 min, sized against the $20 Budgets ceiling in a src/voice.js comment) 429s voice_budget_exhausted and books nothing; month rollover is free (stale month reads as zero). Honest scope documented in README: caps MINTED seconds, not actual streamed seconds — bridge caps + Budgets are the cost backstops; tightening needs bridge->worker reporting (follow-up). Site (own page dir + own script, disjoint from t15's tutor/learner files): - site-astro/src/pages/voice/index.astro: five gate states (signed-out visible with no JS, consent-needed / not-approved / ready / error), mic start/stop, session timer, granted limits, transcript. - site-astro/src/scripts/voice.js: t4's client audio contract — mic -> downsample -> 16kHz/16-bit/mono LPCM -> {"seq", "audio"} frames (seq ordering-authoritative); 24kHz LPCM downstream via WebAudio; 4001 unauthorized + connect-failure + budget/consent/approval mint errors all surfaced; client-side stop at max_session_seconds. - h7 client half is structural and audited: exactly ONE new WebSocket(), inside the injectable openBridgeSocket seam, whose only call site sits after the token mint's non-ok early return — no token, no WebSocket. check-static-auth.mjs gains a precise VOICE_ALLOWED_SUFFIX_RE (/me, /voice/token) + the documented WebSocket allowance; check-export-pages learns the voice page; tests/test_voice_page.py locks all of it with pure file reads. Verified end-to-end without AWS: a token minted by the real Worker route passes the real bridge $connect gate (handler.lambda_handler) and a forged token is refused 401. Deferred to launch gate: the live Nova Sonic exchange — operator runbook (sam deploy VoiceTokenSecretValue=same secret as the Worker's VOICE_TOKEN_SECRET, BudgetAlertEmail; then VOICE_BRIDGE_URL + secret on the Worker) recorded in workers/learn-api/README.md. Tests: worker 151 -> 166; pytest 500 -> 533 (+ cross-language contract, voice page); site build + check green; black/isort/flake8/bandit/ markdownlint clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
… pinned), monthly voice budget, voice page
…iring (config only), approved-tier tutor surface, personalized cloze to the ledger The Worker diff is ZERO code lines (h11 verbatim): t15 ships as config + docs + a client surface, with the four-level gate and the forward-verbatim broker untouched. Wiring (c24, corrected target): Bedrock's OpenAI-compat surface does NOT serve Nova Pro in any probed region (model_not_found in us-east-1/us-west-2/ eu-west-1/eu-central-1) — the live-verified target is the native Converse API. wrangler.toml records the exact URL commented out (model us.amazon.nova-pro-v1:0, region us-east-1, INFERENCE_TOKEN = Bedrock API key via wrangler secret put, sourced from .env AWS_BEDROCK_API_KEY_SECRET; cost-when-busy: per-token only, zero idle). README "For t15" documents the NO-GO/GO probe results incl. the sanitized live re-verification (HTTP 200, latencyMs 491). test/tutor-converse.test.js (additive, worker suite now 153) proves the unchanged broker forwards a Converse payload verbatim + the learner stamp and relays responses/errors untranslated. Tutor surface (site-astro, approved learners only): TutorPanel.astro on every subject page + src/scripts/tutor.js (DOM) over src/scripts/ tutor-core.js (pure: prompts, Converse builders, defensive parsers, §3.6.1 validator, record builder). Three flows through POST /api/tutor — GRADE (strict rubric + strict-JSON pinned in the system prompt, pass/partial/fail + explanation), NEXT-STEP (adaptive from /api/progress weak items, weakest-first), CLOZE-GEN (a personalized pick-the-right-word story, validated client-side against §3.6.1 INCLUDING item_id reuse from an existing weak item as the stable join key; played inline with the story reader's interaction pattern; tally recorded via the existing POST /api/record with correct/total — no new record fields). Gate honored end-to-end: the panel is signedin-only (signed-out sees nothing, zero fetches); non-approved learners see the "tutoring is admin-approved" note and tutor.js returns before wiring anything; learner.js gains only the two-line learn:me hook. check-static-auth.mjs extended precisely: an anchored TUTOR_ALLOWED_SUFFIX_RE (/tutor$ | /progress/ | /record$), the built-bundle union, a hydrateTutorPanel gate check, and the pre-t15 blanket "no /tutor anywhere" rule replaced by "referenced ONLY through whitelisted fetch templates" (fetch templates, quoted tutor-script paths, and // comments stripped; anything else still fails) — all three mutation-tested. Tests: tests/test_tutor_page.py (25, consent-page pattern) locks the gate note, whitelist precision, prompt pinning, and the config-only wiring; scripts/check-tutor-logic.mjs (16 checks, wired into npm run check) executes tutor-core.js's pure logic — builders, fence/prose-tolerant parsers, every §3.6.1 violation, the join-key rule, tally-to-record shape. Deliberately untestable without live inference: real Nova Pro output quality (t17's gate drives that live). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…grade/next-step/cloze-gen), zero broker code change (h11) Conflicts with t16 resolved as unions: wrangler.toml secret notes, README For-t15/For-t16 sections (both SHIPPED), check-static-auth's four-way whitelist union (learner/consent/voice/tutor) with tutor renumbered section 6; test_tutor_page's union pin updated to four REs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…ce/cloze success signals + boundary invariants The uplift's success signals (spec c8) as an extended launch gate that FAILS against pre-uplift prod and passes post-deploy (h17), plus the mechanical boundary invariants (h16). New: - tools/launch-gate/consent_walk.mjs — the consent -> approval -> deletion -> tutoring/voice signals as HTTP flows over a real origin. LOCAL mode drives the FULL authed flows against the in-process topology (10/10): zero D1 writes until consent (consent row before learner row), re-consent on version bump, self-serve delete + revoke, tutor 403 with zero outbound inference, admin approve/revoke, voice gate order (approval before config), policy/consent/voice pages + cloze content served. LIVE mode probes prod's unauthenticated surface (routes must 401 not 404, pages must 200 with markers). - tests/test_launch_gate_invariants.py — always-on mechanical invariants (h16): no email/password column, no provider SDK in the Worker (dep or import), static-auth check stays wired, no subject prose authored in learn-cli, privacy policy names every processor and its data claims match the schema. - tools/launch-gate/BASELINE-2026-07-11.md — the recorded pre-uplift LIVE baseline: 9/10 consent_walk LIVE checks fail against today's prod (routes/pages 404), the h17 evidence. Changed: - run.sh — two new steps: the Worker unit suite (168, the authoritative authed-flow proof) and consent_walk.mjs (honors LIVE_ORIGIN). - api-server.mjs — fix ERR_HTTP_HEADERS_SENT on Set-Cookie responses (setHeader after writeHead); the harness could not serve consent accept/login/logout/delete e2e. Surfaced by the new consent walk. - report.py — labels for the new audiences. Gates: pytest 568 passed / 12 e2e-skipped; worker 168/168; site build + npm run check green; consent_walk LOCAL 10/10, LIVE 9/10 (baseline); black/isort/flake8/bandit clean; rubric doctor PASS; markdownlint 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…(pending user confirmation) - version-bump minor 0.5.4 -> 0.6.0 with the full uplift changelog - spec: post-convergence correction note on c24 (OpenAI-compat -> native Converse), annotating not rewriting the converged claim; captured as q1 in the frame questions store for user confirmation at this PR gate. h11 (config-only, no provider SDK) still holds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
Contributor
Author
|
/agentic_review |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
Code Review by QodoNew Review StartedThis review has been superseded by a new analysis |
The cross-language contract fixture (tests/fixtures/voice_token_cross_ language.json) is a deterministic HMAC of a PUBLIC payload with a fake, committed test secret — it pins the JS minter / Python verifier agreement byte-for-byte and cannot change. GitGuardian's JWT detector matches its shape; this narrowly-scoped ignore (that one path + the known fake secret string) records it as a documented false positive. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
PR Summary by QodoConsent-first, role-aware Bedrock tutoring uplift
AI Description
Diagram
High-Level Assessment
Files changed (94)
|
Code Review by Qodo
Context used✅ Tickets:
🎫 Learn: explicit user consent before saving learner data (consent gate + right to delete) 🎫 Learn content: strengthen curriculum upstream (french/spanish/culture-guide) + cloze exercises, then re-export 🎫 Learn: Terms of Use + Privacy Policy pages (versioned, linked from /learn, disclose Bedrock processing)✅ Compliance rules (platform):
9 rules 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…e Converse) User confirmed 2026-07-11. c24 and the Model-access decision now name the native Bedrock Converse API (POST /model/us.amazon.nova-pro-v1:0/converse, Bearer Bedrock API key); the addendum is retained as the evidence trail and frame question q1 is resolved. h11 holds — config only, no provider SDK. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
SonarCloud flagged _validate_cloze_exercise (28) and _probe_cloze_items (37) as CRITICAL cognitive-complexity issues, both over the allowed 15 (python:S3776). Extract each distinct validation/probing concern into a small, docstringed helper so both top-level functions become short orchestrators: - _validate_cloze_exercise: _validate_cloze_top_level_fields (text/item_id checks), _validate_cloze_blank_id (id presence/uniqueness/placeholder match), _validate_cloze_blank_answer (options/answer checks), _validate_cloze_blank (per-blank shape + id + answer), and _validate_cloze_orphan_placeholders (text placeholders with no blank). - _probe_cloze_items: _read_story_exercises (one story's story-read + parse), _check_cloze_exercise_id_reuse (cross-story id dedup), _validate_cloze_in_story (one exercise's validation + dedup), _probe_story_cloze_items (one story's exercise loop), and _summarize_cloze_check (final check assembly). Behavior-preserving: identical error strings, error ordering, and return shapes throughout — no test needed to change. Full suite stays green (568 passed, 12 skipped) plus black/isort/flake8/bandit and the `teken cli doctor . --strict` rubric gate all pass.
… table Qodo finding (BUG 2, performance): DynamoSessionStore.count_active() used table.scan(FilterExpression=...) to count active sessions for the $connect concurrency gate. Sessions and inbound audio frames share one DynamoDB table, and frames are the hot path (thousands of writes per session), so a Scan read every frame item before filtering — $connect latency/cost grew with frame volume instead of staying bounded by the (small, cap-limited) session count. Fix: move session-metadata items to a constant partition key (PK="ACTIVE", SK=<connectionId>) instead of PK=SESSION#<connection>/SK=METADATA. Frame items keep their existing PK=SESSION#<connection> shape untouched. count_active() now does table.query(Select="COUNT", KeyConditionExpression="PK = :active", ...) against that one partition, which never touches frame items. put_session/get_session/delete_session updated to the new key shape; no template.yaml schema change needed (PK/SK stay String HASH/RANGE on the base table — no GSI required). Test-first: added FakeDynamoTable (a boto3 Table-resource stand-in that records scan()/query() calls) and four new tests against DynamoSessionStore directly, including test_count_active_queries_the_constant_pk_and_ignores_frame_items, which seeds 3 metadata items alongside 1000 frame items and asserts the count is exactly 3, scan() is never called, and query() is called once keyed on PK="ACTIVE". 568 -> 572 passed, 12 skipped. black/isort/flake8 clean; bandit findings unchanged (4 pre-existing low-severity, none introduced by this change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
…oice budget race BUG 1 (security): POST /api/delete only revoked the calling session's own sid tombstone, so a learner's other still-valid sessions (another tab, a CLI token, a session /api/me had refreshed) survived a self-delete. handleDelete now also stamps a per-uid `revoked_uid:<uid>` KV marker with the delete epoch second; auth.js#requireAuth rejects any token for that uid whose `iat` predates it (strict `<`, not `<=`, so a resignup landing in the same epoch-second as the delete is never false-revoked — see that function's comment for why). session.js#issueSession gained an optional `opts.now` override (mirroring voice.js#mintVoiceToken's own test-only override) so tests can pin `iat` deterministically instead of relying on wall-clock luck. BUG 3 (reliability): handleVoiceToken's monthly voice-budget cap was a plain read-check-write — two concurrent mints could both read the same `used`, both pass the cap check, and both write, minting two sessions' worth of budget for one booking. db.js#setLearnerVoiceUsage is now a compare-and-swap on the learner row's `updated_at` (`WHERE github_user_id = ? AND updated_at = ?`); index.js#bookVoiceUsage retries (bounded, 3 attempts) against a fresh read on a lost CAS and refuses the mint — without leaking the already-minted token — rather than risk exceeding VOICE_MONTHLY_SECONDS_CAP. test/helpers.js's D1 stub models the conditional UPDATE so the race is provably closed without wrangler or real concurrency. New tests: test/auth.test.js (deterministic requireAuth revocation-boundary unit tests), test/export-delete.test.js (multi-session delete revocation + cross-learner isolation), test/db.test.js (setLearnerVoiceUsage CAS unit tests), test/voice.test.js (real concurrent-mint Promise.all test — verified to fail against the pre-fix code, both mints landing 200 and overshooting the cap). 168 -> 183 worker tests, all green; pytest stays 568 passed / 12 skipped. README.md and tools/launch-gate/ docs updated to match (test count, revocation model, atomic-booking behavior). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014QpvZY8K67Q9uGA6keFhTz
|
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.
The consent-tutoring uplift — one coherent phase over #8–#11
Turns
agentculture.org/learnfrom an open sign-in that persists identity withno consent into a consent-first, role-aware tutoring service. Specced with
devague
/think, planned with/spec-to-plan(converged on PR #12), and builtvia
/assign-to-workforce— 18 tasks, 8 dependency waves, one worktree pertask, every merge TDD-gated (tests green before and after).
Closes #8 (experience), #9 (content), #10 (consent gate), #11 (Terms + Privacy).
What ships
issue a short-lived pending-consent session and write zero to D1 until
the learner accepts the published Terms/Privacy — the consent row is recorded
before the learner row (
consentstable, deliberately no FK). ATERMS_VERSIONbump routes a live session back to consent(
403 consent_required,reason: stale_version).GET /api/export(whole-learner JSON acrossevery subject + consent history) and
POST /api/delete(confirm = owngithub id) erasing all three tables and revoking the session.
ADMIN_GITHUB_IDSserver-side allow-list, adminroster, default-private visibility, and an approval-gated Bedrock tutoring
tier: a four-level gate (signed-out < consented < approved < configured)
where a non-approved
/api/tutorcall403s with zero outbound inference;approve is c20-gated on current consent.
and personalized cloze story generation through the existing broker
(config only, no provider SDK in the Worker — h11); an approval-gated
POST /api/voice/tokenmint with a monthly budget, the/learn/voicepage,and a serverless SAM voice bridge under
infra/(API Gateway WebSocket +arm64 Lambda + a $20 AWS Budgets ceiling, zero idle).
shared/terms-version.mjssource, naming GitHub, Cloudflare, and AWS Bedrockas processors, matching what the code persists (no email, no password); a
five-state
/learn/consentpage. Served /learn-local (org gets a link-upissue, already filed).
subject-plugin contract (§3.6.1), shipped upstream by french-cli (feat: the learn platform — three faces, three subjects, signed split, launch gate (learn uplift) #4),
spanish-cli (feat: the learn platform — three faces, three subjects, signed split, launch gate (learn uplift) #4), and culture-guide (spec+plan: consent-first, role-aware tutoring uplift (#8-#11) #12) — all merged + released — and
re-exported to /learn.
tools/launch-gate/consent_walk.mjsdrivesthe consent/approval/deletion/tutor/voice/cloze success signals end-to-end
(LOCAL authed flows 10/10; LIVE unauthenticated probes), plus always-on
mechanical boundary invariants (
tests/test_launch_gate_invariants.py:no email/password, no provider SDK, no forked subject prose). The recorded
pre-uplift
BASELINE-2026-07-11.mdshows the LIVE probes 9/10 fail againsttoday's prod — the h17 "distinguishes shipped from not-shipped" evidence.
c24 (and the "Model access is Bedrock-direct" decision) say Nova Pro tutoring
points
INFERENCE_URLat Bedrock's OpenAI-compatible endpoint(
…/openai/v1). A live probe on 2026-07-11 proved that endpoint returnsmodel_not_foundfor Nova Pro in every region tried, and/openai/v1/modelsisn't even an operation. The native Bedrock Converse API
(
POST /model/us.amazon.nova-pro-v1:0/converse, Bearer Bedrock API key) doesreturn Nova Pro completions and tolerates the broker's
learnerstamp — soh11 still holds (config-only, no SDK); only the endpoint URL/shape changes.
The shipped code already targets Converse; the spec carries a post-convergence
correction note on c24 (annotating, not rewriting the converged claim) plus
frame
q1. Please confirm the retarget so I can fold it into the claim.Test evidence (all green)
node --test, in-memory KV/D1, zero-network).npm run build+npm run checkgreen (static-auth 4-way whitelist,export-pages, 16 tutor-logic checks).
consent_walk.mjsLOCAL 10/10; LIVE 9/10 (baseline, pre-deploy).teken cli doctor --strictPASS; markdownlint 0.
Not in this PR — the supervised deploy (auto-mode blocks it)
Merging auto-deploys Pages (site) via
deploy-site.yml. The Worker +AWS still need a supervised, outside-auto-mode step:
wrangler deployfrommerged main,
wrangler secret put GITHUB_CLIENT_ID(from.envGITHUB_APP_CLIENT_ID), flipINFERENCE_URLto the Converse URL + setINFERENCE_TOKEN, thenVOICE_TOKEN_SECRET+sam deployfor voice. Onlyafter the policy docs are live and consented may the first learner
(gh-id 20955789) be approved (c20). The green
LIVE_ORIGIN=https://agentculture.orglaunch-gate run — recorded while thepublic nav link is live — is the gate the uplift ships behind. Runbook:
workers/learn-api/README.md+tools/launch-gate/BASELINE-2026-07-11.md.