feat(daemon): persist session artifacts across restarts#6557
Conversation
Re-run at maintainer request — Stage 1 (Gate)Template ✓ — all required sections present, bilingual. Problem: Observed gap, not theoretical. V1 artifact metadata is daemon-memory-only; restart loses it. This is a direct follow-up to #5895 (merged V1), and anyone who's restarted a daemon mid-session has hit this. No reproduction needed — the missing feature is self-evident. Direction: Aligned. Artifact metadata persistence across daemon restarts sits squarely in the session-management roadmap area. The V1→V2 phasing was established in #5895. Content retention is explicitly deferred, keeping the scope honest. Size: Large PR touching core infrastructure across four packages:
Core paths ( Approach: Well-structured — JSONL journal/snapshot in the existing chat recording (no new storage layer), rebuild on session load, fork remapping, tombstone tracking, workspace integrity metadata (sha256/mtime), cross-client ownership enforcement. This matches what I'd propose independently. The scope is appropriate — content retention is carved out, and the remaining metadata-persistence surface is cohesive. Test coverage (~6,112 lines for ~3,695 production lines) is thorough. No concerns. Moving to Stage 2. 中文说明维护者要求重跑 — Stage 1（门控）模板完整 ✓ — 所有章节齐全，双语。 问题： 已观测的功能缺口，非理论性问题。V1 artifact metadata 仅存在于 daemon 内存中，重启后丢失。作为已合并 #5895（V1）的直接后续，问题显而易见。 方向： 对齐。跨 daemon 重启的 artifact metadata 持久化完全在 session-management roadmap 范围内。 规模： 大型 PR：~3,695 行生产逻辑，~6,112 行测试，893 行设计文档。维护者已审阅并批准。 方案： 结构合理——JSONL journal/snapshot 复用现有 chat recording。Scope 适当，测试覆盖扎实。无顾虑。 — Qwen Code · qwen3.7-max |
Re-run at maintainer request — Stage 2 (Review + Test)Code Review (re-run)Reviewed the key production files on the current PR head. No new findings since the prior run. Blocker fix confirmed in code: Orphaned sticky IDs minor resolved: All four findings from the previous re-review are resolved:
Test ResultsUnit tests — all pass: Build + typecheck: clean across all packages. Real-Scenario TestingStarted Daemon startup — # capabilities check
>>> 'session_artifacts_persistence' in features
TrueArtifact creation — restorable persisted, ephemeral stays live-only: // POST /session/:id/artifacts (no retention field → defaults to restorable)
{
"artifact": {
"id": "9393235d1f7b35a9",
"title": "Restorable Link",
"retention": "restorable",
"persistedAt": "2026-07-09T12:59:52.327Z"
}
}
// POST with retention: "ephemeral" — no persistedAt, not written to disk
{
"artifact": {
"id": "f6798c4ce0bbc8c8",
"retention": "ephemeral",
"restoreState": "live"
}
}JSONL verification — artifact event persisted, no clientId in durable record: {
"type": "system",
"subtype": "session_artifact_event",
"systemPayload": {
"v": 2,
"changes": [{
"action": "created",
"artifactId": "9393235d1f7b35a9",
"artifact": {
"title": "Restorable Link",
"retention": "restorable",
"persistedAt": "2026-07-09T12:59:52.327Z"
}
}]
}
}
// clientId ABSENT in durable record — blocker fix confirmed on disk
// No record for ephemeral artifact f6798c4ce0bbc8c8Daemon restart + session/load: requires a model provider agent channel (not available in CI). The maintainer ( SummaryNo new issues. All previously identified findings resolved. 230 unit tests pass, build/typecheck clean, JSONL persistence verified on disk. 中文说明维护者要求重跑 — Stage 2（审查 + 测试）代码审查无新发现。阻塞修复已在代码和磁盘上确认。前次 re-review 的四个发现全部已解决。 230 个单测通过。构建和类型检查全部通过。 真实场景测试：daemon 已启动， session/load 恢复路径需要模型提供者（CI 无），但维护者已通过 32/32 E2E 检查独立验证。 无新问题。 — Qwen Code · qwen3.7-max |
Re-run at maintainer request — Stage 3 (Final Assessment)This re-run confirms what the prior triage found, with all outstanding issues now resolved: The PR is well-engineered. The motivation is real (artifact metadata lost on restart), the approach is sound (reuse chat JSONL as persistence backbone), and the implementation is thorough — atomic state management, branch-scoped replay, fork remapping, tombstone tracking, workspace integrity metadata, cross-client ownership enforcement, and secret scrubbing. All four findings from the prior code review are fixed. The previously identified blocker (restored client artifacts being permanently undeletable after restart) is resolved by dropping 230 unit tests pass. Build and typecheck are clean across all packages. JSONL persistence was verified directly — the artifact event is written with correct schema, The maintainer ( Verdict: Ready to merge. Approving. ✅ 中文说明维护者要求重跑 — Stage 3（最终评估）本次重跑确认了前次 triage 的结论，所有未解决问题现已修复： PR 工程质量高。动机真实（重启后 artifact metadata 丢失），方案合理（复用 chat JSONL），实现充分——原子状态管理、分支范围 replay、fork remapping、tombstone tracking、workspace integrity metadata、跨 client ownership 强制、secret 清洗。 前次代码审查的四个发现全部修复。先前的阻塞问题（恢复的 client artifact 在重启后永久无法删除）已通过从 durable 记录中丢弃 230 个单测通过。构建和类型检查全部通过。JSONL 持久化已直接验证。 维护者已独立完成全面的 E2E 验证（32/32 + 因果 A/B）。 结论： 可以合并。批准。 ✅ — Qwen Code · qwen3.7-max |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #6557 — feat(daemon): persist session artifacts across restarts (replacement for #6259)
Type: New Feature
Change size: +8007/-176 across 32 files, single squashed commit
HEAD: 0ba3b7cd
This is NOT a simple squash of #6259 — it contains substantial additional hardening: TOCTOU-safe workspace file detection (O_NOFOLLOW open + handle-based stat), input validation at persistence layer (length limits, XSS filtering, truncation), history gap detection, partial restore failure detection, payload validation before persisting, URL secret detection, sticky ephemeral cap at 500, and multi-workspace routing integration.
Findings Summary
- Major: 2
- Minor: 2
- Nit: 0
Findings
Major: mergeRetention silently undoes sticky ephemeral overrides during same-session re-upsert (sessionArtifacts.ts ~L1579-1594)
applyStickyEphemeralOverride (L1184) sets retention: 'ephemeral' but does NOT set retentionExplicit: true. When the same artifact is re-upserted in the same session (e.g., tool re-generates the same file):
- Sticky override fires →
retention: 'ephemeral',retentionExplicit: false mergeRetentionevaluates:- Guard 1:
incoming.retentionExplicitisfalse→ skip - Guard 2:
existing.retentionExplicitisfalse→ skip - Falls through to
strongestRetention('restorable', 'ephemeral')→'restorable'
- Guard 1:
- Sticky override silently undone. Artifact becomes restorable, gets persisted.
The override works correctly on restore (applied post-merge), but not on same-session re-upsert. Fix: have applyStickyEphemeralOverride also set retentionExplicit: true, or add a sticky flag that mergeRetention recognizes.
Major: Rewind handler synthesizes empty snapshot on loadSession returning undefined (carried from #6259 review)
Still present in this PR. When loadSession returns undefined in the rewind handler, a synthetic empty snapshot is constructed and passed to store.restore(), which clears all durable artifacts. If undefined was caused by a transient failure (not a genuinely empty session), durable artifacts are silently lost. The adjacent catch block (artifactSnapshotUnavailable path) correctly avoids this.
Fix: treat sessionData === undefined similarly to the catch path.
Minor: buildSnapshotPayload no longer filters orphaned sticky IDs (sessionArtifacts.ts ~L872, regression from #6259)
The #6259 version filtered stickyEphemeralIds to only include IDs still in this.artifacts. This PR removed that filter:
// #6259: Array.from(this.stickyEphemeralIds).filter((id) => this.artifacts.has(id))
// #6557: Array.from(this.stickyEphemeralIds)Orphaned sticky IDs accumulate in every snapshot until the LRU cap (500) evicts them. Functionally benign on restore (the override only fires on matching artifact IDs), but every snapshot carries dead weight and the sticky set's effective capacity is reduced.
Minor: durableTombstoneRequired leaks into API responses (sessionArtifacts.ts L377, L1319)
remove() correctly strips durableTombstoneRequired before returning (L514). But upsertMany published-upgrade removal (L377) and evictOverflow eviction removal (L1319) set the field and never strip it. Both flow through persistChanges → SessionArtifactMutationResult.changes → SSE publish. Internal persistence marker leaks to clients.
Fix: strip in persistChanges or applyDurableMarkers after consumption, not just in remove().
Verified Improvements Over #6259
- TOCTOU-safe workspace detection:
lstat→open(O_NOFOLLOW)→handle.stat()→isSameFile()comparison. Properly handles symlink swaps between check and read. - Input validation hardening: Length limits on all persisted string fields (
MAX_PERSISTED_*_CHARS), XSS/control-character filtering in metadata, artifact list capped at 500 with truncation warnings. MAX_WORKSPACE_HASH_BYTES(100MB): Files above this size skip SHA256, preventing unbounded hashing.- Payload validation:
parseSessionArtifactEventPayload/parseSessionArtifactSnapshotPayloadvalidate through normalization before persisting. - Partial restore failure detection: Separate prefixes for total vs partial failure,
isArtifactRestoreFailureWarning()export. rememberStickyEphemeralcap: 500 entries, correct LRU via Set delete+add.rememberTombstoneLRU: 500 entries, correct eviction withtombstonedClientIdscleanup.- URL secret detection:
hasSecretLikeUrlComponentcatches tokens in query params/fragments.
Multi-Workspace Routing
The multi-workspace routing functions in session.ts are pre-existing infrastructure, not introduced by this PR. The PR's actual route changes (~33 lines) are artifact ownership enforcement (requireSessionArtifactClientId) and retention/clientRetained field forwarding. One pre-existing Major was noted (export route hardcodes boundWorkspace) but is outside this PR's scope.
Final Verdict
COMMENT. Two Major findings should be addressed: the mergeRetention sticky ephemeral gap (new regression from the merge logic) and the rewind empty snapshot (carried from #6259). Two Minor findings are noted for cleanup. The overall hardening in this replacement PR is substantial and well-executed.
This review was generated by QoderWork AI
|
Addressed the latest review in Fixed:
Triage note:
Validation:
Known baseline: |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/core/src/services/session-artifact-persistence.ts:377 |
Dead branching: if (change.action === 'removed') branch and fallthrough produce identical output in remapSessionArtifactPayloadForFork |
Remove the if branch; keep only the fallthrough |
packages/cli/src/acp-integration/acpAgent.ts:3263 |
as LoadSessionResponse / as ResumeSessionResponse type assertions mask SDK type mismatch — artifactSnapshot is spread onto the response but not in the SDK type |
Define type LoadSessionResponseWithArtifacts = LoadSessionResponse & { artifactSnapshot?: RebuiltSessionArtifactSnapshot } and use it as the return type |
packages/acp-bridge/src/sessionArtifacts.ts:873 |
Same 12-field anonymous inline type declared twice (return of cloneState() and parameter of restoreState()) — any field change must be maintained in two places |
Extract a private named interface for the snapshot state |
packages/acp-bridge/src/sessionArtifacts.ts:1051 |
Snapshot failure log omits consecutiveSnapshotFailures, backoff multiplier, and durableEventsSinceSnapshot — operators cannot distinguish first transient failure from prolonged breakage |
Add diagnostic fields to the log line |
packages/core/src/services/session-artifact-persistence.ts:581 |
normalizePersistedArtifact returns undefined silently when !isRecord(value) without pushing to warnings[] — inconsistent with other rejection paths in the same function |
Add warnings.push(...) before return undefined |
packages/core/src/services/session-artifact-persistence.ts:724 |
normalizeMetadata silently skips entries with control characters, unsafe payloads, long keys, prototype keys — no warning pushed despite warnings[] being available |
Add batched warning for filtered entries |
packages/acp-bridge/src/sessionArtifacts.ts:1196 |
Tombstone eviction uses LRU-on-re-tombstone via Set.delete()+Set.add() ordering, but the policy is undocumented — future maintainers will assume FIFO |
Document the LRU eviction policy in a comment |
packages/acp-bridge/src/sessionArtifacts.ts:971 |
persistChanges mutates caller's changes array entries in place without documenting the shared-mutation contract |
Add a docstring documenting the in-place mutation |
packages/acp-bridge/src/sessionArtifacts.ts:653 |
restore() second rollback condition triggers on any completeness warning (including MAX_PERSISTED_ARTIFACTS truncation) even when all retained artifacts restore successfully — could prevent restore in sessions with 500+ artifacts |
Tighten the condition: add restoredCount < snapshot.artifacts.length or exclude truncation-only warnings |
— qwen3.7-max via Qwen Code /review
|
Addressed the latest suggestion summary in Fixed:
Triage notes:
Validation:
|
|
Updated the PR for the 1-8 review decisions and pushed Implemented follow-ups:
Local validation:
|
| let sessionId = fallbackSessionId; | ||
| let sawRecord = false; | ||
|
|
||
| for (const record of records) { |
There was a problem hiding this comment.
[Suggestion] Rebuild still normalizes every historical artifact record from the beginning even though each later snapshot clears and replaces the artifact state. That means periodic snapshots do not bound replay CPU/allocation; load/fork still scale with the full artifact journal.
Suggested fix: locate the latest valid snapshot first, seed from it, and replay only later events, falling back to older snapshots or full replay only when no valid snapshot exists.
— gpt-5 via Qwen Code /review
There was a problem hiding this comment.
Valid suggestion, but I am deferring it from this correctness fix. Latest-snapshot-first rebuild is a replay performance optimization and should be handled as a focused follow-up so the rebuild ordering/fallback behavior gets dedicated tests. Leaving unresolved rather than marking this as fixed.
| ); | ||
| artifact.status = status.status; | ||
| artifact.sizeBytes = status.sizeBytes; | ||
| const changed = isWorkspaceContentChanged(artifact, status); |
There was a problem hiding this comment.
[Suggestion] When a workspace file is touched without changing content, getWorkspaceStatus() hashes it and returns the new sha256/mtimeMs, but this path only updates status and size. The stored qwen.workspace.mtimeMs remains stale, so after the TTL the same file is hashed again instead of taking the size+mtime fast path.
Suggested fix: when the refreshed status is available and unchanged, merge the refreshed workspace hash/mtime metadata back into the stored artifact metadata.
— gpt-5 via Qwen Code /review
There was a problem hiding this comment.
Valid suggestion, but I am deferring it from this review-fix commit. Refreshing unchanged workspace hash/mtime metadata is a performance/cache optimization; it does not change persistence correctness, and it deserves a narrow follow-up with stale-TTL/hash-path coverage. Leaving unresolved rather than marking this as fixed.
|
Addressed the latest review in Fixed and resolved threads:
Left unresolved for design/maintainer decision:
Deferred suggestion-level follow-ups:
Validation:
Known baseline: root |
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/core/src/services/session-artifact-persistence.ts:719 |
Restore-path normalizeMetadata does not filter secret-like metadata keys (authorization, token, apiKey) or secret-like values (bearer tokens, sk-* keys) that the live-ingestion path enforces. Old or tampered JSONL records with secret-bearing metadata pass through restore unfiltered, contradicting the stated defense-in-depth claim. |
Import or duplicate isSecretLikeUrlText and isSecretLikeMetadataValue into the persistence module's normalizeMetadata, or export a shared validator from a common module. |
packages/core/src/services/session-artifact-persistence.ts:399 |
remapSessionArtifactForFork deletes contentRef and expiresAt from forked artifacts but not clientId. The PR design contract states clientId is dropped during fork, but the fork remap does not enforce it at the persistence boundary. A future version that persists clientId would silently leak source-session ownership into forked sessions. |
Add delete next.clientId; alongside the existing delete next.contentRef; delete next.expiresAt;. |
— qwen3.7-max via Qwen Code /review
|
Addressed the latest critical review in Fixes included:
Validation:
|
|
Thanks for the re-verification. I added the non-blocking migration note to the PR description in both English and Chinese:
No code change was needed for this follow-up. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
|
@qwen-code /resolve |
Merge origin/main into feat/session-artifacts-persistence-v2-design-clean. The only conflict was in packages/sdk-typescript/scripts/build.js where both branches independently bumped MAX_DAEMON_BROWSER_BUNDLE_BYTES (PR: 140KB for history_truncated/transcript-status; main: 139KB for workspace ACP status/preheat). Resolved by taking the combined budget of 141KB to accommodate both feature sets.
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge Conflict Resolution Summary — PR #6557ConflictFile: Both branches independently bumped the
ResolutionTook the PR branch's higher value as the starting point, then added 1KB to account for main's separate ACP status/preheat feature that the PR branch didn't include. Final value: 141KB. Preserved both branches' comment audit trails so the bump history remains traceable. CommitSingle file modified: |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
No critical issues found. Build passes, 227 tests pass. Suggestion-level recommendations are in the Suggestion summary comment below.
— qwen3.7-max via Qwen Code /review
Re-Review at HEAD
|
| # | Finding | Severity | Status |
|---|---|---|---|
| 1 | mergeRetention silently undoes sticky ephemeral overrides |
Major | ✅ FIXED — applyStickyEphemeralOverride now sets retentionExplicit: true |
| 2 | Rewind synthesizes empty snapshot on loadSession returning undefined |
Major | ✅ FIXED — rewind handler distinguishes undefined (transient failure → artifactSnapshotUnavailable) from genuinely empty session |
| 3 | buildSnapshotPayload no longer filters orphaned sticky IDs |
Minor | Array.from(this.stickyEphemeralIds) without .filter(id => this.artifacts.has(id)) |
| 4 | durableTombstoneRequired leaks into API responses |
Minor | ✅ FIXED — stripDurableTombstoneMarkers() called on all mutation paths |
3 of 4 findings addressed. The remaining Minor (orphaned sticky IDs in snapshots) is functionally benign on restore but wastes snapshot payload space and reduces effective sticky set capacity.
Additional Changes Since Last Review (24 new commits)
Notable additions:
- Fork marker identity preservation (
c1006e2b,02b1a851,1c427f48): tombstonedIds and stickyEphemeralIds now remapped during fork viaremappedArtifactIds, fork marker metadata sanitized. Correct improvement over clearing them entirely. - Rollback/restore warnings (
5aa80be0,b622d97a): artifact rollback and restore warnings surfaced with details. - Artifact persistence recovery hardening (
0de10d1f,503f4fee): restore boundaries tightened. - Artifact ownership capability alignment (
cf1df36a): ownership enforcement aligned across REST and ACP paths.
Verdict
Both Major findings from the previous review are fixed. One Minor remains (orphaned sticky IDs). The additional fork marker remapping and rollback warning surfacing are solid improvements. The PR is in good shape for merge once the remaining Minor is addressed or explicitly accepted.
|
Addressed the remaining Minor from the re-review in What changed:
Validation:
|
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Cross-repo lightweight review (no build/test/lint). 8 review agents + verification + reverse audit. 2 Critical, 7 Suggestion findings below.
| const MAX_TOMBSTONED_IDS = 500; | ||
| const MAX_STICKY_EPHEMERAL_IDS = 500; | ||
| const MAX_WORKSPACE_HASH_BYTES = 100 * 1024 * 1024; | ||
| const SECRET_TOKEN_VALUE_PATTERN = |
There was a problem hiding this comment.
[Suggestion] SECRET_TOKEN_VALUE_PATTERN, isSecretLikeMetadataValue, and URL-scanning helpers are copy-pasted independently in both sessionArtifacts.ts and session-artifact-persistence.ts with inconsistent naming (isSecretLikeUrlText vs isSecretLikeText). Bug-fixes to the regex or detection heuristics must be applied in two places or they silently diverge.
Extract the shared helpers into session-artifact-persistence.ts (or a shared internal module) and import them in sessionArtifacts.ts.
— qwen3.7-max via Qwen Code /review
| return this.enqueue(async () => { | ||
| const validationStrict = options.validationStrict ?? options.strict; | ||
| const persistenceStrict = options.persistenceStrict ?? options.strict; | ||
| const before = this.cloneState(); |
There was a problem hiding this comment.
[Suggestion] cloneState() deep-clones the entire artifact map (up to 200 entries), tombstonedIds, tombstonedClientIds, stickyEphemeralIds, and markerArtifacts unconditionally at the top of every upsertMany() call — even when persistence is disabled. For a session near the 200-artifact cap, each call allocates ~400 object spreads on the synchronous hot path.
Guard the clone behind a persistence check:
| const before = this.cloneState(); | |
| const needsRollbackClone = this.persistence !== undefined; | |
| const before = needsRollbackClone ? this.cloneState() : null; |
Then in the catch: if (before) this.restoreState(before);
— qwen3.7-max via Qwen Code /review
| toPersistedArtifact(change.artifact, change.artifact.updatedAt), | ||
| ); | ||
| } | ||
| while (this.stickyEphemeralIds.size > MAX_STICKY_EPHEMERAL_IDS) { |
There was a problem hiding this comment.
[Suggestion] When durable tombstone write fails, this rolls back and returns { changes: [], warnings: [...] }. The empty changes array is the same shape as the "artifact doesn't exist" no-op, making persistence failures indistinguishable from no-ops for callers checking changes.length === 0. Monitoring systems watching for removed events will never see one.
Consider including the attempted removal in changes with a rolledBack: true marker, or promoting the warning to the SSE event stream.
— qwen3.7-max via Qwen Code /review
| return { kind: 'managed_copy', contentId, sha256, sizeBytes, createdAt }; | ||
| } | ||
|
|
||
| function normalizeMetadata( |
There was a problem hiding this comment.
[Suggestion] The bridge's normalizeMetadata rejects secret-like key names (isSecretLikeUrlText(key)) and token-shaped values (isSecretLikeMetadataValue(value)), but this core module's version only checks prototype keys, length, control chars, reserved workspace keys, and byte budget.
Legacy JSONL with metadata: { "authorization": "Bearer sk-..." } passes through core normalization into the rebuilt snapshot. Currently mitigated by bridge restore validation, but any future consumer reading loadSession().artifactSnapshot directly would expose secrets.
Add isSecretLikeText(key) and isSecretLikeMetadataValue(value) checks here, mirroring the bridge.
— qwen3.7-max via Qwen Code /review
| @@ -1407,6 +2869,68 @@ async function getWorkspaceStatus( | |||
| } | |||
There was a problem hiding this comment.
[Suggestion] Files > MAX_WORKSPACE_HASH_BYTES (100MB) skip SHA256 at ingest. On restore with hashWorkspaceContent: false, when expectedSha256 is undefined (never computed) and sizeBytes matches but mtimeMs changed, the code returns status: 'available' — a same-size different-content replacement goes undetected, defeating the workspace integrity guarantee.
On restore, compute SHA256 when the artifact has no stored hash and the file is within the hash size limit:
| } | |
| hashWorkspaceContent: !options.workspaceExpected?.sha256 | |
| && (options.workspaceExpected?.sizeBytes ?? 0) <= MAX_WORKSPACE_HASH_BYTES, |
Or mark as restoreState: 'unverified' when SHA256 is unavailable for comparison.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
6 participants
What this PR does
Replacement for #6259 with the same net changes squashed into one commit for easier review.
Implements V2 daemon session artifact metadata persistence as a follow-up to #5895. The PR restores restorable artifact metadata across daemon restart/session replay, records durable tombstones and snapshots in JSONL, rebuilds artifact state during load/fork, remaps forked artifact ownership, and keeps ephemeral artifacts live-session-only.
The persistence boundary is metadata-only in this PR: workspace-backed artifacts record status metadata such as sha256/size/mtime for staleness detection, but artifact content is not retained, copied, quota-managed, or garbage-collected here. Content retention remains out of scope for this replacement PR.
Why it's needed
V1 artifact APIs exposed only live-session state, so page refresh could recover from a live daemon but daemon restart or historical load could not restore artifact metadata. V2 gives clients a stable metadata restore path while keeping content retention separate from this base persistence change.
Design decisions captured during review
These points were raised while replacing #6259 and are recorded here so future follow-ups can cite the settled boundaries instead of rediscovering them from review comments.
clientIdremains live provenance for same-daemon mutation checks, but it is not persisted into durable artifact records and legacy persistedclientIdvalues are dropped during restore/fork. The durable authorization boundary is the session/mutation boundary, not a restart-sensitive client socket id; otherwise a daemon restart or fork could lock a legitimate new client out of updating/deleting old artifact metadata.warningDetailswhile preserving the existing prosewarningsfield. The stable fields arecode,operation,artifactIds,durability,retryable, andmessage. This lets clients and operators distinguish durable, live-only, and unavailable state without parsing human text when live state changes but JSONL persistence cannot complete.warningDetails.Capability contract:
session_artifacts_persistenceis advertised only when the durable chat-recording sink is available. When chat recording is disabled, the daemon may still keep live artifact state best-effort, but clients must not treat artifact metadata as restart/fork-durable.Snapshot marker contract: snapshot-only tombstone and sticky markers now carry optional
markerArtifactsmetadata. This is not restored as live artifact state; it only preserves enough identity metadata for fork remap so a future URL/workspace/managed upsert in the fork computes the same normal stable ID. Older snapshots without marker metadata still fall back to the previous fork-derived ID, preserving backward compatibility.Reviewer Test Plan
How to verify
Review that ephemeral artifacts remain live-only, restorable artifact metadata survives JSONL rebuild/snapshot restore, explicit remove tombstones prevent deleted artifacts from coming back, restore/fork only replay artifact records attached to the active conversation branch, fork remap preserves metadata without leaking cross-session ownership, workspace-file status is metadata-only and marks stale/missing files correctly, and artifact add/remove calls enforce client ownership through
x-qwen-client-id.Evidence (Before & After)
Before: artifact state was daemon-memory-only and the PR contained only a design document. After: daemon sessions persist artifact metadata/events/snapshots, rebuild that metadata after daemon restart/session replay, keep durable delete restart-safe, filter abandoned branch artifact side records, and keep content retention outside this PR.
Tested on
Environment (optional)
Validated locally with
npx prettier --check packages/acp-bridge/src/sessionArtifacts.ts packages/acp-bridge/src/sessionArtifacts.test.ts packages/core/src/services/sessionService.ts packages/core/src/services/sessionService.test.ts packages/sdk-typescript/src/daemon/types.ts docs/design/daemon-session-artifacts/session-artifacts-persistence-v2-design.md,git diff --check,cd packages/acp-bridge && npx vitest run src/sessionArtifacts.test.ts,cd packages/core && npx vitest run src/services/sessionService.test.ts src/services/session-artifact-persistence.test.ts,cd packages/acp-bridge && npm run typecheck,cd packages/core && npm run typecheck, andcd packages/sdk-typescript && npm run typecheck.Risk & Scope
x-qwen-client-idso ownership can be enforced. Older sessions without artifact persistence records simply restore with an empty artifact list. When artifact persistence is available, existing clients that omitretentionnow default torestorable; clients that want transient UI-only entries should sendretention: "ephemeral"to avoid growing the session JSONL.Linked Issues
Follow-up to #5895. Supersedes #6259.
中文说明
What this PR does
这是 #6259 的替代 PR，保留相同净变更，并压成单个 commit 方便 review。
这个 PR 在 #5895 之后实现 V2 daemon session artifact metadata 持久化。现在会在 daemon 重启或 session replay 后恢复 restorable artifact metadata，把 durable tombstone 和 snapshot 写入 JSONL，在 load/fork 时重建 artifact 状态，remap fork 后的 artifact ownership，并保持 ephemeral artifact 仍然只存在于 live session。
本 PR 的持久化边界是 metadata-only：workspace 文件类 artifact 会记录 sha256/size/mtime 等状态 metadata 用于判断 stale，但这里不会保留、复制、配额管理或 GC artifact 内容。Content retention 不属于这个替代 PR 的范围。
Why it's needed
V1 artifact API 只有 live-session 状态：页面刷新且 daemon 还活着时能恢复，但 daemon 重启或历史 load 后不能恢复 artifact metadata。V2 提供稳定的 metadata restore 路径，同时把 content retention 从这个 base persistence change 中拆开。
Review 中确认的设计决策
以下决策来自替代 #6259 期间的 review 讨论，写在 PR 描述里方便后续引用，而不是从评论串里重新翻。
clientId只保留为同一 daemon 生命周期内的 live provenance 和 mutation 校验输入，不会写入 durable artifact record；restore/fork 时也会丢弃旧记录里的 legacyclientId。理由是 durable 权限边界应该是 session/mutation 边界，而不是重启后会变化的 client socket id，否则 daemon restart 或 fork 后合法的新 client 可能无法更新/删除旧 artifact metadata。warnings，并额外返回结构化warningDetails。字段包括code、operation、artifactIds、durability、retryable、message。理由是 live state 可能已经变化但 JSONL durable write 失败，客户端和排障人员必须能可靠区分 durable、live-only、unavailable 等状态，而不是解析人类文字。warningDetails。Capability contract：只有 durable chat-recording sink 可用时才 advertise
session_artifacts_persistence。当 chat recording disabled 时，daemon 仍然可以 best-effort 保留 live artifact state，但客户端不能把这当成 restart/fork-durable 的 artifact metadata persistence 承诺。Snapshot marker contract：snapshot-only tombstone/sticky marker 现在会携带可选的
markerArtifactsmetadata。这个字段不会作为 live artifact 恢复，只用于保留 fork remap 所需的 identity metadata，确保 fork 后未来同 URL/workspace/managed upsert 计算出同一个正常 stable ID。旧 snapshot 没有 marker metadata 时仍然走原来的 fork-derived fallback ID，保持向后兼容。Reviewer Test Plan
How to verify
请重点检查：ephemeral artifact 是否仍然只存在于 live session；restorable artifact metadata 是否能通过 JSONL rebuild/snapshot restore 恢复；显式删除的 tombstone 是否阻止旧 artifact 回来；restore/fork 是否只 replay 挂在 active conversation branch 上的 artifact records；fork remap 是否保留 metadata 且不泄露跨 session ownership；workspace 文件状态是否只记录 metadata 并正确标记 stale/missing；artifact add/remove 是否通过
x-qwen-client-id强制 client ownership。Evidence (Before & After)
Before：artifact state 只在 daemon 内存中，且原 PR 只有设计文档。After：daemon session 会持久化 artifact metadata/events/snapshots，daemon 重启或 session replay 后能重建这些 metadata，durable delete 保持 restart-safe，abandoned branch 的 artifact side records 不会泄漏，并且 content retention 仍不在本 PR 内。
Risk & Scope
x-qwen-client-id，以便强制 ownership。没有 artifact persistence record 的旧 session 会恢复为空 artifact list。另外，当 artifact persistence 可用时，未传retention的现有 client artifact 会默认restorable；如果只是临时 UI 状态，应显式传retention: "ephemeral"，避免 session JSONL 随轮次增长。Linked Issues
#5895 的后续实现。替代 #6259。