Skip to content

fix(artifacts): review follow-ups — id-cache invalidate after the queued write; honest dedupe scope - #320

Merged
itsdestin merged 1 commit into
masterfrom
fix/artifact-oom-review-followups
Aug 16, 2026
Merged

fix(artifacts): review follow-ups — id-cache invalidate after the queued write; honest dedupe scope#320
itsdestin merged 1 commit into
masterfrom
fix/artifact-oom-review-followups

Conversation

@itsdestin

Copy link
Copy Markdown
Owner

Follow-ups from an adversarial review of #318 (no behaviour change users would notice):

  • invalidateSidecarIdCache now runs after appendVersion resolves (the append is queued, so invalidating first left a small window where the watcher could rebuild from the old sidecar).
  • Drop unused applyGitTreatment import.
  • Correct the docs/type comment: the dedupe is per desktop session id — fully idempotent for native sessions, but a Claude Code --resume mints a fresh id, so a resumed CC conversation is re-recorded once per resume (bounded, crash-free; not growth-free). Follow-up captured in the workspace ROADMAP.

🤖 Generated with Claude Code

… queued write; state the dedupe's real scope

Adversarial review of #318 found:
- invalidateSidecarIdCache ran BEFORE the (now queued) append, so a watcher
  rebuild in between could read the old sidecar and leave a just-created
  artifact unmapped until the next TTL. Moved after the write commits.
- applyGitTreatment was an unused import.
- The 'replays are idempotent' claim was overstated: the dedupe key is
  (DESKTOP sessionId, toolUseId), and a Claude Code --resume mints a fresh
  desktop id (session-manager.ts randomUUID()), so a resumed CC conversation
  is re-recorded once per resume — bounded and crash-free, not growth-free.
  Native sessions keep their id and are fully idempotent. The type comment
  and docs/artifacts.md now say exactly that and name the follow-up.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@itsdestin
itsdestin merged commit a0c1228 into master Aug 16, 2026
4 checks passed
@itsdestin
itsdestin deleted the fix/artifact-oom-review-followups branch August 16, 2026 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant