Repository navigation
test(opencode): make the write-permission and SDK-parity tests portable - #182
Merged
Merged
Conversation
`tool.write > sets file permissions when writing sensitive data` hardcoded `0o644`, which is only the mode the tool produces under umask 022. The write tool never chmods — it calls `fs.writeWithDirs` — so the mode is the 0o666 creation default masked by the process umask. On a machine with the equally common umask 002 the file lands at 0o664 and the test fails. Derive the expectation from `process.umask()` so it still pins the tool to the platform default without encoding the CI runners' umask. `HttpApi SDK > matches generated SDK instance read routes` drives sixteen read routes in one test, several of which shell out to git/ripgrep or probe the formatter and LSP. It runs in ~5s unloaded but hit the 30s harness default when the rest of the suite was competing for the machine. Pass an explicit 120s from the file's `serverPathParity` helper, which keeps every upstream call site byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Ran the full local suite (
bun turbo test,bun turbo typecheck,oxlint, app Playwright e2e,packages/client check:generated,packages/opencode test:httpapi, storybook build) and fixed the two failures it surfaced. Both were environment assumptions in the tests rather than product bugs.tool.write > sets file permissions when writing sensitive dataAsserted
stats.mode & 0o777 === 0o644. The write tool never chmods — it callsfs.writeWithDirs— so the mode is just the0o666creation default masked by the process umask.0o644silently encodes the umask 022 that CI runners happen to use; on a developer machine with the equally common umask 002 the file lands at0o664and the test fails.Now derives the expectation from
process.umask(). It still pins the tool to the platform default, so a straychmodwould break it, but it no longer hardcodes one runner's umask. Verified passing under both umask 002 and umask 022.HttpApi SDK > matches generated SDK instance read routesTimed out at exactly the 30s harness default during a full-suite run. It drives sixteen read routes in one test, several of which shell out to git/ripgrep or probe the formatter and LSP — ~5s unloaded, but well over 30s when the rest of the suite is competing for the machine.
Passes an explicit
120_000from that file's ownserverPathParityhelper. Applying it in the helper rather than at the call site keeps every upstream call site byte-identical, so the change is a 5-line diff instead of a ~55-line reindent on an upstream file. Headroom only — a genuine hang still exceeds the bound.DIVERGENCES.mdgets a new row for the umask change and an extension of the existing slow-runner-headroom row for the timeout.Verification
bun turbo testopencode3197 pass,core1085,app619+31,session-ui75,ui14)bun turbo typecheckbun --cwd packages/app test:e2e:localpackages/client bun run check:generatedpackages/opencode bun run test:httpapibun --cwd packages/storybook buildbun run lintmain— 4740 warnings + 1 pre-existing error (see below)Pre-existing lint error, not fixed here
oxlintreports one error onmain, untouched by this PR:packages/session-ui/src/v2/components/prompt-input/index.tsx:159writes a zero-width space into a Tailwind arbitrary value ascontent-['\200B']inside a JSX class string, where the backslash-200 is parsed as a JS octal escape. The class that actually reaches the DOM therefore holds U+0080 followed byB, not the four literal characters Tailwind generated its rule from, so the selector never matches and the placeholder is dead.It came in with the upstream v1.18.4 sync and lint is not wired into CI, so fixing it belongs in a separate change: there is no escaping that satisfies both the JS string literal and Tailwind's raw-source scanner at once, so the fix needs a different approach plus visual verification and its own
DIVERGENCES.mdrow.🤖 Generated with Claude Code