[luv-296] fix: populate Transcript field across all harnesses#294
[luv-296] fix: populate Transcript field across all harnesses#294NiveditJain merged 2 commits intomainfrom
Conversation
…l harnesses Only Claude reliably puts transcript_path on hook stdin; Codex/Copilot/ Cursor don't, the OpenCode and Pi shims don't forward one, and Gemini coverage is uneven across versions. The dashboard rendered "Transcript: —" for nearly every non-Claude row even though every harness except OpenCode has the transcript on disk and the repo already had per-CLI find*Transcript helpers in lib/. New src/hooks/resolve-transcript-path.ts mirrors the existing resolve-permission-mode.ts dispatch pattern: trust stdin first, fall back to the per-CLI helper when sessionId is known. OpenCode (DB-backed) synthesizes opencode-db://<sessionId> so the field is non-empty and distinguishable from a genuine miss; both detail panels render a muted "(stored in opencode DB)" suffix for that scheme. The handler-side fallback covers OpenCode and Pi without touching their shims (no duplicated disk-walk). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCentralizes transcript-path resolution via a new ChangesTranscript Path Resolution & OpenCode Marker Support
Sequence DiagramsequenceDiagram
participant Handler as Handler
participant ResolveFunc as resolveTranscriptPath
participant Stdin as Stdin Input
participant CliHelper as CLI Helper (e.g., findCopilotTranscript)
participant UI as UI Components
Handler->>ResolveFunc: resolveTranscriptPath(cli, parsed, sessionId)
ResolveFunc->>Stdin: Check parsed.transcript_path
alt Stdin provides path (string)
Stdin-->>ResolveFunc: Return path verbatim
else Stdin missing or non-string
ResolveFunc->>ResolveFunc: Validate sessionId
alt sessionId missing/invalid
ResolveFunc-->>Handler: Return undefined
else sessionId present
alt integration == "opencode"
ResolveFunc-->>Handler: Return opencode-db://<sessionId>
else integration has disk helper
ResolveFunc->>CliHelper: find*Transcript(sessionId)
CliHelper-->>ResolveFunc: Return path or null
ResolveFunc-->>Handler: Return discovered path or undefined
end
end
end
Handler->>UI: Provide session.transcriptPath
UI->>UI: Check for "opencode-db://" prefix
alt Prefix present
UI-->>UI: Append "(stored in opencode DB)" note
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/resolve-transcript-path.ts`:
- Around line 46-48: The code currently casts parsed.transcript_path to string
and returns it without runtime validation; change this by validating types at
runtime: check that parsed.transcript_path exists and typeof
parsed.transcript_path === "string" (and optionally non-empty after trim) before
assigning/returning stdinPath, and likewise ensure sessionId is a string before
using it (do not rely on TypeScript cast). Update the logic around the stdinPath
variable and the sessionId check in resolve-transcript-path.ts to gate returns
on these runtime checks and handle non-string values by returning undefined or
normalizing to a safe string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 576b5c5e-3b82-439a-a022-ee7bc86340b8
📒 Files selected for processing (6)
CHANGELOG.md__tests__/hooks/resolve-transcript-path.test.tsapp/components/session-hooks-panel.tsxapp/policies/hooks-client.tsxsrc/hooks/handler.tssrc/hooks/resolve-transcript-path.ts
Per CodeRabbit review on PR #294: parsed.transcript_path comes from raw JSON, so a non-string value (e.g. 42, null, an object) would silently propagate to the activity record and break downstream string operations (item.transcriptPath?.startsWith("opencode-db://") would throw on a non-string-or-nullish value). Tighten the entry guard from `as string | undefined` cast to a runtime typeof check, and reject empty-string sessionIds explicitly so they short-circuit before dispatching to per-CLI find*Transcript helpers. Adds 3 regression tests covering non-string transcript_path passthrough, empty-string sessionId, and non-string sessionId. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
—for nearly every non-Claude row because only Claude reliably putstranscript_pathon the hook stdin. Codex / Copilot / Cursor don't include one, the OpenCode + Pi shims don't forward one, and Gemini coverage is uneven across versions.src/hooks/resolve-transcript-path.tsmirrors the existingresolve-permission-mode.tsdispatch pattern: trust stdin first, fall back to the per-CLIfind*Transcript(sessionId)helper inlib/<cli>-sessions.ts. Runtimetypeofguards on bothparsed.transcript_pathandsessionIdso a malformed JSON payload can't propagate a non-string value into downstream string operations. OpenCode (transcripts in SQLite, no on-disk file) gets a syntheticopencode-db://<sessionId>marker so the field is non-empty and distinguishable from a genuine miss; both detail panels render a muted(stored in opencode DB)suffix when the value carries that scheme.discoverPiSessionId).—~/.codex/sessions/<YYYY>/<MM>/<DD>/...—~/.copilot/session-state/<id>/events.jsonl—~/.cursor/projects/.../<id>.jsonl—~/.pi/agent/sessions/.../<id>.jsonl~/.gemini/tmp/.../<id>.json—opencode-db://<id>+ "(stored in opencode DB)"Test plan
bun run test:run— 1483/1483 unit tests pass (was 1457; +26 new in__tests__/hooks/resolve-transcript-path.test.tscovering stdin passthrough × 7 CLIs, missing-sessionId × 7, per-CLI fallback dispatch × 7, stdin-precedence-beats-fallback, plus 3 runtime-type-guard cases)bun run test:e2e— 290/290 e2e tests passbun run build— clean Next.js + dist buildbun run lint— only the pre-existing unrelated<img>warning inapp/components/log-viewer/tool-input-output.tsx/policiesactivity tab, expand a Copilot row, confirmTranscript: <real path>appears instead ofTranscript: —🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Tests
Documentation