Uh oh!
There was an error while loading. Please reload this page.
feat(terminal): preview images shown by codex viewed image output - #262
Merged
Conversation
Adding .gitkeep for PR creation (default mode). This file will be removed when the task is complete. Issue: ProverCoderAI#261
Issue 261 reports that the inline terminal image preview added in PR 241 does not trigger for the "Viewed Image" output that the Codex CLI prints as a tree pointer line followed by a relative path: • Viewed Image └ app/docs/screenshots/issue-237/proof/pr238-proof-...png The detector previously only matched absolute paths starting with "/", and the api fetch validator rejected anything that was not absolute. This change adds a second detection pass anchored on the tree pointer glyphs `└` and `├` and a relative path with a supported image extension. Detection deduplicates against the existing absolute-path pass so plain absolute paths preceded by a pointer are not double-counted. The api validator `planTerminalImageFetch` now accepts an optional `baseDir` and resolves bare relative paths against it. Terminal sessions remember the project's target dir (e.g. `/home/dev/app`) and pass it as the base directory when fetching an image, so the relative path printed by codex resolves to the same file the user is looking at. All previous safety checks still apply: the base dir itself must be absolute and free of traversal/control characters, and the joined path is re-validated for traversal segments and unsafe characters before any docker exec runs. Tests cover the detection and validator changes including the issue 261 reproduction, alternative `├` pointer, multiple consecutive viewed images, no-base-dir backwards compatibility, and unsafe-base rejection. Refs ProverCoderAI#261
konard
marked this pull request as ready for review
May 9, 2026 17:08
konard
commented
May 9, 2026
ContributorAuthor
Working session summaryPR #262 is updated and ready for review: #262 Summary of what shipped:
This summary was automatically extracted from the AI working session output. |
konard
commented
May 9, 2026
ContributorAuthor
🤖 Solution Draft LogThis log file contains the complete execution trace of the AI solution draft process. 💰 Cost: $4.109216📊 Context and tokens usage:Claude Opus 4.7: (2 sub-sessions)
Total: (1.9K new + 151.9K cache writes + 4.6M cache reads) input tokens, 33.8K output tokens, $4.109216 cost 🤖 Models used:
📎 Log file uploaded as Gist (2144KB)Now working session is ended, feel free to review and add any feedback on the solution draft. |
konard
commented
May 9, 2026
ContributorAuthor
✅ Ready to mergeThis pull request is now ready to be merged:
Monitored by hive-mind with --auto-restart-until-mergeable flag |
This reverts commit 80e4a5d.
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#261.
Summary
Enables the inline terminal image preview (added in #241, extended in #253) to also fire when the Codex CLI prints its
Viewed Imageblock, which uses a tree pointer line followed by a relative path:Before this change the detector only matched paths starting with
/, and the API validator rejected anything that wasn't absolute, so these images were not previewed.What changed
Frontend —
packages/app/src/web/terminal-image-paths.ts└and├and a relative path with a supported image extension.startIndex, so an absolute path that happens to follow a pointer is reported once.Backend —
packages/api/src/services/terminal-image-fetch-core.tsplanTerminalImageFetchaccepts an optional{ baseDir }and resolves bare relative paths against it. The base dir itself must be absolute and free of traversal/control characters; the joined path is re-validated through the existing safety checks (traversal, control chars, supported extensions) before anydocker execruns.Backend wiring —
packages/api/src/services/terminal-sessions.tstargetDir(e.g./home/dev/app) and pass it as thebaseDirwhen reading the image. This way a relative path likeapp/docs/...printed inside a session whosecwdis/home/devresolves to the same file the user is looking at.Reproduction
The new tests reproduce the issue end-to-end:
packages/app/tests/docker-git/terminal-image-paths.test.tsdetects relative image paths under a tree pointer (issue 261 viewed image format)detects multiple viewed images across separate linesdetects relative image paths under the alternative branch tree pointerdoes not duplicate absolute paths preceded by a tree pointerignores tree pointers that do not precede an imagepackages/api/tests/terminal-image-fetch-core.test.tsresolves a relative path against the provided absolute base directoryignores trailing slashes on the base directory when resolving a relative pathstill rejects relative paths when no base directory is suppliedrejects relative paths when the supplied base directory is not absoluterejects an unsafe base directory containing traversal segmentsrejects relative paths that contain traversal segments after resolutionTest plan
bun run --cwd packages/api typecheckbun run --cwd packages/app typecheckbun run --cwd packages/api vitest run tests/terminal-image-fetch-core.test.ts— 17/17 passbun run --cwd packages/app vitest run tests/docker-git/terminal-image-paths.test.ts— 18/18 passbun run --cwd packages/app vitest run— 274/274 passbun run --cwd packages/api vitest run— 90/91 pass (one pre-existing failure intests/projects.test.tsrequires a local Docker daemon and is unrelated to this change)Safety
The path that is finally handed to
docker execis still subject to all of the previously existing checks:./..segments (raw or percent-encoded)png,jpg,jpeg,gif,webp)Additional checks introduced here:
baseDiris supplied it must itself be absolute and free of traversal/control charactersbaseDircontinue to fail with the same message as before