Skip to content

fix(rich): D5.1 publish-boundary Rich trust closure - #694

Merged
jnhu76 merged 2 commits into
masterfrom
fix/669-rich-d5-1-publish-trust
Oct 3, 2026
Merged

jnhu76 merged 2 commits into
masterfrom
fix/669-rich-d5-1-publish-trust

Conversation

@jnhu76

@jnhu76 jnhu76 commented Oct 3, 2026

Copy link
Copy Markdown
Owner

This closes a latent publish-boundary trust gap found during D5.
It does not change Rich authority or publication semantics.

Refs: #669 (Phase D5.1), #693 (D5), ADR-019, rich-content-semantic-contract.md

Root cause

publishExam projected repository-loaded question.contentDocument /
option.contentDocument straight through plainTextProjection, assuming the
TypeScript ContentDocumentV1 shape was already trustworthy. A corrupt
historical/bypassed row (the canonical write seam can never emit one) reached
the projection and raised TypeError ("inlines is not iterable") → HTTP 500
instead of a controlled publish rejection.

Classification: P3, PUBLISH_BOUNDARY_TRUST_GAP, supported-writer
reachability NO, corrupt-historical reachability YES.

Change

The publish freeze gate now consumes the existing Phase D5 semantic
classification (classifyPersistedQuestionContent in @exam/contracts) before
any projection — no second Rich validity oracle was introduced:

  • rich_valid → unchanged content == plainTextProjection(document) invariant
    (question and option messages byte-identical).
  • rich_noncanonical → ValidationError (canonicality-at-publish derived from
    frozen authority: ADR-019 single-write-seam rule + contract §2/§8 — publish
    creates a new frozen commitment, and only canonical Rich may be frozen; the
    D5 DISPLAY grant never conferred canonicality).
  • unsupported_version / corrupt → typed ValidationError; no projection,
    no fallback to the stored content compatibility field, no raw payloads in
    messages.
  • Plain (contentDocument == null) branch untouched.
  • Validation-before-freeze: buildQuestionSnapshot now runs only after every
    per-question trust/invariant check passes, so a corrupt row is never
    materialized into a provisional snapshot.

No repair writes: publish validates then freezes; a noncanonical row is never
normalized (repair is a separate explicit migration).

Call-site classification (publish-time plainTextProjection on repository rows)

Call site Class Action
examCommands.ts question path TRUST_CHECK_REQUIRED fixed
examCommands.ts option path TRUST_CHECK_REQUIRED fixed (parity)
contracts/src/question.ts (×2) TRUSTED_BY_WRITE_BOUNDARY (request data parsed by ContentSlotSchema in the same zod pipeline) none
apps/api/src/lib/attemptExportAnswer.ts TRUSTED_BY_D4_CLASSIFIER (projects only classified read.document) none
apps/api/src/routes/questionContent.ts (×2) TRUSTED_BY_CANONICALIZER (projects canonicalizeContentDocument success value) none
apps/web/.../contentAdapter.ts (×2) client authoring adapter, freshly constructed editor blocks — not a repository publish seam out of scope

Package dependency decision

@exam/exam-engine gains @exam/contracts (acyclic: contracts → domain only;
lint:arch forbids neither edge). One Rich validity authority preserved — the
engine imports the D5 classifier directly instead of duplicating it.

Regressions (engine level; exact error class asserted)

  • R1 canonical rich question publishes (positive control)
  • R2 corrupt question envelope → ValidationError, not the pre-fix
    TypeError (red run captured "inlines is not iterable")
  • R3 unsupported question docVersion: 2 → typed rejection
  • R4 corrupt option document → ValidationError (mandatory option path)
  • R5 unsupported option docVersion parity
  • R6 corrupt document + plausible content string → still rejected (no Plain
    fallback; B′ ownership)
  • R7 projection-mismatch rejections — pre-existing regressions unchanged and
    green
  • R8 schema-valid but noncanonical document whose content matches its own raw
    projection → rejected by the canonicality policy itself

Route-level mapping ValidationError → 400 VALIDATION_ERROR for the publish
surface is already permanently covered by
apps/api/src/routes/examPolicyValidation.test.ts; the central handler sends
any non-AppError (the old TypeError) to 500, which is exactly the channel
this gate closes.

Verification

  • Red → green TDD; pnpm --filter @exam/exam-engine test 762 passed
  • pnpm verify PASS (verify:static incl. lint:arch, eslint, typecheck,
    openapi; full workspace coverage on real PostgreSQL; build)
  • Exact-head CI: pending on push

jnhu76 added 2 commits October 3, 2026 19:10
publishExam projected repository-loaded question/option contentDocument
straight through plainTextProjection, so a corrupt historical/bypassed
row (the canonical write seam can never emit one) crashed the freeze
gate with a TypeError instead of a controlled publish rejection.

The gate now classifies every repository-loaded document through the
shared Phase D5 read authority (classifyPersistedQuestionContent):
rich_valid proceeds to the unchanged projection invariant;
rich_noncanonical / unsupported_version / corrupt fail closed as typed
ValidationErrors, and the frozen snapshot is built only after all
per-question trust checks pass (validation-before-freeze).

Canonicality at publish follows from frozen authority (ADR-019
single-write-seam; semantic contract §2/§8): publish creates a new
frozen commitment, and only canonical Rich may be frozen. As-built
references recorded in the semantic contract (§6/§18) and ADR-019
compliance notes; no authority semantics changed.

