feat(terminal): preview images shown by codex viewed image output#262
Open
konard wants to merge 3 commits intoProverCoderAI:mainfrom
Open
feat(terminal): preview images shown by codex viewed image output#262konard wants to merge 3 commits intoProverCoderAI:mainfrom
konard wants to merge 3 commits intoProverCoderAI:mainfrom
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
Contributor
Author
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. |
Contributor
Author
🤖 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. |
Contributor
Author
✅ 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.
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 free
to 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