query-orchestration/plans/mutable.metadata.testing.review.codex.md

# Mutable Metadata v4 Testing Review

Reviewed:
- `plans/mutable.metadata.plan.combo.v4.md`
- `plans/mutable.metadata.plan.combo.v4.tracking.md`

## Findings

1. High: `GET /custom-metadata/version` and `GET /custom-metadata/history` are externally facing endpoints in the plan, but the tracking doc does not explicitly carry them into Milestone 2 integration coverage.
Plan evidence:
- `plans/mutable.metadata.plan.combo.v4.md:950-976` defines both endpoints.
- `plans/mutable.metadata.plan.combo.v4.md:1584` requires HTTP-level integration tests for all new `/custom-metadata/*` endpoints.
Tracking evidence:
- `plans/mutable.metadata.plan.combo.v4.tracking.md:339-340` adds handler-level TDD coverage only.
- `plans/mutable.metadata.plan.combo.v4.tracking.md:351-360` does not call out either endpoint in Milestone 2 integration tests.
- `plans/mutable.metadata.plan.combo.v4.tracking.md:356` references `/custom-metadata` generically, but does not explicitly track `/version` or `/history` route/method cells.
Why this matters:
- Milestone 2 can be marked complete without HTTP-level proof that version lookup, history pagination, and role gating work on those two routes.
Recommended tracking fix:
- Add explicit Milestone 2 integration tasks for:
  - `GET /custom-metadata/version` happy path
  - `GET /custom-metadata/history` happy path with pagination assertions
  - authorization cells for both routes

2. High: The `GET /document/{id}` enrichment is documented as a user-visible feature and explicitly called out for integration verification in the plan, but the tracking doc only carries it at unit/controller level.
Plan evidence:
- `plans/mutable.metadata.plan.combo.v4.md:686-708` defines the externally visible `customSchemaId` and `hasCustomMetadata` additions.
- `plans/mutable.metadata.plan.combo.v4.md:1530` says Milestone 2 integration tests should verify that `GET /document/{id}` reflects the new fields.
- `plans/mutable.metadata.plan.combo.v4.md:722` includes `GET /document/{id}` enrichment in the metadata-read authorization surface.
Tracking evidence:
- `plans/mutable.metadata.plan.combo.v4.tracking.md:342-346` covers builder/controller TDD only.
- `plans/mutable.metadata.plan.combo.v4.tracking.md:351-360` has no explicit Milestone 2 integration item for `GET /document/{id}`.
- `plans/mutable.metadata.plan.combo.v4.tracking.md:356` does not explicitly cover the existing `document` resource behavior for this modified endpoint.
Why this matters:
- The modified read path could regress, or be wired to the wrong resource behavior, without blocking milestone signoff.
Recommended tracking fix:
- Add explicit Milestone 2 integration tasks for:
  - document with no schema -> `customSchemaId: null`, `hasCustomMetadata: false`
  - document with schema but no metadata -> bound schema ID, `hasCustomMetadata: false`
  - document with schema and metadata -> bound schema ID, `hasCustomMetadata: true`
  - role coverage confirming `GET /document/{id}` remains on the existing `document` resource behavior

3. High: The reset-metadata endpoint's documented HTTP contract is only partially tracked at integration level.
Plan evidence:
- `plans/mutable.metadata.plan.combo.v4.md:620-673` documents the endpoint, including `200`, `404`, and `409` behaviors.
- `plans/mutable.metadata.plan.combo.v4.md:1612-1619` explicitly requires integration coverage for the happy-path response assertions, legacy-extraction guard (`409`), and document-not-found (`404`).
Tracking evidence:
- `plans/mutable.metadata.plan.combo.v4.tracking.md:389-415` covers these branches at service/handler TDD level.
- `plans/mutable.metadata.plan.combo.v4.tracking.md:419-423` only tracks the upgrade flow, already-clean idempotency case, schema-version concurrency, auth matrix, and fullsuite gate.
Why this matters:
- Milestone 3 can be declared complete without HTTP-level verification of two documented error responses.
- The current happy-path integration item also under-specifies the response assertions the plan requires (`previousSchemaId`, `previousSchemaName`, `previousSchemaVersion`, `metadataVersionsDeleted`, and the post-upgrade `GET /document/{id}` check).
Recommended tracking fix:
- Add explicit Milestone 3 integration tasks for:
  - reset on legacy-extraction document -> `409`, DB unchanged
  - reset on non-existent document -> `404`, DB unchanged
  - reset happy path asserts the documented response fields and verifies the follow-up `GET /document/{id}` state after rebind

4. Medium: Two documented HTTP behaviors are tracked only below the integration layer: clearing a document schema binding with `customSchemaId: null`, and the server-derived actor identity contract on write endpoints.
Plan evidence:
- `plans/mutable.metadata.plan.combo.v4.md:571-576` documents `PATCH /super-admin/documents/{id}/schema` with `customSchemaId: null` removing the binding when no metadata exists.
- `plans/mutable.metadata.plan.combo.v4.md:867-869` and `plans/mutable.metadata.plan.combo.v4.md:1604` make actor identity a user-visible contract: request-body `createdBy` is ignored, stored/returned actor fields come from the JWT subject.
Tracking evidence:
- `plans/mutable.metadata.plan.combo.v4.tracking.md:293` tracks the null-unbind path at service TDD level, but `plans/mutable.metadata.plan.combo.v4.tracking.md:351-360` has no HTTP integration item for it.
- `plans/mutable.metadata.plan.combo.v4.tracking.md:194`, `plans/mutable.metadata.plan.combo.v4.tracking.md:336`, and `plans/mutable.metadata.plan.combo.v4.tracking.md:411` track actor handling in controller/handler tests, but no milestone integration section explicitly carries the contract.
Why this matters:
- The API could pass service/controller tests while still missing the actual HTTP request/response behavior promised to callers.
Recommended tracking fix:
- Add a Milestone 2 integration case for `PATCH /super-admin/documents/{id}/schema` with `{"customSchemaId": null}` on a pre-bound document with no metadata.
- Add integration cases for the write endpoints that expose actor fields in persisted data or responses, at minimum `POST /super-admin/custom-schemas`, `POST /super-admin/custom-schemas/{schemaId}/versions`, `POST /custom-metadata`, and `POST /super-admin/documents/{id}/reset-metadata`.

## Verified

- The tracking doc already encodes the fail-first TDD rule and the "do not mark done until tests pass" rule:
  - `plans/mutable.metadata.plan.combo.v4.tracking.md:5`
  - `plans/mutable.metadata.plan.combo.v4.tracking.md:26`
  - `plans/mutable.metadata.plan.combo.v4.tracking.md:31`
  - `plans/mutable.metadata.plan.combo.v4.tracking.md:508-510`
- I did not find conflicting language that would allow a checkbox or milestone to be marked complete before the associated tests are passing.