[PeterGuy326]

Tracking record

Closes #606. Maintenance inventory and historical evidence: #607.

Consumed revision: R1.

Summary

Running a rejected project task carries its saved human criterion verdicts, rejection reason and exact rejected delivery into the assigned employee's next explicitly started personal-session execution. The existing bounded, sanitized thread-context assembler supplies the feedback even when ordinary history is disabled. Every new result still needs human acceptance.

Repairs use existing acceptance/execution records. Group execution skips personal repair injection and never consumes the personal repair ceiling. A saved plan fingerprint distinguishes real plan edits from health refresh clocks; legacy records remain readable with conservative revision checks. No automatic scheduler, separate task store, business ontology or factory-specific behavior is added.

Current review head: cdf8998b79ca7436e412280110d20ac51fb79188, based on main 331401361a290cdcc0a3b622c8b6f3d6a2c7742e. Ordinary merges preserve approved main changes through #620 and the 0.6.0 release baseline (#624), without rewriting history. The latest merge resolves imports and runtime inventory while retaining both implementations; pending #606 entries remain under Unreleased. The feature diff against main remains eight files, 380 insertions and one deletion. Prior head-specific evidence remains in PR comments and #607; it is not evidence for this new SHA.

Acceptance criteria

Requirement and acceptance	Result	Evidence
REQ-001 / AC-001: persisted feedback and reviewed delivery reach the assigned personal task	PASS	Real HTTP/session/store tests capture unchanged input, sanitized feedback, delivery and context receipt, including history disabled.
REQ-002 / AC-002: ownership, current plan/evidence and finite attempts remain authoritative	PASS	Plan digest, criterion/source/session guards; durable three-attempt ceiling survives restart, counts failed personal attempts and excludes group spawns; unavailable history fails closed.
REQ-003 / AC-003: human acceptance remains authoritative	PASS	Operator-started dispatch only; another result requires a fresh verdict; group HTTP execution stays compatible.
REQ-004 / AC-004: regressions and documented boundaries	PASS	14 repair regressions plus existing acceptance, relay/group/context suites in the full gate; documentation and packaging inventory updated.

Validation ledger

Check	Expected	Actual	Evidence
V1: fresh npm ci	Reproduce approved main dependency baseline	PASS	Node 24.11.1 / WSL Ubuntu 22.04; audit 0. Approved main already contains Vite 6.4.4.
V2: complete final-source npm run check	Every documented local check passes	PASS: 3131 passed, 0 failed, 3 existing conditional skips	Scripts 233; UI 53; server 1011 + 3 skips; renderer 1264; desktop 570. Build/type checks, production renderer build, updater preparation and audit 0 pass. Logs: work/roleweave-maintenance-20261018/608-post620-install.log and 608-post620-final-check.log outside source.
V3: integration inspection and git diff --check	Preserve repair, briefing/retry and relay boundaries	PASS	Retry restores its saved project briefing identity before repair checks. Group skip remains before repair history access. Personal repairs append to existing supplemental context; briefing/collaboration and relay stores stay main-owned. Resolved import/inventory conflicts preserve both sides; no whitespace errors, new dependency or parallel store.
V4: hosted checks for cdf8998b	Required checks green at the exact head	PASS: 11/11	Both full Node 24 jobs, CodeQL, dependency review, Scorecard, both native staging/installer jobs and layout parity passed. Verify.
V5: independent approval for cdf8998b	Non-author approval before normal guarded squash merge	NOT VERIFIED	Bindy-lbb/sun-970 requests retained; author PeterGuy326 cannot supply this approval. No admin bypass.

Tests and coverage

Repair regressions exercise real group dispatch against a rejected task, personal-only attempt counting, durable health refresh versus a real plan edit, legacy acceptance validation, restart, foreign ownership, stale evidence, concurrency, failed attempts, unavailable history, feedback redaction and byte bounds. The full final-source gate includes the newly shipped relay handoff/group/context regressions after #620; no test or gate was disabled. Existing opt-in skips retain their environment conditions. No coverage percentage, live-model quality or real-customer acceptance is claimed.

Change classification

• User-visible behavior
• Internal refactor or maintenance only
• Documentation only
• Build or packaging inventory
• Breaking API change
• Security-sensitive context and dispatch guards

Risk and rollback

Moderate: task context and dispatch eligibility. New acceptance records contain optional planDigest; older records remain readable and use the conservative clock guard until a fresh verdict is recorded. No automatic migration runs. A server predating the additive reader cannot read new decisions with its strict validator: preserve acceptance records and retain the additive reader compatibility when reverting execution behavior. Do not discard audit history to downgrade. Original request, permissions, budgets and cancellation remain authoritative.

Breaking or security notes

Human feedback and delivery excerpts are untrusted task data, never tool authorization. Feedback exceeding 8 KiB stops dispatch; delivery excerpts are bounded by the existing assembler with truncation receipts. An unavailable/oversized workspace ledger stops repairs rather than resetting the attempt count. No credentials or customer data are introduced. No business-case code is moved into RoleWeave.

Author checklist

• Ready issue and bounded R1 scope confirmed.
• Existing branch updated by ordinary merge/fast-forward push; unrelated dirty worktrees preserved.
• Full final-source local gate, fresh install, diff check and audit passed.
• Documentation, changelog, packaging inventory and legacy-reader boundary recorded.
• Reviewed dispatch, ownership, context and integration behavior.
• Current-head hosted checks and independent approval complete.

Reviewer notes

Review cdf8998b79ca7436e412280110d20ac51fb79188: group skip and relay compatibility, personal-attempt filter, plan digest and atomic recording, conservative legacy behavior, and human acceptance after each repair. General execution/review belongs in RoleWeave; reusable business cases remain in workbench-examples; enterprise facts/ontology/evidence remain in Enterprise Context. AI-assisted implementation; no automated Git co-author. Normal squash merge with an exact-head guard and completed-branch deletion is authorized once all gates pass; no further user confirmation is needed.

Latest integration regression — 2026-10-21 (Asia/Shanghai)

Main #620 restores goal/task identity from a failed execution's saved briefing when an explicit retry omits those IDs. A mechanical merge would run repair validation too early and permit another unreviewed repair. The new HTTP regression failed before the ordering fix (200 instead of expected 409). Repair validation now follows server-owned retry-binding restoration, so the request is rejected before another model call. This reuses the existing binding/acceptance stores; no second retry/task model.

Focused repair, briefing, collaboration, relay and group suites pass 57/57 after the fix. The entire local gate was then restarted on final committed source and passes 3131 tests, 0 failed, 3 existing conditional skips, audit 0. Before-fix reproduction: 608-post620-regression-before.log; focused checks: 608-post620-focused.log; final gate: 608-post620-final-check.log in the existing external maintenance folder. The interrupted pre-final run is not acceptance evidence.

Current-head hosted CI is PASS (11/11); independent non-author approval is still pending. Earlier head CI/reviews must not be reused. The already-published 0.6.0 changelog is preserved, and this still-pending feature remains Unreleased; this maintenance change does not publish a desktop release.

[PeterGuy326]

Current-head CI and merge handoff

PR #608 is frozen at 1a4702f. All 11 executed checks passed, including both complete Node 24 jobs, CodeQL, dependency review, scorecard, both staging smokes, both native installers and layout parity.

• Verify: https://github.com/bytefolk/roleweave/actions/runs/39114619446
• Security: https://github.com/bytefolk/roleweave/actions/runs/39114619319
• Scorecard: https://github.com/bytefolk/roleweave/actions/runs/39114619323

No independent approval is recorded. Bindy-lbb and sun-970 have been requested through the standard review mechanism. GitHub reports REVIEW_REQUIRED; auto-merge is disabled. Author self-approval or administrator bypass would violate the inherited contribution/engineering policy, so no merge was attempted without the required review. After an eligible reviewer approves this exact head, re-read the head/base/checks and squash-merge using --match-head-commit. Do not publish a desktop release as part of this change.

Maintenance disposition: four original PRs closed (#563/#505 absorbed; #560/#524 retired implementations with requirements retained), two completed implementation issues closed (#347/#511), eight original PRs and 39 original issues retained with reasons in #607. Current totals are 9 open PRs and 41 open issues, including new #606/#607/#608. #606 and #607 remain open because the merge gate is outstanding.

Responsibilities remain: general execution/review in RoleWeave; reusable business implementations in workbench-examples; enterprise facts, ontology and evidence in Enterprise Context. The first real factory pilot remains receivables reconciliation and customer follow-up with human confirmation; no customer data, financial writes or automatic messages were introduced here.

[PeterGuy326]

Reviewed at exact head 1a4702fe (base 471beb3e). The design is sound: repair context is derived only from existing acceptance/execution records, the attempt ceiling is read from the durable ledger, history-read failures fail closed, and completion still requires a human verdict. I cannot approve this PR because I am the author of record, so this is a review comment only; approval must come from another CODEOWNER.

Blocking

1. apps/server/src/goals/rework.ts:24-28 — group turns that reference a rejected task now hard-fail with 409.

executeTurn calls prepareGoalRework(..., group ? undefined : session?.sessionId) (apps/server/src/routes/turns.ts:257), so for every group spawn sessionId is undefined. The third clause rejection.source.sessionId !== sessionId is then unconditionally true, and the function throws goal_conflict instead of returning [].

Group requests are allowed to carry goalId/branchId (apps/server/src/routes/groups.ts:105-190) and the spawn body persists them (apps/server/src/routes/turns.ts:432-433). So a group message bound to a task whose latest verdict is rejected now fails before any model call, where it previously ran normally. Repair injection is a personal-session feature; a group turn is not attempting a repair, so it should be skipped rather than rejected.

Suggested fix: return [] when sessionId === undefined (or when group !== undefined) before the ownership checks, and keep the 409 for the personal-session case where the session genuinely differs from rejection.source.sessionId.

2. Same block — no test covers the group path. All nine fixtures go through POST /sessions/:id/turns. Please add one case that posts a group turn (or calls prepareGoalRework with sessionId === undefined) against a rejected task and asserts the previous behaviour (dispatch, no repair context) or the explicitly intended 409.

Should fix

3. apps/server/src/goals/rework.ts:29-32 — goal.updatedAt is the record clock, not a plan revision.

At decision time recordAcceptance sets updatedAt = decidedAt (apps/server/src/goals/store.ts:607-615), so the equality holds initially. But updatedAt is also bumped by getDetail whenever computed health changes (apps/server/src/goals/store.ts:455-478), i.e. by a plain GET /goals/:id after a failed or indeterminate turn, and by any update(). Nothing on the plan changed in those cases, yet every later repair dispatch returns 409 with "repair criteria or goal state changed; review the plan before continuing", and the only way back is to record another verdict. Today this is latent because branches is never populated (so computeHealthFromTurns returns early), but the coupling becomes a real trap the moment branch health is persisted.

Suggestion: store a plan fingerprint on the acceptance record (criteria snapshot is already there — extend it with the work-item assignee/status rows and the branch set) and compare that, instead of the whole-record updatedAt.

4. apps/server/src/goals/rework.ts:40-42 — the ceiling and its message duplicate the constant. MAX_TASK_REPAIRS = 3 is exported, but the operator-facing text hardcodes "(3)". Derive the number from the constant so the two cannot drift.

Notes

• reportRecords throws once the workspace ledger exceeds MAX_HISTORY_BYTES, so a large workspace can never repair (500, no dispatch). Fail-closed is correct; worth stating as a known limitation in the docs paragraph rather than only in tests.
• taskTurns counts every turn bound to the task, including group spawns that per (1) can never be repairs. Once (1) is resolved those turns will still consume the ceiling; consider counting only turns whose conversationRef === sessionId.
• The 8 KiB guard covers feedback only. output is capped separately by the assembler (MAX_FIELD_BYTES), and the docs already state that excerpts may be truncated — no change requested, just confirming the boundary is intentional.

Verified

• Nine new tests exercise real HTTP/session/store boundaries: limit across store restarts, foreign session, changed criteria, unavailable ledger, redaction, concurrent dispatch, digest tamper, failed attempts, oversize feedback. The [200, 409] concurrency assertion matches the runningTurns reservation semantics I checked in executeTurn.
• runtime-layout.cjs adds dist/src/goals/rework.js to SERVER_RUNTIME_FILES, so the packaged server imports the new module rather than failing at runtime.
• CHANGELOG entry under [Unreleased] uses Refs #606; docs section scopes the feature as human-started, not an autonomous loop. No secrets, no customer data, no dependency changes in the diff.
• All 11 executed checks pass at 1a4702fe, including both Node 24 legs, CodeQL, dependency review, scorecard, both staging smokes, both unsigned installers and layout parity.

Approval still required from a non-author CODEOWNER (Bindy-lbb or waterbro-8) at head 1a4702fe; no administrator bypass was attempted.

[PeterGuy326]

Re-reviewed at c04c7ac (the two follow-up commits after 1a4702fe). All three findings from review 5563796272 are resolved, and I could not reproduce a new blocking path.

1. Group regression (was blocking) — fixed. apps/server/src/routes/turns.ts now passes group ? undefined : session?.sessionId, and prepareGoalRework returns [] before touching the ledger when sessionId is undefined, so group turns keep the ordinary execution path. Covered by "a group turn bound to a rejected task still dispatches without repair context" and "group spawns do not consume the personal repair ceiling".

2. Plan-unchanged predicate (was should-fix) — fixed properly. goalPlanDigest excludes branch createdAt/updatedAt and is persisted on the acceptance record (store.ts), so a health refresh no longer invalidates a repair while real edits to criteria, work items or the branch plan do. The legacy fallback (planDigest === undefined → goal.updatedAt === rejected.decidedAt) is conservative, documented in docs/thread-context-and-collaboration.md, and tested ("legacy rejection records remain readable and retain conservative revision checks").

3. Hard-coded limit message (was should-fix) — fixed. The 409 message is built from MAX_TASK_REPAIRS.

Two things I checked beyond my previous review and that hold up:

• The full-ledger turnStore.reportRecords read and the 8 KiB feedback cap only run after a saved rejected decision exists, so ordinary turns that merely carry goalId/branchId neither pay the ledger read nor inherit the fail-closed behaviour on an oversized ledger. The "unavailable execution history fails closed" and "oversized feedback" tests pin both.
• Concurrency: "concurrent repair requests dispatch once and require a fresh human verdict" asserts exactly one 200 and one 409 with a single model call, which is the failure mode that would otherwise double-spend a repair attempt.

No blocking findings from me at c04c7ac. Since the author is PeterGuy326, this review cannot carry the approval: per #607 AC-004 this needs one independent approval on this exact head from Bindy-lbb or sun-970, then squash with --match-head-commit. If a follow-up commit lands, this review is stale and the new head needs its own verification.

[PeterGuy326]

Current review head: fc12e8d. Integrated current main (#612/#616) with ordinary merges; full final-source local gate PASS 3054, 0 failed, 3 existing conditional skips, audit 0. Hosted CI is running; previous-head checks are historical only. @Bindy-lbb @sun-970 please review this exact head after its required checks pass. The maintainer authorized normal guarded squash merge after independent approval; no author self-approval or protection bypass.

[PeterGuy326]

Integrated approved main #614 (2dc7764) without conflicts or history rewrite. Current review head d45cff4. Final-source full local check: 3071 PASS, 0 FAIL, 3 existing conditional skips, audit 0. All 11 preceding-head checks passed but are historical now; new CI is running. Existing independent reviewer requests are retained; no approval is supplied by the author. Scope remains the generic #606 repair path, with no duplicate task store or business-case logic.

[PeterGuy326]

Main advanced through independently approved #506 (8930e45). Integrated it without conflicts or history rewrite; current head 85a6d24. Fresh install and full final-source check: 3082 PASS, 0 FAIL, 3 existing conditional skips, audit 0. New hosted CI is running; independent non-author approval remains required on this head. The #606 scope and existing review requests are unchanged.

[PeterGuy326]

Approved main #618 advanced the relay/thread-context path. Integrated it by ordinary conflict-free merge; current head d506942. Re-inspected the shared dispatch seam: group repair skip and shipped relay handoff remain separate; no duplicate task/context store. Fresh npm ci + full final-source gate PASS 3097, 0 failed, 3 existing conditional skips, audit 0. New CI is running; independent approval is still needed on this exact head. Consolidated the PR body so the current evidence is clear; previous receipts remain in comments/#607.

[Bindy-lbb]

Review 结论：建议修复 3 个问题后再合并

审阅固定版本：d5069427，merge base 为 c569aa87。全部 8 个变更文件均已审阅，包括默认工具筛选之外的测试和文档；这是文件审阅范围，不是测试覆盖率。以下结论来自代码检查及真实 HTTP/session/store + 确定性 driver 的复现。

1. [P1 / High] 失败返工的普通“重试”会绕过反馈注入和三次上限

位置：routes/turns.ts:259–261，关联 rework.ts:16。

prepareGoalRework 在解析 retryOf 原始记录之前运行，依赖当前请求自带的 goalId/branchId。实际 TurnPanel.retry → App.createSessionTurn 只发送原 input 和 retryOf，因此这里直接返回空上下文，反馈和上限都不再检查。

复现：完成原始任务 → 人工退回 → 第一次返工超时 → 连续重试 5 次。5 次请求全部返回 HTTP 200，模型累计调用 7 次（初次交付 + 首次返工 + 5 次重试），5 次重试均没有 Human-reviewed repair 或退回原因。在精确 head 上，重试记录还丢失任务标签；与当前 main 合并后的预览里，任务标签被 briefing 逻辑恢复，但发生在该检查之后，仍然可以连续重试而不触发上限。

建议：先从同会话的可信 retry source 解析任务身份，再执行返工校验。失败返工的重试应明确遵循返工策略：保留反馈并计入限额，或明确拒绝；不能退化成普通未绑定会话而跳过限制。请补实际 UI 请求形状（仅 retryOf）的回归。

2. [P1 / High] 返工进入工具审批后，审批恢复不能产出可验收的同任务结果

位置：rework.ts:47–51，关联新增的执行入口 hook。

精确 head 复现：人工退回 → 返工产生 approval.requested / engine.approval_required → 操作人批准。恢复执行虽然完成，但没有返工反馈，也没有原任务绑定；项目最新绑定执行仍是等待审批的失败回合，恢复产出无法通过原任务的人工验收入口取得。

当前 main 集成预览复现：#620 会为审批恢复补回任务 briefing/绑定，此时恢复进入这里，但 projectTaskDeliveries 看到最新的 approval-required 回合而返回空，直接抛 goal_conflict（the latest execution needs a new human review before another repair）。没有再次调用 driver，审批最终变成 indeterminate，列表显示 approval_execution_unknown。这需要解决与已合并功能的实际集成，而不是依靠旧 head 的绿灯。

建议：将批准后的 continuation 绑定为同一次返工，恢复其原退回来源和反馈，按审批规则重新验证；不要要求先验收一个尚待工具批准、没有完成交付的回合，也不要把恢复重复算作一次新返工。请覆盖单项和批量审批恢复。

3. [P2 / Medium] 反馈的 8-KiB 检查在过滤之前，过滤扩容后会静默丢失 verdicts 和原因

位置：rework.ts:58–63。

此处检查原始 JSON 的大小，随后 shared context assembler 才过滤凭据，并把每个字段截到 8 KiB。替换为 [redacted] 可能让内容变长，所以通过这里的反馈仍可能被截断。

复现：使用合法的单条 criteria："token=x ".repeat(500).trim()（小于 4096 字符），配合普通退回原因。反馈原文只有 4504 bytes，通过检查；过滤后实际 handoff input 被截为 8192 bytes，JSON 已不完整，位于 criteria 之后的 verdicts 和退回原因都消失，但请求仍返回 HTTP 200 并调用模型。精确 head 和 main 集成预览均可复现。这里的 x 是合成测试值。

建议：使用实际的同一过滤器处理完整 feedback 后再校验 UTF-8 字节数；无法完整携带时明确拒绝/要求拆分，不依赖后续静默截断。补一条过滤后扩容的边界回归。

验证与合并前事项

• 精确 head：构建通过；新增返工测试 13 项 + session-retry 4 项，17/17 通过。补充的 3 条真实 HTTP 复现均确认上述异常。
• 当前 main 33140136 的隔离合并预览：保留双方实现，仅处理合并冲突；构建通过；返工、session-retry、task-briefing、task-collaboration 四组 36/36 通过；相同 3 条补充复现仍确认问题。预览不是作者最终解决冲突后的版本。
• 当前 PR 与 main 的冲突位于 apps/server/src/routes/turns.ts 和 apps/desktop/packaging/runtime-layout.cjs。请同步 main、修复以上路径后，在新 SHA 上重跑 CI 并重新审阅。当前 head 的 11 个成功检查不代表新的集成结果已经验证。
• 使用 OCR delegate 模式确认文件范围和规则，由宿主代理直接审阅；OCR 外部模型接口未配置，未将其失败的 review 命令描述为成功结果。复现为本地 Node 22.23.2、隔离 loopback 环境和合成数据，未声明真实模型质量或本地完整跨平台验证。

本次结果仅发布在 PR 评论，未向该分支推送代码或文档改动。

[PeterGuy326]

Integrated main through #620/#624 to cdf8998, preserving both imports/runtime modules. Added a real HTTP integration regression: retry restoring saved briefing IDs must still enforce repair review. It failed before the ordering fix (200 instead of409); moving the existing guard after server-owned binding restoration makes it pass without another model call. Focused57/57 and restarted full final-source gate3131PASS/0FAIL/3existing skips/audit0. New hosted CI and independent approval pending. Existing reviewer requests retained; no self-approval or bypass. Pending #606 changelog entries stay Unreleased.