Skip to content

fix(claude): keep IDE-context envelopes out of first_message previews - #1265

Open
adammw wants to merge 8 commits into
kenn-io:mainfrom
adammw:fix/claude-ide-envelope-preview
Open

fix(claude): keep IDE-context envelopes out of first_message previews#1265
adammw wants to merge 8 commits into
kenn-io:mainfrom
adammw:fix/claude-ide-envelope-preview

Conversation

@adammw

@adammw adammw commented Jul 25, 2026

Copy link
Copy Markdown

Fixes #1238 (the previous PR did not fix the issue) for me.
I haven't evaluated if the internal/parser/claude.go changes are the best solution or if there's a way to combine the work from the previous change.

Claude Sonnet 5 explanation follows:

The VS Code extension often prepends a <ide_opened_file> or <ide_selection> wrapper directly onto a real prompt in the same user entry. PR #1252 promoted standalone envelopes to hidden system metadata, but this mixed case (envelope + real prompt in one message) falls outside that strict standalone match by design, so the raw markup stayed in first_message and the visible transcript.

Split a leading IDE-context envelope off from the rest of the message: the envelope becomes its own hidden system-metadata message (same ide_opened_file/ide_selection subtype as the standalone case), and the remaining real prompt becomes an ordinary user message. first_message and user turn counts are computed from the same firstMessageAndUserCount pass used before, so they now derive from the real prompt only.

Bumps the parser data version to 75 so existing rows re-parse.

@roborev-ci

roborev-ci Bot commented Jul 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (26eaed8)

Review identified one medium-severity issue affecting identity and provenance during Claude message splitting.

Medium

  • Duplicate SourceUUID across split messagesinternal/parser/claude.go:1110, internal/parser/claude.go:1136, internal/parser/claude.go:2156, internal/parser/claude.go:2180

    Both split messages receive the same SourceUUID. During the forced v75 reparse, source-anchored pins may move to the hidden envelope or duplicate across both rows, while Recall evidence treats duplicate source UUIDs as ambiguous and revokes provenance.

    Recommended fix: Preserve the original UUID only on the real prompt, and give the synthetic metadata row a distinct deterministic identity or no UUID. Add reparse tests covering pins and Recall evidence.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 4m26s

@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (15dbd40)

Code review found two medium-severity correctness issues; no security concerns were identified.

Medium

  • internal/db/recall_evidence_window_test.go:247 — The test inserts an envelope before otherwise unchanged messages, but the actual v74→v75 reparse changes the original row from <ide…> prompt to prompt. Because recall reconciliation recomputes the content digest, evidence spanning that row will still lose provenance_ok despite the distinct UUID.

    • Fix: Test the actual combined-to-split transformation and add narrowly scoped digest migration/normalization for the IDE-envelope rewrite, or explicitly treat revocation as expected.
  • internal/db/orphaned.go:578 — Ordinal fallback also applies when a non-empty source UUID is missing or ambiguous in the rebuilt database. Since reparsing can shift or remove rows, this may silently move a pin to an unrelated message at the former ordinal.

    • Fix: Use ordinal fallback only when the source row has an empty UUID; otherwise drop unresolved pins or validate them using stable content identity.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 6m43s

@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (58dabd8)

Code changes are generally sound, but one medium-severity pin-remapping issue should be fixed.

Medium

  • internal/db/orphaned.go:1220 — The ordinal fallback runs when a nonempty source_uuid resolves to zero or multiple destination rows. If the original source message disappeared and another message now occupies that ordinal, the pin can silently attach to unrelated content.

    Fix: Use ordinal fallback only for legacy NULL/empty UUIDs. For duplicate UUIDs, require the ordinal candidate to have the same UUID; otherwise drop the pin. Add coverage for a missing UUID with a reused ordinal.


Reviewers: 2 done | Synthesis: codex, 9s | Total: 5m34s

@mjacobs
mjacobs force-pushed the fix/claude-ide-envelope-preview branch from 58dabd8 to 5193f4a Compare August 1, 2026 03:32
@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (5193f4a)

