fix: 中文 model_version_id 轉檔派工 400 根治（全欄位 sanitize + 回拋對帳 + #/conv dispatch_error 可見） - #206
Merged
Merged
Conversation
…error 可見）(#205) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…us + import 清單補 IfcReadyListItem（四軸 review 修正落地） Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… artifact_id） 純函式 + 五個回歸鎖測試（vitest），尚未接線 dispatch 路徑（Task 1）。 規則逐字鎖 conversion_authority.py SAFE_ID_RE = ^[A-Za-z0-9_.-]+$。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rsion_id 不再 400 (#205) toInternalIfcReadyEvent 內 ifc_artifact.artifact_id 改用 sanitizeArtifactIdPart 組裝，使含中文的 external_model_version_id 通過 conversion 端 SAFE_ID_RE。 補單元回歸鎖（純英文 id 輸出不變）與端到端 dispatch 整合測試（stub conversion 以同款 SAFE_ID_RE 驗收，證明中文 id dispatch 不再被 400 擋下）。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nator summarize） Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
job 有 dispatch_error 時於該列「dispatch」欄附註截斷明細（完整字串走 title）， 沿用既有 ec-warn-note 樣式；無錯誤不渲染（顯示 —）。新增 mount 測試覆蓋 有錯/無錯兩種 job 的渲染行為，無 mock 假資料。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
task#2 把 dispatch_error 加成 IfcReadyListItem 必填欄位，但同目錄 sibling fixture（#182 起既有）未同步補欄位，tsc --noEmit 報 TS2741。vite/vitest 不跑 tsc 故 runtime 全綠，屬 merge 前型別清潔度問題。修法：fixture 補 dispatch_error: null。tsc 該錯已清，IntakeSelectPage 6/6 + console 37/37 綠。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… E2E + evidence (#205) #/conv → Refresh queue → 真 coordinator GET /api/external/ifc-ready → 真實 ifcready_* ID 的 user-facing vertical slice。 - 中文 external_model_version_id（271_pieple_管線）POST ifc-ready → sanitize 後 artifact_id 為 safe，conversion 端真 SAFE_ID_RE 不再 400 → dispatched（非 dispatch_failed）。 - 另造必失敗 job（forcefail 哨兵 → stub 回 400）→ dispatch_failed， #/conv 顯 conv-dispatch-error-<jobId> 節點，title 含完整錯誤字串（明細可見）。 採 (B) STUB CONVERSION API（誠實鐵律）：spec 自起本 branch coordinator（tsx） + 同 SAFE_ID_RE 規則的 Node http stub conversion server；真規則已在單元/整合層 （external-ifc-ready.test.ts）鎖死。evidence README 標 STUB CONVERSION API。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ion 不再 400 (#205) mv1/mv2 對抗複驗根治：conversion_authority.py:261-264 對 model_version_id / correlation_id / tenant_id / project_id 全跑 _safe_id（SAFE_ID_RE）。原修復只 sanitize ifc_artifact.artifact_id，中文 model_version_id 與 worker 派生含冒號的 correlation_id 仍會被真 API 擋成 400；整合測試 stub 只驗 artifact_id 故假綠。 - toInternalIfcReadyEvent：model_version_id / correlation_id 走 sanitizeArtifactIdPart， tenant_id / project_id 走 sanitizeSafeIdField（非字串透傳由 authority 套 server-side 預設）； external_model_version_id 保留原始供 callback/binding 對帳，外部契約不變。 - 整合 stub 升級為對齊真 API 驗證面：對 artifact_id + 上述四欄全跑 SAFE_ID_RE，任一非 safe → 400。 - 補測試：中文 model_version_id 內部事件兩欄（sanitize 後 / 原始）+ 含冒號 correlation_id sanitize 單元測試；worker 派生 correlation_id（worker:project::version::task）整合測試走嚴格 stub dispatched。 - 審計結論：tenant_id/project_id 在 coordinator 端無 SAFE_ID_RE 輸入驗證（requiredIdentity 只擋空值）， 中文 project/tenant 命名會踩同一條 400，故一併 sanitize。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) - mv2b(critical): toInternalIfcReadyEvent 的 event_id fallback 不再用原始 correlationId， 改 evt_${sanitizeArtifactIdPart(correlationId)}；外部帶 event_id 也過 sanitize。 conversion_authority.py:238 對 event_id 跑 _safe_id，worker 派生冒號 correlation 未 sanitize 會先炸 400。加 worker-compat 無 event_id 的整合回歸測試。 - cr1(high regression): correlationIndex 同時登記 sanitize 後鍵（sanitized !== 原始時 雙鍵指向同一 job），讓 conversion result 以 sanitize 後 correlation_id 回拋仍命中 原 job（非 404）。加端到端對帳測試（worker 冒號 correlation → result callback 命中）。 - st1(honesty): startSafeIdValidatingStub 補驗 event_id / idempotency_key（缺省 fallback 到 event_id）/ export_job_id / source_rvt_artifact_id（optional None/空字串放行）， 對齊真 conversion_authority create_conversion_job 全 _safe_id 驗證面。 - f1b(揭露): conversion-artifact-id-sanitize.spec.ts 補 conditional-skip 限制明文 （skip≠fail、靜默全 skip=假信心、本機/指揮官 gate、無 CI e2e job、升級條件）。 - ch1(措辭): console.test.tsx describe 標題改為如實（欄位形狀對齊真後端 schema， 渲染層驗證；真後端值由 E2E 驗），不再宣稱「真實後端欄位無 mock 假資料」。 - ce1(evidence): commit run2 最終碼 summary.json + conv-list.png。 - rl1(回歸鎖): streaming-conversion-client.test.ts 補全 safe 輸入逐欄 identity 斷言 （model_version_id/correlation_id/tenant_id/project_id/event_id === 原始值）。 驗證：bim-review-coordinator npm run build 通過、vitest 全量 314 passed； web-viewer-sample console.test.tsx 37 passed。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 30 minutes and 30 seconds. Learn how PR review limits work. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR fixes issue ChangesArtifact ID sanitization and dispatch-error handling
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #205 where Chinese characters in external_model_version_id (e.g., 271_pieple_管線) caused the coordinator's conversion dispatch to fail with a 400 error because the conversion authority's SAFE_ID_RE regex rejects non-ASCII characters. The fix introduces a deterministic sanitize function on the coordinator side, applies it to all fields validated by the conversion API's _safe_id checks, fixes correlation index reconciliation for sanitized keys, and makes dispatch_error details visible in the #/conv UI.
Changes:
- Added
sanitizeArtifactIdPartpure function that preserves safe-character-only IDs unchanged (backward compatible) and appends a SHA256 hash suffix for IDs containing non-safe characters; applied to all fields sent through_safe_idvalidation (artifact_id,model_version_id,correlation_id,event_id,tenant_id,project_id). - Added dual-key registration in
ExternalIfcReadyStore.correlationIndex(original + sanitized key) so conversion results can reconcile back to the originating job even when the correlation ID was sanitized. - Frontend
#/convpage now showsdispatch_errordetails in a new "dispatch" column with truncation at 80 characters and full text in thetitletooltip.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
bim-review-coordinator/src/services/streamingConversionClient.ts |
Core sanitize function + apply to all _safe_id fields in toInternalIfcReadyEvent |
bim-review-coordinator/src/services/externalIfcReadyStore.ts |
Dual-key correlation index registration for sanitized correlation IDs |
bim-review-coordinator/tests/streaming-conversion-client.test.ts |
Unit tests for sanitizeArtifactIdPart + toInternalIfcReadyEvent field-level assertions |
bim-review-coordinator/tests/external-ifc-ready.test.ts |
Integration tests with SAFE_ID_RE-validating stub for CJK and worker-colon scenarios |
bim-review-coordinator/tests/host-native-conversion-ingest.test.ts |
Correlation reconciliation test (sanitized correlation roundtrip) |
web-viewer-sample/src/console/coordinatorClient.ts |
dispatch_error field added to IfcReadyListItem type |
web-viewer-sample/src/console/pages.tsx |
#/conv table renders dispatch_error column with truncation + title tooltip |
web-viewer-sample/src/console/console.test.tsx |
Vitest for dispatch_error rendering (present + absent cases) |
web-viewer-sample/src/console/IntakeSelectPage.test.tsx |
Test fixture updated with dispatch_error: null |
web-viewer-sample/e2e/conversion-artifact-id-sanitize.spec.ts |
Playwright E2E: self-contained coordinator + stub conversion, CJK dispatch success + forcefail error visibility |
openspec/changes/conversion-artifact-id-sanitize/* |
OpenSpec change artifacts (proposal, tasks) |
docs/superpowers/specs/2026-06-11-conversion-artifact-id-sanitize-design.md |
Spec design document |
docs/superpowers/plans/2026-06-11-conversion-artifact-id-sanitize.md |
Implementation plan |
docs/evidence/conversion-artifact-id-sanitize/* |
Evidence README, summary JSON, screenshot |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| res.end(JSON.stringify({ detail: `Invalid ifc_artifact_id: ${artifactId}` })); | ||
| return; | ||
| } | ||
| res.writeHead(200, { "Content-Type": "application/json" }); |
| // STUB CONVERSION API：與 conversion 端 SAFE_ID_RE 同款規則驗 ifc_artifact.artifact_id。 | ||
| // 哨兵 forcefail → 400（製造 dispatch_failed）；非 safe → 400（防回歸：sanitize 失效時會紅）； | ||
| // 其餘 → 200 queued。 | ||
| async function startConversionStub(): Promise<void> { |
| return; | ||
| } | ||
| } | ||
| res.writeHead(200, { "Content-Type": "application/json" }); |
![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: 7d9dfcf55c
ℹ️ 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".
| this.correlationIndex.set(correlationId, jobId); | ||
| const sanitized = sanitizeArtifactIdPart(correlationId); | ||
| if (sanitized !== correlationId) { | ||
| this.correlationIndex.set(sanitized, jobId); |
There was a problem hiding this comment.
When a new job has an unsafe correlation ID, this writes the sanitized value into the same correlationIndex used by findExisting() for incoming intake deduplication. If another request uses that sanitized string as its real X-Correlation-Id with a different idempotency key, it will be treated as a replay of the first job; in the reverse order, this assignment overwrites the existing safe job's key so later conversion-result ingestion for that safe correlation updates the wrong job. The sanitized lookup should be isolated to conversion result reconciliation, or at least collision-checked, rather than sharing the raw correlation index used for intake identity.
Useful? React with 👍 / 👎.
|
|
||
| ### New Capabilities | ||
|
|
||
| - `conversion-artifact-id-sanitize`: 外部（含中文/特殊字元）model_version_id 經確定性 sanitize 後可通過 conversion authority 驗證完成派工；派工失敗原因於 `#/conv` 可見。 |
There was a problem hiding this comment.
This declares a new OpenSpec capability, but the change directory only adds proposal.md and tasks.md; there is no openspec/changes/conversion-artifact-id-sanitize/specs/conversion-artifact-id-sanitize/spec.md requirement/scenario delta. In this state the completed change cannot be archived into openspec/specs/, so the externally observable behavior and acceptance criteria for the sanitize contract will be lost after the PR even though the proposal says this is a new capability.
Useful? React with 👍 / 👎.
Contributor
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web-viewer-sample/e2e/conversion-artifact-id-sanitize.spec.ts (1)
79-111: 💤 Low valueConsider validating additional
_safe_idfields in the conversion stub to mirror real API behavior.The stub currently validates only
ifc_artifact.artifact_id, but per context snippet 1 (streamingConversionClient.ts:115-138), the real conversion authority also validatesevent_id,correlation_id,tenant_id,project_id, andmodel_version_idwith_safe_id. While the current stub suffices for the primary test goal (Chineseexternal_model_version_id→ sanitizedartifact_id), validating additional fields would:
- Catch regressions if coordinator fails to sanitize other fields
- Better mirror the E2E's stated goal ("stub 與 conversion 端 SAFE_ID_RE 同規則")
Proposed extension for multi-field validation
let artifactId = ""; + let eventId = ""; + let correlationId = ""; try { - artifactId = (JSON.parse(body || "{}")?.ifc_artifact?.artifact_id as string) ?? ""; + const parsed = JSON.parse(body || "{}"); + artifactId = (parsed?.ifc_artifact?.artifact_id as string) ?? ""; + eventId = (parsed?.event_id as string) ?? ""; + correlationId = (parsed?.correlation_id as string) ?? ""; } catch { artifactId = ""; } if (artifactId.includes(FORCE_FAIL_MARKER)) { res.writeHead(400, { "Content-Type": "application/json" }); res.end(JSON.stringify({ detail: `STUB forced failure for artifact_id: ${artifactId}` })); return; } - if (!SAFE_ID_RE.test(artifactId)) { + const invalidFields = []; + if (!SAFE_ID_RE.test(artifactId)) invalidFields.push("artifact_id"); + if (eventId && !SAFE_ID_RE.test(eventId)) invalidFields.push("event_id"); + if (correlationId && !SAFE_ID_RE.test(correlationId)) invalidFields.push("correlation_id"); + if (invalidFields.length > 0) { res.writeHead(400, { "Content-Type": "application/json" }); - res.end(JSON.stringify({ detail: `Invalid ifc_artifact_id: ${artifactId}` })); + res.end(JSON.stringify({ detail: `Invalid fields: ${invalidFields.join(", ")}` })); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-viewer-sample/e2e/conversion-artifact-id-sanitize.spec.ts` around lines 79 - 111, The conversion stub in startConversionStub currently only applies SAFE_ID_RE to ifc_artifact.artifact_id; extend its validation to also parse and validate event_id, correlation_id, tenant_id, project_id, and model_version_id from the incoming JSON using the same SAFE_ID_RE and existing FORCE_FAIL_MARKER check (mirror the real behavior in streamingConversionClient), returning the same 400 error messages when any of these fields are invalid or contain the FORCE_FAIL_MARKER; keep the existing response flow and error formats, reusing the artifact_id error wording pattern for each failing field so tests detect sanitization regressions across all fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bim-review-coordinator/src/services/externalIfcReadyStore.ts`:
- Around line 74-75: The correlationIndex currently collapses raw and sanitized
correlation keys causing aliasing; split into two maps (e.g.,
rawCorrelationIndex and sanitizedCorrelationIndex) and update
registerCorrelationKeys to register the original raw key in rawCorrelationIndex
and the sanitized key in sanitizedCorrelationIndex, then modify findExisting()
and getByCorrelation() to consult the appropriate index depending on whether
they expect raw vs sanitized keys (and update any callback reconciliation code
to use sanitizedCorrelationIndex). Ensure all call sites that read/write
correlationIndex (including registerCorrelationKeys, findExisting,
getByCorrelation, and callback reconciliation paths) are updated to the new
indices, add tests covering both raw->sanitized collisions, and note blast
radius: HIGH.
In `@docs/evidence/conversion-artifact-id-sanitize/README.md`:
- Around line 1-2: The README "Evidence — conversion-artifact-id-sanitize（中文
model_version_id 派工修復 + dispatch_error 可見）" is missing a document nature marker;
update the top of that README to include a single-line nature tag (e.g.,
"Document nature: evidence / runbook") so it conforms to docs/**/*.md
guidelines; ensure the marker appears near the main heading and uses the phrase
"Document nature:" followed by the chosen category (evidence or runbook) to make
intent explicit.
In `@docs/superpowers/plans/2026-06-11-conversion-artifact-id-sanitize.md`:
- Around line 1-8: Add the required document nature marker to this markdown (the
"Conversion artifact_id sanitize（中文 model_version_id 派工修復）Implementation Plan"
doc) by inserting a single-line nature tag immediately under the title (e.g.,
"Document nature: spec design" or the appropriate category from the docs
guidelines) so the file explicitly declares its nature per docs/**/*.md rules;
ensure the marker is plain text on its own line and not wrapped in code blocks
or HTML so automated checks will detect it.
---
Nitpick comments:
In `@web-viewer-sample/e2e/conversion-artifact-id-sanitize.spec.ts`:
- Around line 79-111: The conversion stub in startConversionStub currently only
applies SAFE_ID_RE to ifc_artifact.artifact_id; extend its validation to also
parse and validate event_id, correlation_id, tenant_id, project_id, and
model_version_id from the incoming JSON using the same SAFE_ID_RE and existing
FORCE_FAIL_MARKER check (mirror the real behavior in streamingConversionClient),
returning the same 400 error messages when any of these fields are invalid or
contain the FORCE_FAIL_MARKER; keep the existing response flow and error
formats, reusing the artifact_id error wording pattern for each failing field so
tests detect sanitization regressions across all fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8ab58ab-e9ae-40e7-b280-6eddc0f053b6
⛔ Files ignored due to path filters (1)
docs/evidence/conversion-artifact-id-sanitize/conv-list.pngis excluded by!**/*.png
📒 Files selected for processing (16)
bim-review-coordinator/src/services/externalIfcReadyStore.tsbim-review-coordinator/src/services/streamingConversionClient.tsbim-review-coordinator/tests/external-ifc-ready.test.tsbim-review-coordinator/tests/host-native-conversion-ingest.test.tsbim-review-coordinator/tests/streaming-conversion-client.test.tsdocs/evidence/conversion-artifact-id-sanitize/README.mddocs/evidence/conversion-artifact-id-sanitize/conversion-artifact-id-sanitize-summary.jsondocs/superpowers/plans/2026-06-11-conversion-artifact-id-sanitize.mddocs/superpowers/specs/2026-06-11-conversion-artifact-id-sanitize-design.mdopenspec/changes/conversion-artifact-id-sanitize/proposal.mdopenspec/changes/conversion-artifact-id-sanitize/tasks.mdweb-viewer-sample/e2e/conversion-artifact-id-sanitize.spec.tsweb-viewer-sample/src/console/IntakeSelectPage.test.tsxweb-viewer-sample/src/console/console.test.tsxweb-viewer-sample/src/console/coordinatorClient.tsweb-viewer-sample/src/console/pages.tsx
…ta + 文件標記/stub 202） - [P2/Major] externalIfcReadyStore：sanitize 後 correlation 鍵移入獨立 sanitizedCorrelationIndex，只供 conversion 結果回拋（getByCorrelation fallback） 查詢；intake 去重（findExisting）只看原始鍵，杜絕「真實 correlation 恰等於 他 job sanitize 值」被誤判 idempotent replay 的 aliasing；新增 intake aliasing 回歸測試（host-native-conversion-ingest.test.ts） - [Blocker] 補 openspec/changes/conversion-artifact-id-sanitize/specs/.../spec.md ADDED requirements/scenarios（sanitize 規則、回拋對帳 + aliasing 防護、 #/conv dispatch_error 可見），openspec validate 轉綠 - [Major] evidence README 與 implementation plan 補「文件性質」標記 - [nit] conversion dispatch stub 受理回應 200 → 202（對齊真 API）；設計文件 操作者一詞混入西里爾字母修正為 operator Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fbd531499
ℹ️ 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".
| // 先查原始鍵；查不到再查 sanitize 鍵（conversion 回拋帶的是 sanitize 後值）。 | ||
| // 原始鍵優先：若另一 job 的真實 correlation 恰等於某 sanitize 值，回拋對帳 | ||
| // 仍以「持有該原始鍵」的 job 為準，不被 sanitize 鍵搶走。 | ||
| const id = this.correlationIndex.get(correlationId) ?? this.sanitizedCorrelationIndex.get(correlationId); |
There was a problem hiding this comment.
When an unsafe correlation ID sanitizes to a string that another job already uses as its real X-Correlation-Id, conversion results for the unsafe job arrive with that sanitized value but this lookup now prefers correlationIndex first, so the result is applied to the safe job instead. The fresh evidence is this new raw-first fallback: it avoids intake replay pollution, but still leaves result reconciliation ambiguous for the exact aliasing case; use conversion_job_id or collision rejection rather than resolving solely by the sanitized correlation string.
Useful? React with 👍 / 👎.
… r2） job A（unsafe correlation，sanitize 後為 S）與 job B（真實 correlation 恰為 S） 並存時，A 的 conversion 結果以 S 回拋，單靠 correlation 字串無法裁決。 getByCorrelation 改收 optional conversionJobId：原始鍵與 sanitize 鍵的候選 job 中優先回傳 conversion_job_id 吻合者，無法消歧時維持原始鍵優先（向後 相容）；ingestConversionReport 帶入 report.conversion_job_id。新增 store 層 collision 消歧回歸測試。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
PR Review Agent Summary
Blockers
Warnings
Validation Commands
Checks
Human Review Notes
|
monkey1sai
added a commit
that referenced
this pull request
Jun 18, 2026
#229) 把 13 個產品碼已 merge 進 main 的 active change 歸檔為不可變快照， 並將其 spec delta 併入 canonical specs。對齊 #197 收斂規約。 歸檔（archive/<merge-date>-<id>，git 偵測為 R100 純改名、零內容漂移）： a1-m1-closeout(#213) a2-version-diff-selector(#207) conv-coverage-report(#218/#220) conv-prioritize-retry(#221) conv-watch-toggle(#225) conversion-artifact-id-sanitize(#206) governance-service-deploy(#215) minio-fileserver-source(#204) minio-watch-auto-intake(#210) raise-claude-md-line-budget(#199) sessions-terminate(#226) stop-all-single-pid-cleanup(#217) test-deploy-rebuild-workflow(#198) canonical 併入： - 9 個新 capability（純 ADDED → 新建 spec）：a1-m1-closeout、a2-version-diff-selector、 conv-coverage-report、conv-prioritize-retry、conversion-control、conversion-artifact-id-sanitize、 minio-fileserver-source、minio-watch-auto-intake、test-deploy-rebuild-workflow。 - review-session-request-lifecycle：append sessions-terminate 的 ADDED requirement 「Operator 結束 session controlled action」（5 scenario），既有 7 requirement 不動。 - one-click-deploy-hybrid：併入 governance-service-deploy 與 stop-all-single-pid-cleanup 兩 delta，採「合併不取代」保全既有更豐富內容。依 deploy.ps1 現況權威 （4a=governance/4b=conversion/4c=Kit/4d=docker）調和 Phase 4 編號，並修正 canonical 其他兩處陳舊的舊 3 段編號（Mode C 入口 scenario、退出碼 stage 清單補 4d）。 - agent-doc-context-budget：raise-claude-md-line-budget 的 130 行預算已於 #199 併入， 本次為 archive-only。 驗證： - 結構檢查無殘留 ## ADDED/MODIFIED header、每 requirement 皆有 scenario、 43 archive 檔全 R100、git diff --cached --check 無 whitespace。 - 雙 agent 對抗驗證：完整性 PASS（無規範遺失）；一致性初判 FAIL 抓到 2 處 Phase 4 編號矛盾，已修正後複驗。 - 本機 openspec CLI 不可用（結構驗證代替）；openspec validate --strict 由 CI pr-review-agent 執行。 Claude-Session: https://claude.ai/code/session_01JEyNWhEmb3x8oinY3B2v9V Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Why（Fixes #205 的 sanitize 部分）
真實 MinIO intake 帶中文
external_model_version_id（271_pieple_管線）的 job 下載成功但派工 400：coordinator 把外部 id 原樣嵌進 conversion 內部識別欄位，被 conversion authoritySAFE_ID_RE = ^[A-Za-z0-9_.-]+$擋下。對抗複驗以真 conversion_authority.py 模組實打再挖出兩顆連環雷（model_version_id、worker 派生含冒號的event_idfallback），並抓到第一版修復自己引入的 correlation 回拋對帳斷裂 — 全部在 merge 前修復並以真模組 ACCEPT/REJECT 鑑別力 probe 驗證。Spec：
docs/superpowers/specs/2026-06-11-conversion-artifact-id-sanitize-design.md（隨 PR 入庫）Plan：
docs/superpowers/plans/2026-06-11-conversion-artifact-id-sanitize.mdOpenSpec change：
openspec/changes/conversion-artifact-id-sanitize/What Changes
sanitizeArtifactIdPart純函式：safe 字元 identity 回傳（純英文/uuid id 輸出零變化，逐欄回歸鎖測試）；含非 safe →${safe}_${sha256[:8]}（確定性+防碰撞）；全非 safe →mv_<hash8>。_safe_id欄位：ifc_artifact.artifact_id、model_version_id、correlation_id、event_id（外部給定與 fallback 兩路）。external_model_version_id等對外對帳欄位保留原始值。correlationIndex雙鍵登記（原始+sanitized）：conversion result 以 sanitize 值回拋仍命中原 job；callback outbox 對外用原始 correlation_id（端到端對帳測試：worker 冒號 correlation dispatch → sanitized 回拋 → 非 404 正常推進）。_safe_optional_id語意）— 對抗複驗以 counterfactual 突變證明 stub 真會咬 regression。#/conv：dispatch_error明細可見（>80 字截斷 + title 完整字串；無錯不渲染錯誤節點）。Frontend Verification Table（product-operability §4）
/ui/#/convconv-dispatch-error-<jobId>明細external_model_version_id="271_pieple_管線"（POST /api/external/ifc-ready，X-Webhook-Secret header）+ 一筆 forcefail jobPOST /api/external/ifc-ready（202）→ dispatch →GET /api/external/ifc-ready（list 含 dispatch_error）ifcready_1781153867837_5f76456c→ dispatched（dispatch_error=null）；必失敗 job：ifcready_1781153867868_53f20883→ dispatch_failed 且明細可見#/conv列表：中文 job conversion=queued/dispatched；forcefail job 顯示 400 明細（title 含完整字串）npx playwright test e2e/conversion-artifact-id-sanitize.spec.ts（引擎：Playwright；1 passed，自起隔離 coordinator + STUB CONVERSION API）docs/evidence/conversion-artifact-id-sanitize/{conv-list.png, summary.json}；local trace：artifacts/e2e/conversion-artifact-id-sanitize-trace/#/conv看 job 進 dispatched；停掉 conversion API 再 POST 一筆 → 看 dispatch_error 明細Deploy Path Verification Table（product-operability §7）
.\scripts\dev\rebuild-test-deploy.ps1 -Build）/ui/#/conv（:8004 待 merge 後重建驗證）docs/evidence/conversion-artifact-id-sanitize/（tracked）+artifacts/e2e/conversion-artifact-id-sanitize-*（local）Impact / Review 揭露
IfcReadyListItem6 importers — 僅新增欄位，零修改既有欄位）；toInternalIfcReadyEvent模組內部 mapper（0 callers）。detect_changes 於 linked worktree 走已知 fallback（git diff --cached自查），task3 與兩輪 fix cycle 標 fallback，其餘 pass。event_id第三顆雷（host py312 直載真 conversion_authority.py REJECT 實證）+ 第一版修復引入的 correlation 回拋 404 regression（critic high）；fix67518f5後 r2 全 5 項 truly_closed（counterfactual 突變驗證 stub 咬合力、ACCEPT/REJECT 鑑別力 probe、證據 blob hash 三方一致）、critic overall_safe=true。🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests