Agent-System-Documents/docs/reports/2026-10-13-fast-slow-u2-u3-code-review.md

独立审阅者 /root/fast_slow_final_review 的原件归属：/tmp/fast-slow-u2-u3-20261013/review/；原始重放日志在该目录。当前代码审阅PASS，模型质量FAIL；未复制评测材料或凭据。

Independent fix re-review

Reviewer /root/fast_slow_final_review, 2026-10-13. Read-only review; no source edits, deployment or paid model calls. Reviewed Backend 3d94c5df1a20bb6b2351a4cf353699bed77345a8..4a0418e211a260e89fe9ed79764138733ab7350c, Frontend c63f6c74707dc57fcdd1ed96e6a25cce32e7d61a..861c45edb5729a9ea1e08a1d53c5fa90cda15531; Core/UI unchanged from original review. Root delivery identity supplied as d25bd8a plus evidence-only updates; final CI/merge identity remains the executor gate.

Previous findings and supplemental startup review

• R1 deleted artifact indexing: CLOSED. Both lexical and vector cleanup calls restored after authoritative deletion. Independently replayed lexical deletion, and vector-row/retrieval deletion on isolated PG.
• R2 queued notice revalidation: CLOSED. Delivery validates task/attempt/conversation and uses current visibility-filtered title/summary before writing the system message and emitting results. Memory and persisted notice regressions pass.
• R3 same-name plugin: CLOSED. subRun.localCheckpoint records actual builtin registration; only that path intercepts the name. Plugin dispatch otherwise remains normal. Repro passes.
• R4 preview race: CLOSED. Async source open is owned by store and fenced by conversation epoch and explicit preview intent; later source opens, navigation, tab selection and close invalidate pending responses. Three added timing/navigation tests pass.
• Startup wiring: CLOSED for inspected/replayed paths. Recovery follows complete policy/CardSvc/budget/command/usage/PG event wiring and precedes public listen. Command consumption begins after recovery. Actual subprocess startup test confirms orphan queued Task is interrupted before HTTP readiness and disabled project mirroring stays disabled.
• Bound legacy child resume: CLOSED. Canonical task run IDs excluded, binding query failure stops automatic resume. Bound child test and existing unbound legacy/recovery-hold tests pass.
• SSE readiness: subscription now precedes readiness flush; consistent with avoiding a ready-before-subscribed gap. Source inspected; no separate new live-SSE replay claimed by this reviewer.
• Source catalog duplication: removed from the higher-level builder while retaining the original one; no source identity change.

Independent REPLAY

Raw logs record exact command and object. These are reviewer REPLAY, not GATE.

1. Backend SQLite 4a0418e211a260e89fe9ed79764138733ab7350c: go test ./internal/store ./internal/runmgr ./internal/runtime ./internal/app -run 'TestReviewDeletedArtifactMustLeaveKnowledge|TestReviewQueuedNoticeMustRevalidateSource|TestReviewCheckpointNameMustNotInterceptPlugin|TestTaskStartupReconcilesQueueBeforeServing|TestTaskBoundInterruptedChildDoesNotRespawn|TestResumeInterruptedChildrenRespawns|TestRecoveryHold' -count=1 -v — exit 0, backend-fixes.log.
2. Same Backend, isolated PG 127.0.0.1:39334/fast_slow_test, Redis 127.0.0.1:39335: go test ./internal/store ./internal/runmgr -run 'TestReviewDeletedArtifactMustLeaveKnowledge|TestDeletedArtifactClearsVectorRowsAndRetrieval|TestReviewQueuedNoticeMustRevalidateSource|TestTaskBoundInterruptedChildDoesNotRespawn|TestTaskReconcileOwnedQueueAndForeignLease' -count=1 -v — exit 0, backend-fixes-pg.log.
3. Frontend 861c45edb5729a9ea1e08a1d53c5fa90cda15531: ./node_modules/.bin/tsc --noEmit && ./node_modules/.bin/vitest run test/work-state.test.ts — exit 0, frontend-fixes.log.

New finding from real evidence

Important R5 — terminal card selects execution commentary instead of the final result.

• internal/runmgr/runmgr.go:1813 assembles fullText := partsText(subParts). partsText at approximately line 2396 includes PartText with Process=true. internal/runmgr/task_projection.go:340 then selects the first short paragraph from that combined text for the conclusion.
• Actual persisted candidate A01 card event 544/revision6 is completed, but conclusion says it will search knowledge, publish a phase result, then output the full analysis. These are earlier execution notes followed by a heading, not a result. The same text reaches the task-bound notice.
• Raw evidence: candidate-A01-persisted-events.json, event 544; original subagent assistant message run_21039c462892 has two process text parts before its 2829-character final non-process result.
• Fix: separate final non-process answer text from the full execution transcript when building the completion payload; derive a faithful complete result paragraph or clearly neutral fallback from that final answer only. Preserve the process transcript for expanded reading. Test pre-tool notes + final answer, heading-prefixed final answer, and long numeric/date-containing conclusions without arbitrary truncation.

Declined to judge

• Revised prompts/harness edits after Backend4a0418e: not part of this fixed review object; require their own affected review/replay.
• Candidate A02/A04 model quality: not yet present in the supplied evidence at this review time.
• Ten-second user readability and full real browser acceptance: not independently browser-replayed here.
• Normal notification-model response quality: controlled comparison suppresses it equally; this review checks notice content mechanics.
• Billing charges: provider token reports can be reconciled, actual monetary billing was not supplied or accessed.
• Existing services/deployment, final CI/platform admission and cleanup: outside this reviewer execution scope.

Verdict

Original four blockers and startup defect are resolved on the named component SHAs. Overall ready to merge: No. R5 remains a user-visible correctness issue; real-model A01 acceptance also fails and cannot be relabeled as passing because quotes exist or mechanism tests pass. See model-assessment.md for independent claim-by-claim assessment and comparison limitations.

Follow-up on Backend 3e330e3d37293556d6ca475e251748bd1a396cd4

Inspected the complete 4a0418e..3e330e3 delta. Canonical completion now builds its payload from non-Process parts while retaining all parts in the persisted transcript; empty final body gets an honest fallback. R5 is CLOSED on this SHA. Independent command go test ./internal/runmgr -run 'TestTaskConclusionSkipsProcessText|TestTaskCheckpoint|TestMaterial' -count=1 -v exited0; full output/object recorded in conclusion-recipe-fix.log.

The new recipe explicitly distinguishes total/phase scope, inferred arithmetic/source fact, proposed/approved state, and unspecified numbering or unique claims. It requires natural-language visible labels and meaningful separate reading/crosscheck discoveries without inventing quantity. Recipe identity advances to v5, so stale v4 results are not reused. These are defensible changes addressing the observed failure; text inspection and mechanism tests do not establish model-quality success.

Harness clears the Cards slice before each decode, preserving previously captured Task objects and preventing omitted fields from persisting into later responses. Remaining-case selection only accepts candidate A02,A04 and writes candidate-remaining.json. Original A01 remains untouched. Remaining allowance support is test-only; no production route/credential change reviewed.

Updated code verdict: No remaining identified code blocker on the inspected Backend3e330e3 / Frontend861c45e combination. Overall U3 acceptance and merge verdict remain pending/No because original A01 failed and revised A01 is not run; A02/A04 and final exact-head GATE results still require assessment. This is not a claim that prompt refinement fixes the historical failed sample.

Final minimal review — Backend ee65604d831caed768f6c798cda6122acb33f13d

Reviewed the full f6b5360..ee65604 delta read-only; checkout was clean at verification. No paid model or credential use.