Medium-severity correctness issue found in UUID-based metadata remapping.

Medium

  • internal/db/orphaned.go:1208 — UUID remapping verifies uniqueness only in the destination. If the old session contains duplicate source_uuid values but only one survives re-parsing, a pin on the removed duplicate may be incorrectly transferred to the surviving message instead of being dropped. Require the UUID to be unique in both old_db.messages and main.messages; otherwise use the guarded ordinal fallback.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 3m21s

@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (dbe4eba)

Code changes are generally sound, but one medium-severity parser preprocessing issue remains.

Medium

  • internal/parser/claude.go:1117, internal/parser/claude.go:2172 — The IDE envelope is split after preprocessClaudeUserText runs, causing the revealed remainder to bypass command and system-reminder preprocessing. For example, <command-name>/clear</command-name> after an IDE envelope may be stored as raw XML and incorrectly become first_message instead of normalizing to /clear and being excluded from previews.

    Fix: Split the IDE envelope before preprocessing, or preprocess the remainder and honor its skip result in both extraction paths.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 5m44s

@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (26c1417)

Review identified two medium-severity correctness issues in migration reconciliation and pin remapping.

Medium

  • internal/db/recall_evidence_window_test.go:249 — The new Recall test inserts an envelope before an unchanged prompt, but the real migration changes the prompt from <ide_…>…</ide_…> prompt to prompt. Because Recall’s canonical digest includes message content, reconciliation detects a mismatch and revokes provenance for evidence anchored to the affected prompt despite its stable UUID.

    • Fix: Test the actual combined-to-split transformation and add migration-aware digest reconciliation for removal of a recognized leading IDE envelope.
  • internal/db/orphaned.go:1235 — The duplicate-UUID ordinal fallback can misattach a pin after ordinal shifts. If messages A and B share a UUID, A at ordinal 1 is pinned, and reparsing removes A so B shifts from ordinal 2 to 1, the guard incorrectly accepts B because its UUID and new ordinal match.

    • Fix: For duplicate UUIDs, require stronger identity—such as matching content and role with unchanged multiplicity—before using the ordinal fallback; otherwise drop the pin.

Reviewers: 2 done | Synthesis: codex, 12s | Total: 5m4s

adammw and others added 7 commits August 1, 2026 13:37
The VS Code extension often prepends a <ide_opened_file> or
<ide_selection> wrapper directly onto a real prompt in the same user
entry. PR kenn-io#1252 promoted standalone envelopes to hidden system
metadata, but this mixed case (envelope + real prompt in one message)
falls outside that strict standalone match by design, so the raw
markup stayed in first_message and the visible transcript.

Split a leading IDE-context envelope off from the rest of the
message: the envelope becomes its own hidden system-metadata message
(same ide_opened_file/ide_selection subtype as the standalone case),
and the remaining real prompt becomes an ordinary user message.
first_message and user turn counts are computed from the same
firstMessageAndUserCount pass used before, so they now derive from
the real prompt only.

Bumps the parser data version to 75 so existing rows re-parse.
The IDE-envelope split gave both the synthetic hidden message and the
real prompt the same source UUID. Pins and Recall evidence resolve
messages by (session_id, source_uuid) and require the key to be unique:
the duplicate made Recall endpoint resolution ambiguous (permanently
revoking previously trusted evidence on the v75 reparse) and let pin
restoration attach a pin to both rows. The prompt now keeps the entry's
own uuid and the synthetic envelope row derives a distinct
'<uuid>:ide-context' identity (empty stays empty).

Full-rebuild pin migration in CopySessionMetadataFrom mapped pins across
DBs by (session_id, ordinal) alone, so inserting the hidden envelope row
shifted every later ordinal and moved pins onto unrelated rows. The copy
now prefers the source_uuid natural key when the old row has one that
resolves to exactly one fresh row, falling back to ordinal only for
pins the uuid pass could not resolve.

Records the mixed envelope-plus-prompt transcript shape in
docs/internal/session-format-sources.md per the provider format
provenance rule.