#669 (Phase D5.1, follows #693 / D5)
R1 canonical rich publish positive control; R2/R4 corrupt question and
option documents (fabricated bypassed rows via the established
as-unknown cast pattern) reject as ValidationError, not the pre-fix
TypeError ('inlines is not iterable') that would surface as an HTTP
500; R3/R5 unsupported docVersion parity for question and option; R6
proves no fallback to the stored content projection field; R8 proves
the noncanonical rejection is the canonicality policy itself — the
fixture's content matches the raw document's projection, so only the
freeze-gate policy can reject it. The pre-existing projection-mismatch
regressions (R7) are unchanged and still green.

#669 (Phase D5.1)
@coderabbitai

coderabbitai Bot commented Oct 3, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c4dc4913-a88b-4f1f-aa05-2e172662c8bd
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

jnhu76 commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

Independent focused review — D5.1 publish-boundary Rich trust

Verdict: PASS / MERGE RECOMMENDED.

I independently checked the current head 2994c10e7ee29eb5f5948822973877e6fd034be4, the publish implementation, the D5 classifier reuse, the question/option regressions, the package dependency edge, the authority notes, and exact-head checks.

Findings

  • Question path: PASS. A non-null repository-loaded question.contentDocument is classified by the shared classifyPersistedQuestionContent authority before plainTextProjection; only rich_valid proceeds.
  • Option path: PASS. The exact same helper protects option.contentDocument; no question/option asymmetry remains.
  • Corrupt / unsupported: PASS. Both become typed ValidationErrors and cannot reach V1 projection or fall back to the stored content compatibility field.
  • Noncanonical publish policy: PASS. Rejecting rich_noncanonical at new publication/freeze is consistent with ADR-019's single canonical write seam and the Rich contract's canonical-persistent-truth model. This does not revoke D5's read-only DISPLAY permission for already-existing historical noncanonical content.
  • Projection invariant: PRESERVED. Valid canonical Rich still goes through the existing byte-for-byte content == plainTextProjection(document) check.
  • No repair write: PASS. Publish validates/fails closed; it does not normalize or persist a repaired historical row.
  • Single semantic authority: PASS. exam-engine imports the existing D5 classifier rather than duplicating docVersion/preflight/schema/canonicality logic. The new exam-engine -> contracts -> domain runtime dependency is acyclic and exact-head static architecture checks pass.
  • Regression evidence: PASS. R1–R8 cover canonical positive control, corrupt/unsupported question and option rows, no-Plain-fallback, preserved projection mismatch, and noncanonical canonicality rejection.

Non-blocking observation

Moving buildQuestionSnapshot until after all per-question checks is safe and keeps corrupt Rich out of even a provisional in-memory snapshot. It can change which ValidationError wins when a single fabricated input contains multiple independent publish defects (for example a missing question id plus a corrupt supplied row). I found no frozen/public error-precedence contract for that combination, so this is not a blocker, but Phase E should include failure-precedence combinations rather than assuming all validation-order permutations are semantically irrelevant.

Disposition

D5_1_QUESTION_TRUST        = PASS
D5_1_OPTION_TRUST          = PASS
NONCANONICAL_PUBLISH       = PASS
CORRUPT_TYPED_FAILURE      = PASS
UNSUPPORTED_TYPED_FAILURE  = PASS
PLAIN_FALLBACK_BLOCKED     = PASS
PROJECTION_INVARIANT       = PASS
DUPLICATE_RICH_ORACLE      = NONE
REPAIR_WRITE               = NONE
AUTHORITY_CONFLICT         = NONE

PHASE_D5_1                 = PASS
MERGE_RECOMMENDATION       = MERGE
READY_FOR_PHASE_E          = YES_AFTER_MERGE
GATE_1                     = NOT_EVALUATED

Exact-head GitHub Actions are green and CodeRabbit is success. No further D5.1 correction requested.

@jnhu76

jnhu76 commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

Re the review observation (snapshot-move changing multi-defect error precedence): adjudicated with an independent evidence pass — no fix needed, mergeable as-is.

  • Order delta is real but unobserved: the move flips missing-question (Question X not found) to fire after courseId / fill_blank / answerMode / Rich trust / projection / rubric / standardAnswer checks. Nothing pins the old order: the contract's §13 failure-precedence chain is SaveAnswer-scoped (publish is not in it); exam-semantic-boundaries.md only requires that publish rejects a missing question, not when; no test publishes a missing-question-plus-other-defect exam.
  • Wire-level delta is zero: every flipped defect throws ValidationError → AppError(code="VALIDATION_ERROR", statusCode=400), and the central handler replies with the registry message for the code — the engine message string never reaches the response body (buildErrorResponse sends details === undefined; D0.5 invariant) and the AppError branch doesn't log. A multi-defect publish returns byte-identical {400, VALIDATION_ERROR} before and after the change; the ordering difference is observable only in engine-level unit assertions that don't exist.
  • Reachability requires two independent corruptions: stale exam.questionIds after a question delete (the DELETE path never scrubs references — a pre-existing, separate gap) plus a Rich row that bypassed the single write seam. No supported-writer path produces the state.
  • Restoring the old precedence (option B) would need a pre-loop referential check duplicating buildQuestionSnapshot's guard — a second writer of the same fact — for zero wire-observable benefit; reverting (option C) would churn a green commit whose ordering now matches the D5.1 as-built "validates then freezes" statement.

So the observation is accepted as correct-but-inconsequential; no rework. (The dangling-reference scrub on question delete is a separate pre-existing issue, not introduced or expanded here.)

@jnhu76
jnhu76 merged commit 67079ef into master Oct 3, 2026
11 checks passed
@jnhu76
jnhu76 deleted the fix/669-rich-d5-1-publish-trust branch October 3, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant