Repository navigation
Conversation
A cache file written with control bytes (e.g. NUL) made Project.resolve adopt that value as the project id. The id is also a path segment, so every later start failed and the TUI could not offer model selection, while the corrupt value stayed in the project table and regenerated on each resolve. Treat a cached value carrying control characters as absent so resolution falls through to the normal sources.
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: |
|
Noting the automated This change targets Net effect: the linked-issue field stays empty on a |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks right to me. I reproduced #53991 on the base from source (serve plus run --server, because standalone can't start from source): a git repo whose .git/opencode holds 40 NUL bytes fails with UnexpectedStatus: 500. With this change the project resolves and the run continues. test/project.test.ts passes (23/23), the new corrupt-cache test fails on the base, and the packages/core typecheck is clean.
Nothing blocking. On v2 nothing ever writes the cache file, so skipping a bad value on each start is enough; no repair step is needed. The two added code comments aren't needed for a fix this small and can be dropped so the diff stays minimal.
A cache file written with control bytes (e.g. NUL) made Project.resolve adopt that value as the project id. The id is also a path segment, so every later start failed and the TUI could not offer model selection, while the corrupt value stayed in the project table and regenerated on each resolve. Treat a cached value carrying control characters as absent so resolution falls through to the normal sources.
|
Thanks for reproducing this independently and for confirming the resolution path. Both added comments are dropped as suggested — the change is now just the Re-verified after the edit: the two new tests and the rest of Noted on the repair step — if nothing on |
|
Closing this to stay within GitHub's 3-open-pull-request limit for contributors without write access, which currently blocks every new pull request here. The branch remains on my fork, so I can reopen it on request. |
Issue for this PR
Closes #53991
Type of change
What does this PR do?
A git repo whose
.git/opencodecache file holds control bytes (the report shows 40 NUL bytes) resolved to that value as its project id.Project.cached()read the file, calledtrim(), and handed whatever survived toID.make().trim()only strips whitespace, so NUL bytes passed through untouched and became the id.The id is not just an opaque key: it is used as a path segment for the shell output directory (
<data>/shell/<id>). A NUL byte in a path makes every later filesystem call fail withpath ... without null bytes, which is theUnexpectedStatus: 500in the report, and the TUI cannot offer model selection for that project. Because the cached value was treated as authoritative, the same broken id was re-derived on every resolve — so the stale row stayed inprojectand the failure repeated, as @layermedya noted.This change treats a cached value carrying control characters as absent, so resolution falls through to the sources that already work — @layermedya confirmed renaming
.gitaway resolves the directory to a valid id. The cache stays a convenience rather than an authority: a corrupt file no longer becomes the id.git logon the region showscached()was added as a fast path in front of the real resolution sources; rejecting a value that cannot be a valid id keeps that contract, and a usable cached value is still used unchanged.How did you verify your code works?
Two tests in
packages/core/test/project.test.ts, beside the existing "returns previous cached id from common dir":keeps a usable cached id— a normal cached value is still used as bothpreviousandid, so the fast path is unchanged.ignores a corrupt cached id— a cache file of 40 NUL bytes:previousisundefined,idresolves to the root commit id from the normal source, and the id contains no NUL byte.Red/green on
v2at1a8988efe0, pinned bun 1.4.2:ignores a corrupt cached idfails becauseReceivedis 40 NUL bytes instead ofundefinedFull
packages/coresuite: 6519 pass / 49 fail after, versus 6518 pass / 50 fail before — the single delta is this new test. The 49 remaining failures are pre-existing on this machine and unrelated (pty,Session.shell, real-shell and ripgrep-dependent tests fail in this sandbox).Also clean:
tsgo -b tsconfig.json tsconfig.tests.jsonforpackages/core, and oxlint on both edited files.Screenshots / recordings
If this is a UI change, please include a screenshot or recording.
Checklist