• material_record admission now constructs Text only from host source labels and exact validated quotations, deduplicates identical label/quote lines, and clears model Impact/Assumptions. Source ownership, current-version witnesses, atomic projection and final64KiB validation remain in place. Raw candidate digest is computed before canonicalization, preserving same-ID/different-payload conflicts and idempotence. Interpretive kinds remain unchanged and do not acquire verified status. Recipev6 prevents reuse under the older recipe identity.
• The oversized-correction regression deliberately uses analysis text now, so canonicalization does not accidentally turn the old overflow test into an ordinary small-payload test. Correction history/fallback coverage remains meaningful.
• Evaluation wrapper now preserves any positive output cap below4000; it only supplies4000 when uncapped or above4000. The HTTP fixture confirms the actual request carries780 when production supplied780. This closes the fixture fidelity defect prospectively; it does not alter the interpretation of old A02 output obtained with the previous cap.

Independent REPLAY:

1. Isolated PostgreSQL: go test ./internal/runmgr -run 'TestMaterialRecordCannotAddAnUnstatedApproval|TestTaskCheckpoint' -count=1 -v — exit0; object and full raw output in literal-record-pg.log.
2. Local fake HTTP only: go test ./internal/server -tags live -run '^TestEvaluationPreservesProductionOutputCap$' -count=1 -v — exit0, subcases780→780,5000→4000,0→4000; raw output in eval-cap-fix.log. The live build tag does not imply a live-model request; this exact test uses httptest and fixture credentials only.

Final code-review assessment on the inspected delta: no remaining identified blocking code defect. Original blockers, startup recovery, process-summary and evaluation-cap defects are closed on the reviewed sequence of SHAs. Remaining model limitations are explicitly retained: canonicalization prevents unsupported wording in newly admitted material_record findings, but it does not verify selected context, analysis/provisional/needs_user conclusions, final prose, or old persisted results. Previously observed stale nextStep text remains a readability limitation in the historical samples.

Overall U3 acceptance/merge verdict remains NOT PASSED / pending: originalA01 failed; revisedA02 had semantic errors and cap-fidelity limitations; revisedA04 exhausted budget before final output. No additionalA01 run was executed by this reviewer or credited as evidence. A separately authorized new sample, if any, must be judged on its actual final recipe/cap and cannot retroactively rewrite the original failures. Final exact-head CI/platform admission remains the executor's gate.

Terminal guidance follow-up — Backend 2a64432fc896e7a44d793fb8eb8b10c77fd5905d

Inspected ee65604..2a64432. Normal completion/failure/cancellation through finishTaskProjection now replaces model NextStep with host-owned guidance; quota failure uses readable wording and precise execution exceptions are retained in Run.Error. No authority/schema/budget expansion. Independent go test ./internal/runmgr -run 'TestFailedTaskDoesNotPromiseAutomaticContinuation|TestBackgroundCardLifecycle|TestTaskConclusionSkipsProcessText' -count=1 -v exited0; terminal-state-replay.log contains exact object and full output.

One same-scope omission remains on this SHA: reconcileTaskProjections at task_projection.go approximately297-309 separately sets interrupted but keeps copied NextStep. Orphan queued cards therefore still promise to start after capacity appears; interrupted running cards can still promise final output despite canonical automatic resume being disabled. Set host-owned stopped/request-again guidance on this recovery path as well and assert it in queue/startup recovery tests before claiming all terminal paths are covered. Reported to executor for this narrow correction. Original model acceptance status remains unchanged.

Final recovery closure — Backend 6a2d85cbb9d1692dc2abf85ef59e3f5ff23748f0

Reviewed the narrow 2a64432..6a2d85c delta. reconcileTaskProjections now overwrites NextStep with host-owned stopped/request-again guidance. The existing recovery test asserts it, while retaining the foreign live PostgreSQL task check. The terminal-recovery omission is CLOSED.

Independent PostgreSQL REPLAY: go test ./internal/runmgr -run '^TestTaskReconcileOwnedQueueAndForeignLease$' -count=1 -v — exit0 on exact Backend6a2d85c; full raw output in final-recovery-nextstep.log. No checkout edits, existing-service operations or paid calls were performed.

