Skip to content

Resolve file-reference copy actions to absolute run paths - #30

Draft
RVZO6 wants to merge 4 commits into
SunkenInTime:mainfrom
RVZO6:fix/absolute-file-reference-paths
Draft

RVZO6 wants to merge 4 commits into
SunkenInTime:mainfrom
RVZO6:fix/absolute-file-reference-paths

Conversation

@RVZO6

@RVZO6 RVZO6 commented Oct 9, 2026 •

Copy link
Copy Markdown

Dependency and review diff

Depends on #28. This is a draft upstream PR because its context-menu asset comes from that refresh and does not exist in the old bundle. The upstream Files changed tab includes the parent until it merges; review only this file-path change here.

GitHub native stacks cannot represent this cross-fork dependency, and I do not have write access to create branches in the upstream repository. Once the parent merges, this branch can be rebased onto upstream main so its normal PR diff is focused.

Bug and fix

File references such as outputs/file.txt can be presented as “Copy path” even though they are relative and ambiguous outside the run's working directory. Resolve against the explicit run cwd, falling back to the thread cwd; preserve POSIX, Windows, UNC, and file-URL handling. When an absolute path cannot be determined, label the action “Copy relative path” rather than inventing a root directory. Copy contents reads the same resolved path.

The minimal context-menu adaptation is reproducible through the pinned upstream patch script. It does not modify the native companion or read local user files during the tests.

Validation

  • pnpm test and the existing isolated Zen lifecycle test pass on the refresh-plus-path branch.
  • Extend the existing feature-parity test with path cases and execution of the actual packaged Copy path/Copy contents callbacks, including explicit cwd, thread fallback, and unavailable cwd. No separate test file or suite is added.
  • POSIX symlink-parent traversal is preserved until the host resolves it, and slash-prefixed Windows drive paths are recognized. The maintained tests read a disposable POSIX symlink fixture and execute the packaged callbacks for both regression cases.
  • These checks validate the menu callback and path resolver directly; an authenticated sidebar file-menu interaction has not been replayed.

Generated by GPT 6.1 Sol in Codex, using T3 Code.

RetriggerConfidence Score: 5/5

The current code addresses both previously reported copy-action regressions. No outstanding findings require changes before merging.

Summary

The PR resolves file-reference copy actions against the run's working directory, with a thread fallback. The latest changes address both previous findings.

  • File copy actions use paths from the run’s working directory.

Both previous findings are fully addressed in the current code. No new actionable issues were identified. Tests were inspected, not rerun.

Reviews (2) · Last reviewed commit: "Preserve host path traversal and accept ..." · Reviewed by Greptile

@coderabbitai

coderabbitai Bot commented Oct 9, 2026

Copy link
Copy Markdown

Walkthrough

Warning

Review details and warnings were omitted to fit the comment limit.

Comment thread extension/codex-sidepanel/firefox-file-path.js Outdated
Comment thread extension/codex-sidepanel/firefox-file-path.js
@RVZO6

RVZO6 commented Oct 9, 2026

Copy link
Copy Markdown
Author

Review fixes pushed in 2e52118.

Both path-handling findings are addressed and their threads have replies/resolutions. Full npm test passes, including a disposable POSIX symlink-parent fixture and execution of the packaged Copy path/Copy contents callbacks for preserved parent traversal and /C:/ drive paths. The focused comparison link in the description now includes this commit. This PR remains draft because #28 must merge first; the parent asset refresh is still included in its upstream diff. No authenticated sidebar-menu replay is claimed.

The updated changes are ready for another review; the latest review checks are still running.

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