Skip to content

feat(hitl): record decision-time review context - #234

Open
shuntianyifang wants to merge 2 commits into
theam:mainfrom
shuntianyifang:codex/record-hitl-review-context
Open

feat(hitl): record decision-time review context#234
shuntianyifang wants to merge 2 commits into
theam:mainfrom
shuntianyifang:codex/record-hitl-review-context

Conversation

@shuntianyifang

@shuntianyifang shuntianyifang commented Aug 31, 2026

Copy link
Copy Markdown

Summary

  • add a versioned ReviewContextV1 snapshot for plan_acceptance decisions
  • make Web approval/rejection a two-step flow: present server-created evidence, then reference that exact event when deciding
  • show repository, branch, plan base SHA, presented SHA, drift, and changed paths beside the confirmation controls
  • store the full context in review_presented and copy it into approved/rejected events for self-contained auditability
  • publish the same context with GitHub architect plans and make /builder copy only that immutable publication evidence
  • reject legacy GitHub proposals without publication evidence and preserve the existing Builder freshness gate
  • generate the OpenAPI/SDK surface without adding a database migration

This follows up on the audit gap discussed in #147 and on the provenance work in #205.

Compared with the original #147 comment, this PR includes the minimum UI binding needed to prove what was actually presented. It does not add drift thresholds, override policy, or other admission policy.

Refs #147
Follow-up to #205

Queue-comment workaround

This PR does not change queued-comment behavior and does not include #223. Deployment/test projects should keep:

{
  "execution_lane": {
    "architect": "platform",
    "builder": "platform",
    "codex-architect": "platform",
    "codex-builder": "platform"
  }
}

Tests should enter through GitHub /architect or /codex-architect, not the direct console or MCP proposal trigger. A deployment-test identity may also omit runs:trigger while retaining hitl:decide.

Verification

Automated verification passed:

  • isolated Linux pnpm verify on the main implementation
  • pnpm typecheck
  • NEXT_TELEMETRY_DISABLED=1 pnpm build
  • review-context unit/API tests: 4 passed
  • Builder plan policy integration: 31 passed
  • GitHub platform-lane integration: 63 passed
  • Web review-context rendering and request regression tests: 3 passed
  • DB critical tests: 18 passed
  • Gateway critical tests: 57 passed
  • committed OpenAPI contract equivalence
  • targeted Biome checks and git diff --check
    After the follow-up fix that omits Content-Type from the bodyless review-context request, the targeted Web tests, Web typecheck, production Web build, Biome checks, and git diff --check were rerun successfully.
    Live Docker Compose verification passed:
  • API health and readiness checks
  • CLI doctor: 11/11 checks passed
  • smoke test with the configured platform lane
  • GitHub webhook and OAuth flows
  • Web rejection copied the complete review_presented context into the rejected event without creating a Builder
  • Web approval copied the complete context into the approved event
  • expired review contexts were rejected and required presentation of a new context
  • moving the branch after presentation did not alter the stored evidence
  • the existing freshness gate blocked execution with stale provenance
  • rerunning the architect flow with fresh provenance allowed the Builder to run successfully
  • GitHub publication evidence was copied unchanged by the /codex-builder approval path
  • the successful Builder run passed its own lint, typecheck, 40 unit tests, 10 end-to-end tests, and production build
  • the successful Builder produced test PR shuntianyifang/facility-test-app#5
  • the queued-comment configuration workaround did not produce the unrelated unmarked queue comments
    Windows-only repository limitations observed:
  • the repository's test:critical launcher calls spawnSync("pnpm"), which does not resolve pnpm.cmd
  • the existing Windows seed path normalization stores bundled prompt names with backslashes, so the full API suite cannot find prompts/architect fixtures
  • the full Web suite passes 71/72; the only failure is the pre-existing, unrelated claude.svg byte-hash mismatch

@shuntianyifang
shuntianyifang force-pushed the codex/record-hitl-review-context branch from 2fb11b8 to eaa5efa Compare September 1, 2026 18:03
@shuntianyifang
shuntianyifang marked this pull request as ready for review September 1, 2026 18:08
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