Adds parser coverage for the split identities, a metadata-copy test for
pins across the ordinal shift (including the legacy ordinal fallback),
and a Recall evidence test that the split shape remaps endpoints instead
of revoking provenance.
The cross-DB pin copy fell back to plain ordinal matching whenever a
nonempty source_uuid did not resolve to exactly one fresh row. When the
pinned message disappeared in the re-parse and another message took over
its ordinal, the pin silently attached to unrelated content.

The fallback now applies only to legacy pins with no source_uuid, or
when the row at the old ordinal carries the pin's own uuid (the
duplicate-uuid case, where the old coordinates pick which copy was
meant). A nonempty uuid with no matching fresh row drops the pin.

Adds coverage for a vanished uuid whose ordinal was reused and for a
duplicated uuid resolved by old ordinal.
The uuid pin-copy pass verified uniqueness only in the fresh DB. When
the old DB itself held duplicate source_uuids and the re-parse removed
all but one, a pin on a removed duplicate matched the lone survivor and
transferred to a message it was never on. The uuid now has to be unique
on both sides; old-side duplicates go through the guarded ordinal
fallback, which restores a pin only when the row at its old ordinal
carries the same uuid and drops it otherwise.

Adds coverage for a pin on a removed old-side duplicate (dropped) and
on the surviving duplicate (restored at its ordinal).
splitLeadingClaudeIDEEnvelope runs after preprocessClaudeUserText, so a
command envelope hidden behind an IDE-context wrapper bypassed command
and system-reminder preprocessing: the revealed remainder was stored as
raw command XML and could become first_message. The remainder now goes
through preprocessClaudeUserText in both extraction paths, normalizing
command XML (e.g. to /clear) and honoring the skip result for command
envelopes that cannot be normalized.

Covered by the already-pending v79 reparse in this change; no further
data version bump is needed.
@mjacobs
mjacobs force-pushed the fix/claude-ide-envelope-preview branch from 26c1417 to af011ba Compare August 1, 2026 17:40
@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (af011ba)

Code is generally sound, but one medium-severity pin-mapping issue should be fixed before merging.

Medium

  • internal/db/orphaned.go:1238 — UUID-less pins still fall back unconditionally by ordinal. After an IDE-envelope split, the old combined prompt’s ordinal belongs to the hidden envelope, so the pin can silently attach to metadata instead of the prompt.
    • Fix: Require matching role/content identity for UUID-less ordinal fallback, explicitly map the split shape to the following prompt, or drop the pin when identity cannot be established.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 4m23s

Legacy pins without a source_uuid still restored by bare ordinal in the
cross-DB copy. After an ordinal-shifting re-parse such as the IDE
envelope split, the old combined prompt's ordinal belongs to the hidden
envelope row, so a legacy pin silently attached to system metadata
instead of the prompt. The ordinal fallback now also requires the
candidate row to match the pinned row's role and content, both when the
old DB has an empty source_uuid and when it predates the column
entirely; pins whose identity cannot be established are dropped.

Adds coverage for a uuid-less pin on a split combined prompt.
@roborev-ci

roborev-ci Bot commented Aug 1, 2026

Copy link
Copy Markdown

roborev: Combined Review (ef49dd6)

Pin restoration is mostly hardened, but one medium-severity duplication bug remains for orphaned sessions.

Medium

  • internal/db/orphaned.go:1194copyPinnedMessagesForIDs copies pins before hardened restoration and joins on source_uuid without requiring uniqueness. A pinned message with a duplicated UUID can therefore attach its pin to every matching orphaned message, and the later INSERT OR IGNORE does not remove the incorrect pins. Data-version 80’s full resync may duplicate pins in orphaned sessions.
    • Suggested fix: Apply the same uniqueness and guarded fallback rules in copyPinnedMessagesForIDs, or copy orphan pins only by unchanged ordinals. Add a regression test covering an orphaned session with duplicate UUIDs.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 3m42s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Strip out / parse <ide_opened_file>/<ide_selection> in Claude Code via VSCode extension

2 participants