The accompanying fake mechanism stream now chooses a real acceptance-condition quotation when available instead of supplying the same launch quotation for both material records. Frontend 4d1d64a..3087b0621cc1bf6c1ece39c5f4e148e24ef1cf59 changes only four browser expectation lines to match literal source text. No Frontend production behavior changes in that final delta. I inspected these changes; the executor's actual HTTP/browser replay was not rerun or represented as independent browser evidence here.

Final code-review verdict: PASS / no remaining identified blocking code findings for the reviewed U2/U3 change sequence, ending Backend6a2d85c and Frontend3087b06, with unchanged Core6e38574 and UI3b34dfd. This is a code-review verdict, not overall delivery acceptance. The inherited login feature was reviewed only at the merge/startup boundary as recorded above. Root7fd5288/fresh-main4ad5b3c and Documents53a7d37 were supplied as delivery composition metadata; full root build/test/CI and planning closeout are executor gates, not independently asserted by this focused replay.

Overall U3 acceptance remains NOT PASSED / pending on the six paid samples. OriginalA01 failure, revisedA02 semantic/fidelity limitations and revisedA04 budget-stopped final output remain documented without alteration. Additional failed-sample rerun approval has not been received or assumed by this reviewer. No code-review PASS should be used to state model acceptance, exact-head CI or merge approval is already complete.

Post-disconnection recovery review — Backend e69865e11ba3f74887f92c2433b5ec37c881d0db / Core beafd7537a6770bd6888095b48926980fce487d8

Read-only review resumed at the current exact objects. Inspected zero-findings fix62a2ab4, upstream merge7855d82 and fixture adaptatione69865e. Checkout was clean; before/after object IDs stayed identical during all replays.

The zero-accepted-findings failure copy is now truthful: only active findings count as available phase findings; absent active findings yield outcome none, explicit no-usable-findings conclusion, honest remaining work and a new-request suggestion without implying stored results. The exact budget exception remains in Run.Error. This closes the extraA01 zero-findings display finding prospectively; original failed sample remains unchanged.

Merged coexistence is intact: Core retains TaskSnapshot/DocumentOrigin while adding SettingScope. Production app injects plugin schema, Cards, execution budgets, shared authorization, mirroring policy, command wiring and PG event wiring before recovery/public intake. SSE still subscribes before readiness, filters task source visibility on replay/live send, deduplicates seq, and now independently rechecks session validity on the upstream timer. No U2 task/source guard was lost in the merge.

The isolated eval fixture now uses one settings service with trusted SetSystemOverride(config), shared by Manager/Server/plugin host. This is compatible with system-managed models/runtime.mock; it avoids silently stripped user overrides. It does not modify a real account setting or introduce a provider/credential change. Material intake thresholds and maxAttachmentChars remain normal effective scoped settings; original material revision logic consumes the effective cap.

Independent REPLAY (all exit0, all exact before/after SHAs recorded):

• post-disconnect-sqlite.log: Backend SQLite, go test ./internal/runmgr ./internal/store ./internal/cardsvc ./internal/server ./internal/app ./internal/settings -run 'TestFailedTaskDoesNotPromiseAutomaticContinuation|TestTask|TestMaterial|TestCardsAtCursor|TestReviewQueuedNotice|TestSSE|TestMergeUserDottedKeyMaskAndNormalizeStored' -count=1 -v. Covers zero findings, canonical queue/recovery, checkpoint/source authority, atomic card/event failure, visibility, production startup, session SSE invalidation and managed-setting scope behavior.
• post-disconnect-pg.log: same Backend, isolatedPG, go test ./internal/runmgr ./internal/store ./internal/cardsvc -run 'TestFailedTaskDoesNotPromiseAutomaticContinuation|TestTask|TestMaterialRecord' -count=1 -v. Covers transactional projection/source/recovery under PG.
• post-disconnect-fixture.log: same Backend, go test ./internal/server -tags live -run '^(TestExtraA01LimitsActualRunIDs|TestEvaluationPreservesProductionOutputCap)$' -count=1 -v. Fake streams/HTTP only, no paid models.
• post-disconnect-core.log: Corebeafd753, go test ./... -count=1.

Current code review: PASS for the inspected U2/U3 fixes and integration boundary; no remaining identified blocking code finding. This does not certify all unrelated upstream settings/auth behavior, final root CI, deployment or overall model quality. Overall U3 quality remains FAIL/not passed, including the explicitly approved extraA01. Source can be prepared as a draft/reviewable candidate; no merge or additional paid sample is justified by these mechanism tests. Raw model evidence and all original failure assessments remain applicable as recorded.

Code-review receipt — latest account hardening integration

Reviewer /root/fast_slow_final_review; source level REPLAY, not GATE; date2026-10-13. Exact reviewed Backend 0bc4b0f794809e048744a4bf0c94a0835cc32665, merge parents e69865e11ba3f74887f92c2433b5ec37c881d0db and cfb8308895d0b94765ba10ed0e5b9bf38b439414. Clean checkout, exact SHA unchanged before/after both independent runs. Machine-readable receipt with raw-log SHA256 identities: account-merge-review-receipt.json.

Read the incoming app/server/accounts/auth/dialect/migration/SSE delta against U2/U3 integration boundaries. The production run-manager, task projection, task-source endpoint, source visibility filter and notice revalidation files have no diff from the previously reviewed candidate. App startup order remains full-wiring/PG-event preparation → reconciliation → pending notices → legacy-unbound resume → command consumer → public HTTP serving. Incoming app changes affect later CLI account/bootstrap/storage helpers, not that startup block.

SSE now periodically rechecks API keys as well as sessions, keeping the original authenticated owner fixed. Existing source visibility filtering still executes for both replay and live card payloads; subscribe-before-readiness, seq dedup and end-of-stream behavior remain intact. Source preview still binds owner/conversation/project and expected content hash. Notice redelivery still revalidates card/task/attempt and current source visibility before creating a new message. The added bootstrap lock has a distinct namespace and does not replace task admission/channel locks. Migration29 adds only redemption-attempt storage; independent migration and task projection replays pass.

Independent focused checks on exact0bc4b0f, both exit0:

1. SQLite: go test ./internal/server ./internal/cardsvc ./internal/runmgr ./internal/app ./internal/store -run 'TestSSE(SessionInvalidation|APIKeyRevocation)ClosesStream|TestTaskSource|TestTaskVisibility|TestReviewQueuedNoticeMustRevalidateSource|TestTaskStartupReconcilesQueueBeforeServing|TestTaskReconcileOwnedQueueAndForeignLease|TestTaskBoundInterruptedChildDoesNotRespawn|TestFailedTaskDoesNotPromiseAutomaticContinuation|TestTaskProjection|TestMigrationUpgradesExistingDatabase' -count=1 -v — account-merge-sqlite.log.
2. IsolatedPG: go test ./internal/server ./internal/cardsvc ./internal/runmgr ./internal/store -run 'TestTaskSource|TestTaskVisibility|TestReviewQueuedNoticeMustRevalidateSource|TestTaskReconcileOwnedQueueAndForeignLease|TestTaskProjection' -count=1 -v — account-merge-pg.log.

Code-review receipt verdict: PASS for the U2/U3 change and this incoming integration boundary; no new blocking finding identified. Review does not certify the entire separate account-hardening feature or all possible credential/replay timing behavior. No model calls, credentials, source edits, deployment or merge operations occurred in this review.

Overall U3 model acceptance remains FAIL, not merge-ready: extraA01 has zero accepted checkpoints and no final child result; original model findings remain unchanged. Dev baseaa6364f, Corebeafd75 and FE0bbccbe are supplied composition metadata, not a substitute for the executor's final exact-root test/build and CI. This receipt supports a draft/reviewable candidate only while the quality gate remains failed.

Declined in this final receipt: full independent audit of unrelated account lifecycle/CLI features; real-model revalidation (not authorized); monetary billing; final root/platform CI; production deployment. Existing original review declined-scope notes remain applicable where not superseded by later focused replays.