feat(coordinator): backfill worker compatibility intake + conversion-ready auto-session（implement spec drift） - #85
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR implements worker-compatibility intake normalization and automatic local review session creation on conversion-ready. The coordinator now accepts simplified on-prem IFC worker payload formats, normalizes them to canonical shape with derived correlation/idempotency keys, and auto-creates active review sessions when conversions reach terminal ready status with idempotent re-entry semantics. ChangesCoordinator webhook and auto-session handoff
Sequence Diagram(s)sequenceDiagram
participant Client as IFC Worker Client
participant Handler as /api/external/ifc-ready Handler
participant Normalizer as normalizeIntakePayload
participant Canonical as Canonical ExternalIfcReadyEvent
participant Dispatcher as Streaming Conversion Dispatcher
Client->>Handler: POST worker payload (status, ifc_path, project_id, version, task_id)
Handler->>Normalizer: parse body as worker or canonical format
Normalizer->>Normalizer: detect format by presence of 'event' vs 'status'
Normalizer->>Canonical: map version→external_model_version_id, task_id→external_conversion_task_id
Normalizer->>Normalizer: derive correlation_id from project_id::version::task_id if header absent
Canonical->>Handler: normalized ExternalIfcReadyEvent shape
Handler->>Handler: inject derived x-correlation-id/x-idempotency-key when headers missing
Handler->>Dispatcher: dispatch canonical event to internal conversion API
Dispatcher-->>Handler: 202 accepted
Handler-->>Client: 202 with ifc_ready_job_id
sequenceDiagram
participant Report as Conversion Report Ingest
participant Ingestion as ingestConversionReport
participant SessionHelper as autoCreateOrActivateSession
participant Store as ExternalIfcReadyStore
participant Sessions as Review Sessions Service
participant Callback as Callback Outbox
Report->>Ingestion: POST conversion result (status: ready)
Ingestion->>SessionHelper: check if ready status and usdc_ref present
SessionHelper->>Store: lookup ifc_ready_job by jobId
Store->>SessionHelper: return job with review_session_id if exists
SessionHelper->>Sessions: create or fetch active session (idempotent)
Sessions->>Sessions: allocate kit instance binding, emit lifecycle events
Sessions->>SessionHelper: return session_id and replay flag
SessionHelper->>Store: recordReviewSession(jobId, session_id)
Store->>SessionHelper: updated job with review_session_id
Ingestion->>Callback: enqueue conversion_ready callback (parallel, decoupled)
Ingestion-->>Report: 202 with session, session_replay, session_reason
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR implements the “apply” phase for the archived spec drift around coordinator IFC-ready intake + conversion-ready auto-session handoff, limited to the bim-review-coordinator/ control-plane and accompanying contract/docs evidence.
Changes:
- Added worker-compatibility intake normalization so
/api/external/ifc-readycan accept both canonical and simplified worker payload shapes and still dispatch the canonical internal streaming contract. - Added conversion-ready auto-session handoff wiring in the conversion ingest path, with idempotent replay via
review_session_idback-reference. - Added/updated tests and verification evidence + roadmap notes to demonstrate coverage of the ratified scenarios.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
bim-review-coordinator/src/app.ts |
Adds worker payload normalization + derived header injection, and auto-session creation during terminal conversion-ready ingestion with additive response fields. |
bim-review-coordinator/src/services/externalIfcReadyStore.ts |
Stores review_session_id back-reference for idempotent auto-session replay. |
bim-review-coordinator/src/types.ts |
Extends IfcReadyIntakeJob with optional review_session_id. |
bim-review-coordinator/tests/external-ifc-ready.test.ts |
Adds test coverage for worker-compat payload acceptance, rejection, idempotency derivation, and “no shape leak” dispatch. |
bim-review-coordinator/tests/host-native-conversion-ingest.test.ts |
Adds test coverage for conversion-ready auto-session creation, idempotency, and webhook/outbox vs session seam behavior. |
tests/contracts/ifc_ready_payload.json |
Adds a worker-compatibility payload example and mapping notes alongside the canonical contract. |
tests/fakes/external_ifc_worker_client.py |
Adds a helper to generate worker-compatibility payloads from the frozen contract. |
openspec/changes/backfill-coordinator-webhook-and-auto-session/tasks.md |
Marks apply tasks as completed and documents the post-merge gate. |
docs/verification/2026-05-21-backfill-coordinator-webhook-and-auto-session.md |
Adds verification write-up mapping spec scenarios to tests and evidence commands. |
docs/plans/AI-BIM-governance-saas-roadmap-2026-05.md |
Adds a roadmap entry summarizing the backfill and its verification status. |
docs/plans/AI-BIM-governance-saas-roadmap-2026-05.html |
Regenerates the HTML mirror to include the new roadmap entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| format: "ifc", | ||
| }, | ||
| requested_outputs: ["usdc", "element_mapping", "entity_index", "metadata"], | ||
| callback_url: typeof body.callback_url === "string" ? body.callback_url : null, | ||
| }; |
| const result = autoCreateOrActivateSession( | ||
| sessionJob, | ||
| { | ||
| usdc_ref: report.artifacts?.usdc_ref ?? null, | ||
| element_mapping_ref: report.artifacts?.element_mapping_ref ?? null, | ||
| manifest_ref: report.artifacts?.manifest_ref ?? null, | ||
| }, | ||
| conversionJobId, | ||
| ); | ||
| if (result.session) { | ||
| session = result.session; | ||
| session_replay = result.replay; | ||
| } else { | ||
| session_reason = result.reason; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee474bfef3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const tenantId = typeof body.tenant_id === "string" && body.tenant_id.trim().length > 0 | ||
| ? body.tenant_id.trim() | ||
| : "tenant_demo_001"; |
There was a problem hiding this comment.
In normalizeIntakePayload, any non-string tenant_id in worker-compat payloads is silently coerced to tenant_demo_001 instead of being rejected. This means malformed inputs (for example tenant_id: 0 or {}) are accepted and recorded under the wrong tenant, which can mis-associate conversion jobs and idempotent replays across tenants rather than returning a client error. The fallback should apply only when tenant_id is truly absent, while invalid typed values should fail validation with 4xx.
Useful? React with 👍 / 👎.
| format: "ifc", | ||
| }, | ||
| requested_outputs: ["usdc", "element_mapping", "entity_index", "metadata"], | ||
| callback_url: typeof body.callback_url === "string" ? body.callback_url : null, |
There was a problem hiding this comment.
The worker-compat path copies callback_url as any string without URL validation, unlike canonical payload parsing. If a worker sends a malformed value (for example "not-a-url"), resolveAllowedCallbackTarget later calls new URL(...) and throws, which is handled as a 500 instead of a request-validation 4xx. This creates an avoidable server error path for bad client input and diverges from canonical intake behavior.
Useful? React with 👍 / 👎.
| if (job.review_session_id) { | ||
| const existing = store.get(job.review_session_id); | ||
| if (existing) { | ||
| return { session: existing, replay: true }; |
There was a problem hiding this comment.
autoCreateOrActivateSession performs a check-then-create flow on job.review_session_id without synchronization, so concurrent terminal-ready ingests for the same correlation can both observe no bound session and each create a new active review session before either call records review_session_id. This violates idempotency under concurrent retries/dual ingest triggers and can double-allocate Kit capacity for one conversion result.
Useful? React with 👍 / 👎.
…ready auto-session（spec drift 收斂） 補 archive 2026-05-21-coordinator-ifc-ready-worker-webhook 的 documentation lag（retro-audit commit a32fcd6 確認 spec 已 ratified、code 從未實作）。 實作（bim-review-coordinator/src/app.ts）： - normalizeIntakePayload helper：external IFC-ready intake 支援 worker compatibility payload（status="ifc_ready" / ifc_path / project_id / version / task_id），正規化為 canonical ExternalIfcReadyEvent，不洩漏 進 streaming internal contract。worker compat 缺 X-Correlation-Id / X-Idempotency-Key 時從 worker:project_id::version::task_id 派生， explicit headers 仍優先（D11）。 - autoCreateOrActivateSession helper：ingestConversionReport terminal ready 分支於 callbackOutbox.enqueue 之後並行呼叫，重用既有 SessionStore.create + allocateKitInstanceBindings + chooseReadyUsdc 邏輯，對 job.review_session_id idempotent，重入回既有 session； terminal failed 不建可串流 session；GPU/Kit 無容量回 queued_for_instance 不丟 review intent。 - ExternalIfcReadyStore.recordReviewSession() 反向綁定 + IfcReadyIntakeJob review_session_id optional 欄位（additive）。 - /api/internal/conversion-result + /api/internal/conversions/:id/ingest response 加 session / session_replay / session_reason 欄位（additive）。 11 個 spec scenarios coverage（intake 4 + auto-session 4 + webhook seam 3） 皆對應 TDD-driven test：external-ifc-ready.test.ts 新增 7 cases、 host-native-conversion-ingest.test.ts 新增 6 cases。 Spec delta：採 Option B（NO-OP-ish MODIFIED re-affirm），三份 capability （local-coordinator-ifc-ready-intake-boundary / review-session-request- lifecycle / conversion-webhook-lifecycle）各加一段 "Implementation status (2026-05-21)" backfill note，scenarios 全保留不變、語意無變動。 Verification: - coordinator npm run verify: 11 files / 165 tests passed; tsc clean - root contracts pytest: 7 passed - openspec validate backfill-coordinator-webhook-and-auto-session --strict: valid - git diff --check: clean - Affected symbols 符合 §0.2 預期，無範圍擴張（fallback git diff --stat） - Render tier (single_kit_render / WebRTC 49100 / browser visual): not_observed (Kit build + GPU host 前置；memory kit-gpu-render-needs-windows-native) - OQ1 (雲端 callback endpoint/auth) / OQ5 (SSO) 仍 pending Evidence: - docs/verification/2026-05-21-backfill-coordinator-webhook-and-auto-session.md - roadmap md + html 同步更新（2026-05-21 backfill apply 通告） 不解 / 不升等: - 不修改 archive 2026-05-21-coordinator-ifc-ready-worker-webhook/tasks.md upgrade（依賴 retro-audit PR #83 先 merge，post-merge gate） - 不啟動 / 控制 Kit 進程、不開 USD stage、不渲染 - 不新增 production dependency Predecessor: 2026-05-21-coordinator-ifc-ready-worker-webhook (archive, documentation lag) Related: PR #83 retro-audit, PR #84 propose (this change) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ted (post-PR #83 merge) retro-audit PR #83 commit e8e576d 已合進 main，archive 2026-05-21-coordinator-ifc-ready-worker-webhook/tasks.md 的 26 個 deferred 標記源自該 commit。本 branch rebase onto latest main 後做 §5.3 post-merge gate 收尾。 改動: - openspec/changes/archive/2026-05-21-coordinator-ifc-ready-worker-webhook/tasks.md: 26 個 [ ] — **deferred** 升級為 [x] — **implemented** by PR #85 (was: **deferred**: ...)，原 retro-audit annotation 完整保留作為 documentation lag 證據；archive 開頭加 closeout 區塊指向 PR #85 evidence。 - openspec/changes/backfill-coordinator-webhook-and-auto-session/tasks.md: §5.3 自身狀態升 [x]。 Verification: - openspec validate backfill-coordinator-webhook-and-auto-session --strict: valid - 純文件改動，零 code 改動 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ee474bf to
d601599
Compare
… 並同步 specs (#86) Implementation PR #85（2026-05-21 merged）的 OpenSpec archive 對齊。 Spec sync 細節（25 capabilities 保持 25）： - conversion-webhook-lifecycle: ~1 modified - local-coordinator-ifc-ready-intake-boundary: ~1 modified - review-session-request-lifecycle: ~1 modified - Totals: + 0 added / ~ 3 modified / - 0 removed 把 archive 2026-05-21-coordinator-ifc-ready-worker-webhook 的 documentation lag 收斂：worker compatibility intake + conversion-ready 自動 review session handoff 已落地、11 個 spec scenarios 對應 TDD-driven test 全綠。三份 capability spec 各加一段 "Implementation status (2026-05-21)" backfill note， 標明 backfill 來源；scenarios 全保留不變、語意無變動（採 Option B NO-OP-ish MODIFIED re-affirm strategy）。 Verification: - openspec validate --specs --strict ✓ 25 passed / 0 failed - Archive folder 建於 openspec/changes/archive/2026-05-21-backfill-coordinator-webhook-and-auto-session/ - roadmap .md + .html 已同步更新 2026-05-21 archive 通告 Not solved by this archive（保留 deferred / not_observed）: - OQ1 雲端 callback 真實 endpoint/auth（凍結契約緩解） - OQ5 SSO - Kit/WebRTC/browser visual = not_observed（需 Kit build + GPU host 前置； 本 archive 不升等該 tier） Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d60159941e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (normalizedStatus === "ready") { | ||
| // 使用 updatedJob 取得包含 review_session_id 的最新 job 狀態。 | ||
| const sessionJob = updatedJob ?? job; | ||
| const result = autoCreateOrActivateSession( | ||
| sessionJob, |
There was a problem hiding this comment.
ingestConversionReport enqueues the callback and records conversion status before calling autoCreateOrActivateSession, but this call is not guarded. If session creation or lifecycle logging throws (for example due to filesystem write errors in SessionStore.create/EventLog.append), the request returns 500 even though the callback outbox side effect already happened; a retry then enqueues another callback entry because outbox enqueue has no dedupe on correlation_id. This breaks the intended callback/session independence and can produce duplicate cloud callbacks from a single conversion result.
Useful? React with 👍 / 👎.
Summary
實作 archive
2026-05-21-coordinator-ifc-ready-worker-webhook的 11 個 ratified-but-未實作 spec scenarios。retro-audit (PR #83) 確認 spec drift；propose (PR #84) 鎖定 backfill scope。本 PR 是 apply 階段，落地 code + tests + evidence。Stacked PR：base 為 PR #84 (propose) branch；待 PR #84 merge 後可將 base 切回 main。
改動範圍
純
bim-review-coordinator/（intake + control-plane）。零 production dependency，不啟動 / 控制 Kit / USD / render。Intake（worker compatibility payload）
bim-review-coordinator/src/app.ts:158-218：新增workerCompatPayloadSchema+normalizeIntakePayload(rawBody)helper（D9）/api/external/ifc-readyroute handler：呼叫 normalize → 必要時注入 derivedX-Correlation-Id/X-Idempotency-Key（explicit headers 優先 — D11）→ 走既有 auth / store / dispatch 路徑status→event/ifc_path→source_ifc.ref/version→external_model_version_id/task_id→external_conversion_task_id，task_id同時為 idempotency fallback（worker:project_id::version::task_id）source_ifc.etag缺時用worker:unknown:<task_id>fallback marker（不宣告為真實 checksum）tenant_id缺時用tenant_demo_001development fallbackAuto-session（conversion-ready → review session handoff）
bim-review-coordinator/src/app.ts:732-825：新增autoCreateOrActivateSession(job, artifacts, conversionJobId)helper（D10），重用既有SessionStore.create+allocateKitInstanceBindings+chooseReadyUsdc邏輯ingestConversionReportterminalready分支：於callbackOutbox.enqueue+recordConversionOutcome之後並行呼叫 helper；任一失敗不阻塞他者IfcReadyIntakeJob.review_session_idoptional 欄位 +ExternalIfcReadyStore.recordReviewSession()反向綁定（D11 idempotency）failed不建 session、GPU/Kit 無容量回queued_for_instance不丟 review intent/api/internal/conversion-result+/api/internal/conversions/:id/ingestresponse 加session/session_replay/session_reason欄位（additive）11 spec scenarios coverage
external-ifc-ready.test.ts新 describe block 7 caseshost-native-conversion-ingest.test.ts新 describe block 6 cases詳見
docs/verification/2026-05-21-backfill-coordinator-webhook-and-auto-session.md§2 對應表。Verification
npm run verify(tsc + vitest 全套)external-ifc-ready+host-native-conversion-ingest)openspec validate ... --strictvalidgit diff --checkgit diff --stat)single_kit_render/ WebRTC 49100 / browser visual)Non-goals
2026-05-21-coordinator-ifc-ready-worker-webhook/tasks.md的 26 個[ ] — deferred升級為[x]— post-merge gate：依賴 PR docs(openspec): retro-audit 8 個 archived changes 未完成 tasks（newer-wins） #83 先 merge（retro-audit 的 deferred 註記源自 PR docs(openspec): retro-audit 8 個 archived changes 未完成 tasks（newer-wins） #83，不在本 PR base）.envsecretsPredecessor
2026-05-21-coordinator-ifc-ready-worker-webhook(archived 2026-05-21, documentation lag)claude/retro-audit-archived-tasks)claude/propose-backfill-coordinator-webhook，本 PR base)Test plan
normalizeIntakePayloadhelper 與/api/external/ifc-readyroute handler 整合autoCreateOrActivateSessionhelper 與ingestConversionReportready 分支接線cd bim-review-coordinator && npm run verify確認 11 files / 165 tests passedpython -m pytest tests -p no:cacheprovider確認 7 passednpx openspec validate backfill-coordinator-webhook-and-auto-session --strict確認valid🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation