feat(space): document-driven collaboration space UI with acceptance gate

[PeterGuy326]

Status: draft — do not merge as a completed acceptance workflow

Draft UI follow-up to the now-merged #559 (acceptance.v1). The base is main. This PR owns space-doc.v1 and its UI; the acceptance contract and service-draft/security prerequisites have already landed.

What works

A document/structure view derives acceptance criteria and task columns from two fixed Markdown headings. The gate requires every criterion to be ticked and a rejection to contain a reason. Records now use #559's required criteriaCount, pass the real validator, and use a stable identity independent of the display name.

Review corrections:

• Missing structured sections are inserted instead of dropping newly supplied data.
• Rewrites preserve separator blank lines and final newlines, including CRLF.
• Simplify the duplicated criterion guard; translate the English explanatory copy.
• Keep the corrected acceptance contract and current main; resolve locale conflicts without losing either feature.
• Do not call onAccept or publish an accepted verdict after a document-write failure.
• Reset space component state when the backing position changes.
• Add an Unreleased CHANGELOG entry that explicitly calls this a preview.

Remaining product blockers

1. The default spine path is a root filename, while the existing position-document write endpoint only permits knowledge/*. The current shell therefore cannot persist that default document. Resolve the space-to-directory/spine mapping, use one stable filename independent of UI language, and verify the real IPC/server path.
2. The shell does not wire a durable acceptance store/callback. Rejections remain in memory, and ticking criteria is not persistence of an AcceptanceRecord. Add durable, version-bound verdict writes and reload them before presenting this as a completed acceptance workflow.
3. This UI does not enforce agent/human authorization on document task-state writes, or wire accepted task transitions into the goals/execution model. The contract validator checks shape, not authority. Do not claim an enforced server-side human gate until that path exists.
4. Concurrent reads/decisions and optimistic-version conflicts need end-to-end acceptance through the real bridge. Current injected-bridge tests are insufficient to release this workflow.

Validation ledger

Environment: macOS arm64, Node 24.14.1, npm 11.11.0. Install via npm ci and build the adjacent design-system at CI-pinned 4a82fce41f89f1a55b6b02cdb54f7a4e9ce9d84d. No external services or real credentials are needed for these tests.

ID	Acceptance criterion	Command from repository root	Expected	Observed
V1	Shared/server contracts compile	npm run build	Exit 0	PASS, exit 0
V2	Coverage contract and surgical document writes	node --test apps/server/dist/test/acceptance.test.js apps/server/dist/test/space-doc.test.js	No failures	PASS, 45/45 (24 acceptance, 21 document)
V3	Gate behavior, failed writes and localization	npm run typecheck:renderer && node scripts/run-renderer-tests.mjs renderer/test/space-module.test.tsx renderer/test/space-acceptance-gate.test.ts renderer/test/i18n.test.tsx renderer/test/i18n-cjk-gate.test.ts	Exit 0, no failures	PASS, 25/25 across 4 files
V4	Package inventory closes over imports	npm run prepare:updater && node --test scripts/test/package-layout.test.mjs	No failures	PASS, 9/9
V5	Browser bundle supports subpath value imports	npm run build:renderer	Exit 0	PASS, exit 0
V6	Updated-head hosted platform/security checks	GitHub verify / Security Baseline	All green	FAIL: Ubuntu full renderer suite reproduces the pre-existing token-loss failure tracked in #512, addressed separately by #513. macOS completes functional tests, then fails at the shared dependency audit gate; 9/11 checks green. The product blockers remain
V7	Durable acceptance through real IPC/server, across reload and failure paths	Real workspace acceptance	Persisted, correctly scoped verdict	NOT VERIFIED / not implemented

Compatibility and rollback

No second document database is added. The two headings are format identifiers; prose outside them is not regenerated. acceptance.v1 is still unreleased and its new count is consumed here. The UI is a draft preview, not release acceptance. Revert the additive module, shell wiring, document subpath and packaging/i18n entries to roll back. Browser screenshots and Windows live GUI acceptance were not performed.

Refs #558; depends on #559.

Current shared audit blocker: GHSA-ch52-4w7c-c8xp. A separate 4.3.0 experiment clears npm's version-range check but still reproduces the shared-cache reuse issue, so it was not published as a security repair. No audit threshold or required check was weakened.

2026-10-05 follow-up

Synced the updated #559 contract branch and #513 connection-draft fix, so this draft's CI includes the PAT protection its previous Ubuntu run was missing. Both #559 and #513 must land before this UI. Included dependency removal from #562 without lowering the audit gate: get 5.1.0 removes the old HTTP cache chain, build Node minimum 22.12.0, legacy Got/GLOBAL_AGENT_* download options no longer apply.

Fresh local build and renderer typecheck passed. Acceptance/doc/downloader tests passed 47/47; five renderer specs (space module, acceptance gate, service connections, i18n and CJK gate) passed 32/32. Fresh current-head CI is running. This remains Draft even if CI turns green: real document path compatibility, durable AcceptanceRecord storage, caller authorization and real IPC end-to-end acceptance are still not implemented. No claim of product acceptance or merge.

Current-head CI result

All 10 executed checks passed at b1a8f8419943e0fe0e2df513a1fe1079b70a337e, including full Node 24 jobs, security, both staging/installer platforms and layout parity. The UI remains Draft for the integration blockers above; green tests do not establish durable end-to-end acceptance.

Post-contract integration (2026-10-05)

#562, #513 and #559 are merged into main. Retargeted this draft to main and synchronized the branch at 9f703721ad6dd2344523b077baf579ceb0f2ae77; resolved additive CHANGELOG and shared-export conflicts without changing the previously tested source tree. All 11 hosted checks passed on this head, including both full platform tests, security, staging, installers and layout parity. The earlier check results above refer to their stated historical heads. All remaining product blockers above still apply; this PR stays Draft and is not authorized for merge.

[PeterGuy326]

CI is green on 373110a2 — all seven verify jobs pass (Node 24 ubuntu + macos-14, unsigned installers, unpacked staging smoke on both platforms, layout parity).

Getting there took four fixes, all of which were real bugs rather than test noise. Worth a skim before you review the diff, because two of them are things the repo enforces but a local run does not surface:

1. noUncheckedIndexedAccess in the server tests (cc748ff). apps/server/tsconfig.json includes test/**/*.ts, so the new test files are type-checked too — 19 errors from list[0] being T | undefined. Added an at(list, i) helper instead of scattering !. The same line existed on feat(space): acceptance.v1 contract for human acceptance verdicts #559, fixed there as well.

2. The packaging allowlist (4ea2434). scripts/test/package-layout.test.mjs asserts SHARED_RUNTIME_FILES matches packages/shared/src exactly and in order. acceptance.js and space-doc.js were not listed — so beyond the red test, neither module would have shipped in the packaged app. Both inserted in alphabetical position.

3. The CJK gate (f849fb8). feat(i18n): zh/en locale switch with full UI string catalog and CJK lint gate #146 forbids CJK string literals under renderer/src; SPACE_SPINE_FILE = "目标.md" was the one offender. Moved to the catalog as space.spineFile («目标.md» / "goal.md"). Also added the new rail entry to the App.test.tsx ordering assertion.

4. Value imports from the @roleweave/shared barrel (373110a). position-id.js does createRequire(import.meta.url) to load position-id.cjs, which build:renderer cannot resolve in a browser bundle. It only becomes reachable when the renderer pulls a value out of the barrel, since the barrel is export * over every module. The control that pinned it: feat(space): acceptance.v1 contract for human acceptance verdicts #559 adds acceptance.js and touches no renderer file, and its verify passes.

   So parseSpaceDoc / renderSpaceDoc now come from @roleweave/shared/space-doc and ACCEPTANCE_SCHEMA_VERSION from @roleweave/shared/acceptance. Types still come from the barrel via import type, which is erased and cannot drag a module in.

   If you have a preferred convention here — subpath for values everywhere, or an exports map that hides the node-only modules from the browser entry — say so and I will follow it rather than deciding per-file.

Still interested in your read on the 验收台 boundary: an agent can move a task to review, but only a human acceptance writes done, and accepting writes it back into the same Markdown the agent reads. Technical only, as usual.

[PeterGuy326]

Review: feat(space): document-driven collaboration space UI with acceptance gate

Scope: 21 files, +2532/-3, head 373110a. This is a substantive review of the contract layer, the parser/renderer pair, and the packaging/i18n wiring. I verified each repo constraint against the tree rather than against the PR description.

---

Verdict: changes requested (architecture is sound; two real defects in the renderer)

The overall shape is good: fixed-H2 structure keeps the file human-readable, the gate funnels through a testable model, and the packaging/i18n wiring is correct. Two defects in renderSpaceDoc should be fixed before this lands.

---

Constraint verification

1. CJK gate (#146) — PASS, but the premise is inaccurate.
The gate lives in apps/desktop/renderer/test/i18n-cjk-gate.test.ts. Its SCAN_ROOTS are apps/desktop/renderer/src and packages/ui/src only, and locales is an EXCLUDED_DIR. packages/shared/src is not scanned by any gate in the tree. Therefore the two fixed H2 literals in space-doc.ts (SPACE_DOC_CRITERIA_HEADING = "验收标准", SPACE_DOC_TASKS_HEADING = "任务") will not fail CI, and no exemption mechanism is required. These are genuine format identifiers rather than user-facing copy, so leaving them as literals is defensible — but be aware the gate does not protect this file at all.

2. Shared barrel trap — PASS. SpaceModule.tsx and acceptance-gate-model.ts take value imports via the subpaths @roleweave/shared/space-doc and @roleweave/shared/acceptance, and use import type for everything from the barrel. space-doc.ts imports goalWorkItemStatuses from ./goals.js, and goals.ts is pure (import type only), so nothing pulls position-id.js/createRequire into the renderer bundle. package.json exports both subpaths. No build:renderer break.

3. runtime-layout.cjs whitelist — PASS. SHARED_RUNTIME_FILES dist/* entries match walkFiles(packages/shared/src) exactly (37 entries) and in the required sorted order: dist/acceptance.js correctly precedes agent-binding.js, and dist/space-doc.js sits between sessions.js and task-board.js. Note this only holds because acceptance.ts is present in the branch — see the divergence note below.

4. rail.space registration — PASS. Present in both zh.ts and en.ts with identical space.* key sets; i18n.test.tsx asserts zh/en key parity and non-empty values, which holds.

5. at(list, i) assertion style — PASS in apps/server/test/space-doc.test.ts (helper defined and used throughout). Minor: apps/server/test/acceptance.test.ts uses value.verdicts[1]?.evidencePath and bare result.value access instead of the same helper — not a failure, just inconsistent with the house style used in the sibling file.

6. Parser total / renderer surgical — the parser holds; the renderer does not. Two defects below.

---

Defects

B1 — renderSpaceDoc silently discards content for a section whose H2 is missing. (Must-fix)
packages/shared/src/space-doc.ts:318-325 only registers a replacement, and the emit loop (:329-348) only rewrites a heading that already exists in source; it never inserts a missing ## 验收标准 or ## 任务.

Repro:

source = "# 目标：X\n\n## 验收标准\n- [ ] a\n"
doc.tasks = [{ title: "new", ... }]
renderSpaceDoc(doc, source)  // => "# 目标：X\n\n## 验收标准\n- [ ] a"  — "new" is gone, no ## 任务 created

This is reachable from the shipped flow, not just synthetic input: in SpaceModule.tsx (handleDecide), accepting a space marks every criterion done and calls renderSpaceDoc(next, source). If the spine document has no ## 验收标准 heading (a realistic draft state), the render produces no structural change, writeFile persists an unchanged file, and the user is told the criteria were marked done while nothing changed on disk. That is silent data loss on the one mutation this PR exists to perform.

Suggest: either synthesise a missing section (append ## 验收标准 / ## 任务 with its canonical body at a deterministic position before EOF), or make renderSpaceDoc return the original string unchanged and have the caller surface an explicit "no section to update" state instead of pretending the write succeeded.

B2 — renderSpaceDoc is not byte-preserving around the rewritten sections. (Should-fix)
The contract comment at space-doc.ts:293-297 promises to leave every other byte untouched. Two violations:

The blank line separating a rewritten section from the next heading is eaten:

source   = "...| done | eng | 2026-01-01 |\n\n## 风险\n\n无。\n"
rendered = "...| done | eng | 2026-01-01 |\n## 风险\n\n无。\n"   // blank line before ## 风险 is gone

Same for a criteria section followed by prose. The loop skips the original body up to the next H2 and pushes the replacement without re-emitting the trailing separator line.

Trailing newline(s) at EOF are dropped when the last section is structured:

"# 目标：X\n\n## 验收标准\n- [ ] a\n"  ->  "...- [ ] a"        // no final newline
"...| a | todo |\n"                     ->  "...| a | todo |  |" // no final newline

That produces "no newline at end of file" on every write, which will churn diffs and trip git diff --check/formatters.

The existing round-trip tests (space-doc.test.ts) assert via .includes(...) and fixed-point equality, so both of these pass CI while the bytes still drift. Recommend adding assertions for the exact separator line and the terminal newline.

B3 — tautological guard. (Should-fix / clarity)
space-doc.ts:319: if (criterionLines.length > 0 || doc.criteria.length > 0). criterionLines is doc.criteria.map(...), so both operands are always equal; one is dead. If the intent was "also rewrite an existing-but-empty criteria section", the || does not express it (the section is rewritten whenever it is present, empty or not, regardless of this condition) — the condition is simply a complicated doc.criteria.length > 0.

B4 — English catalog embeds raw Chinese headings. (Minor)
packages/ui/src/locales/en.ts: space.structure.noCriteria = This document has no "## 验收标准" section yet. and space.structure.unparseable = A "## 任务" table was found.... locales/ is excluded from the CJK gate, so this passes, but shipping Chinese literals in the en catalog is inconsistent with the rest of the file and will read oddly in an English UI. Consider referencing the headings from the shared constants or phrasing without them.

---

Branch divergence (affects mergeability now)

• 373110a is based on faafeb4e, which is 1 commit behind current main (6f2cb283, the #557 goals/progress merge). gh reports the PR as CONFLICTING / DIRTY.
• The concrete conflict is real, not cosmetic: #557 modified apps/desktop/renderer/src/App.tsx and packages/ui/src/locales/{en,zh}.ts, and this PR also edits all three. Both branches append key blocks at the end of the locale files and both touch the App.tsx import block / activeModule union, so a rebase is required. The space/ code itself does not overlap with #557, so the resolution is mechanical.
• Ownership issue: PR #559 (feat(space): acceptance.v1 contract...) is still OPEN and shares this exact base faafeb4e. This PR carries a byte-for-byte copy of packages/shared/src/acceptance.ts (verified identical, 13277 bytes) plus its own edits to packages/shared/src/index.ts, packages/shared/package.json, runtime-layout.cjs, and apps/server/test/acceptance.test.ts — the same files #559 touches. Whichever of the two merges first, the other must be rebased; merging both as-is will conflict on the shared contract file. Recommend deciding which PR owns acceptance.ts (it reads like #559's deliverable) and rebasing the other onto it, so this PR stays UI-only.

---

No blocking issues with

Packaging inventory/order, i18n key parity, barrel-import discipline, the gate's decision invariants (canAccept/canReject/buildAcceptanceRecord cross-checks match the acceptance.v1 validator), and the renderer test coverage. Nice touch: agents can only reach review, and only acceptance moves a criterion to done (SpaceModule.handleDecide) — that separation is enforced and tested.

---

Requested changes

1. Fix B1 (missing-section handling) or make the failed write explicit — this is the one that silently loses data.
2. Fix B2 (preserve the separator blank line and the terminal newline); extend the round-trip tests to assert exact bytes.
3. Simplify the B3 guard.
4. Rebase on main; coordinate acceptance.ts ownership with #559 before merging.

B4 is optional polish.

[Bindy-lbb]

Review 基于当前 head 9f703721ad6dd2344523b077baf579ceb0f2ae77。已核实全部 11 个 hosted checks 通过，包括 7 个 verify job。建议保持 Draft，处理以下问题。

导入约定

renderer 的运行时值统一从明确的 @roleweave/shared/* 子路径导入；类型可以通过 import type 从 barrel 导入。现有 docs、goals、attachments 等模块已经采用这个方式，#560 的 /space-doc 和 /acceptance 改法与现有用法一致。子路径及其传递依赖也必须保持浏览器可用。这个 PR 无需扩展为整个 shared browser exports 的改造。

高优先级：验收写回会丢失原始文档数据

位置：packages/shared/src/space-doc.ts:303–318，调用方 apps/desktop/renderer/src/space/SpaceModule.tsx:120–128。

验收只需要把标准的 checkbox 标记为完成，但 renderSpaceDoc 同时按固定四列重建整个任务表。已用当前源码复现：任务表包含额外的“证据”列、负责人 Alice Smith、截止值 tomorrow 时，仅将验收标准改为 done 后写回，proof.md、负责人和截止原文全部丢失。虽然 parser 保留了 raw，renderer 并没有使用它。

同样，含 65 条验收标准的文档经过 parse/render 后，第 65 条会被静默删除；解析上限不应变成破坏原文的写入行为。

建议验收写回只修改对应 checkbox，保留任务表、未识别内容及超限部分的原文；超限或无法安全编辑时明确阻止写入，不能静默截断。请补充非标准任务表和超限文档的回归用例。

中优先级：主线文件路径不能跟随 UI 语言变化

位置：apps/desktop/renderer/src/space/SpaceModule.tsx:68–70,102 与两份 locale 的 space.spineFile。

把中文字面量移出 renderer 解决了 CJK gate，但中文 目标.md、英文 goal.md 是不同的文件路径。切换语言后，既有 activePath 还会与新的 spineFile 不匹配，使当前文档失去主线身份和验收入口。

请使用与语言无关的稳定路径/契约字段，只翻译显示名称。这与 PR 正文已记录的路径集成 blocker 一致；同时仍需解决根目录主线文件与仅允许 knowledge/* 写入端点的兼容性。

验收边界

“agent 可以推进到 review，human 才能推进到 done”的边界合理，但当前代码仅把验收 checkbox 写成 [x]，没有实现任务 review → done、持久化的 AcceptanceRecord 或服务端调用者身份校验。当前 shell 也没有传入持久化 onAccept。这些已在 PR 正文列为 blocker，因此这里只确认预览性质，不能据此认定端到端人工验收已完成。

验证范围

已审查 OCR delegate 列出的 15/15 个可审查文件，无跳过，并阅读相关测试和端点代码。隔离转译当前源码后运行现有文档测试：21/21 通过；上述两种数据丢失复现仍成立。本轮没有重新运行完整应用构建或真实 IPC/GUI 端到端验收；完整 CI 状态来自当前 head 的 GitHub checks。

[sun-970]

Review note — not approving a Draft; this is a substantive read of the diff against the declared blockers.

Verified positively:

• acceptance-gate-model.ts is the right shape: accept/reject rules are pure and unit-testable, and every record is round-tripped through the real validateAcceptanceRecord, so the gate cannot emit a shape-invalid record.
• The "never rewrite prose the parser did not understand" contract holds for ordinary sections (otherSections kept, H1/prose untouched, CRLF preserved).
• Task column aliasing (任务/标题/task/title…) with per-cell validation + issues[] instead of silent coercion is a good, total design.

Findings to close before this becomes ready:

1. Cap-then-regenerate is destructive on write. parseSpaceDoc stops at SPACE_DOC_MAX_CRITERIA (64) and SPACE_DOC_MAX_TASKS (128) via break, while renderSpaceDoc regenerates both sections from the capped model. Saving a document that holds >64 criteria or >128 tasks would silently delete the overflow lines from the file — which contradicts "the file is the source of truth". Either surface a parse issue and refuse to render, or preserve the unparsed tail verbatim rather than regenerating the whole body.

2. Task cell escaping. renderSpaceDoc joins cells with " | " and writes task.title raw. A title containing | (or a newline) breaks the table and the round-trip. SpaceDocTask.raw exists precisely for this, but the renderer never consults it.

3. SpaceDocCriterion.marker is not honoured on render. The field's comment says the raw checkbox token is retained "so an unrelated state is not silently lost", yet the renderer emits - [x] / - [ ] from done — so X is normalized away. Either honour marker when rendering, or drop the field; as written the comment overstates what survives.

4. Duplicate structured headings. Parse takes last-wins for a repeated ## 任务, but the renderer replaces every occurrence of the heading with the same regenerated body, so a duplicated section would duplicate the table. A single "already emitted per heading" guard, or a parse issue, closes it.

Endorsed as-is: the four product blockers listed in the PR body (spine path vs the knowledge/* write endpoint, no durable acceptance store, no server-side authority on task-state writes, no real-bridge concurrency coverage) are real and correctly scoped. Staying Draft until V6/V7 are green and the App.tsx/locales conflicts against main are resolved is the right call.