Repository navigation
Conversation
pj991207
marked this pull request as draft
November 3, 2026 10:09
pj991207
marked this pull request as ready for review
November 3, 2026 10:24
pj991207
marked this pull request as draft
November 3, 2026 10:36
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.
Draft status
Draft pending agreement on the Windows session-path strategy. The reproduced failure is real, while root-dependent folder naming, the fixed descendant reserve and policy handling introduce storage-layout maintenance choices that need maintainer direction.
Problem
On Windows, a workspace-derived session name can fit the filesystem's component limit but make the complete default
openworker runrecord path exceed MAX_PATH. The CLI then fails while creating the run or spilling tool output.Change
Budget the generated workspace component against the absolute state root and run ID, counting UTF-16 code units and reserving room for current generated record/tool-output names. Keep the existing name when it fits and retain a workspace hash when shortening is required. If even a hashed component cannot fit, return a usage error advising a shorter explicit
--outpath. Explicit output paths keep their current behavior.Preserve explicit extended-length namespace paths and paths on a standard Python runtime with the Windows
LongPathsEnabledpolicy enabled. If the policy cannot be read, use the legacy budget. This follows Python's documented long-path support. Existing process policy caching and custom executable manifests can affect the platform's behavior.Reproduction
On native Windows with legacy path limits, select a state root/workspace that makes the generated run path 301 characters and spill path 394 characters on the baseline. Run a scripted headless session that spills output. The baseline exits 1; the patched case exits 0 with paths of 159 and 252 characters. Also exercise an explicit extended-namespace root, which must remain usable.
Focused path regressions:
Validation
On
5e3d726, a native Windows CLI reproduction exits 1. With the fix it exits 0, reducing its run directory from 301 to 159 characters and its generated spill path from 394 to 252. Coverage includes actual spill writes, distinct workspaces, non-BMP roots, fitting-name preservation and overlong-root errors. The selected CLI run passes 28 tests with three failures reproduced on unmodified main; tool-result tests pass 12/12. The reserve covers current generated records and does not guarantee arbitrary user-created nested paths.Additional adversarial review reproduced a regression in the initial patch with a valid extended namespace root. The follow-up fixes it and adds native filesystem coverage plus enabled/unreadable-policy cases. All eight targeted checks pass. The full local CLI file reports 31 passed, 7 existing PTY skips and three pre-existing path/JSON fixture failures addressed separately in the Windows fixture portability work.
Tested PR head:
097157535910aa28af601fdb8be63da973986a8e. At this documentation update,pytest,gui-unitandgui-e2eall report success for this head: CI run. Local results above are the previously recorded checks; this body update does not claim a new local test run.Evidence and limitations
No visible GUI change. Native CLI exit status, generated path lengths and actual spill writes supply the before/after evidence; screenshots are not applicable. The fixed reserve covers current generated descendants only. Enabled long-path policy was tested through the policy interface; the host had legacy policy, while the explicit extended namespace case was also verified with real filesystem writes. Root/policy-dependent storage naming and future reserve maintenance still require design agreement.
Related work
#835 separately repairs the pre-existing path/JSON test fixtures noted in the full CLI run. A shorter explicit
--outremains an available workaround while this storage-path design is discussed.