Repository navigation
fix: re-enable test and typecheck CI on integration-v2 - #420
Merged
Merged
Conversation
…nly SDK pack check
|
Unit is red, so this does not leave the test workflow green yet.
The Put ripgrep 15.1.0 on |
…Ripgrep and search-tool tests. The fork's workflow runs on GitHub's `ubuntu-latest`, which has no `rg`; upstream uses Blacksmith runners. Without `rg`, the code tries to download ripgrep from GitHub, and the core test harness blocks outside network requests, so the tests fail with "test attempted network request". I reproduced this locally by hiding `rg` from PATH: test/ripgrep.test.ts went 0 pass / 11 fail with the same error. With `rg` present, ripgrep.test.ts and tool-search.test.ts pass (27/27). Fix: the unit job's apt step in `.github/workflows/test.yml` now installs `ffmpeg ripgrep` (renamed "Install ffmpeg and ripgrep", with a short comment). The workflow YAML parses, and actionlint reports only an info-level note that was already there. e2e (not fixed, no code change): One test fails on all three attempts: `e2e/regression/timeline-history.spec.ts` › "mounts every part once and in order while paging to the start". It is an upstream spec that this PR doesn't touch. It fails at the poll after a scroll step (line 114). The CI video and screenshot show the timeline reaching the very top of the session, then not changing until the 10s poll times out. So some timeline part was never mounted earlier in the scroll, and the trace doesn't record which one. Locally the test passes against the production build in local time and with TZ=UTC. It also passes under 4×, 6× and 8× CPU throttling (repeated runs). At 12× it fails at the same line, but there a single slow DOM read times out, which is a different mechanism from CI. I also checked one guess: that the test's 100ms "settled" check fires before an older page renders. Instrumented runs showed the page always rendered first, so that's ruled out locally. The only confirmed difference is the runner: fork CI uses GitHub-hosted `ubuntu-latest`, upstream uses Blacksmith. This PR only changes workflow branch names, the service-smoke state path and stdin EPIPE handling in util; nothing in the app timeline. So I made no speculative change to the test or the app. The e2e failure may be a real virtualizer issue that only shows on slower runners, or a runner-specific test problem. Diagnosing it needs CI-side data, such as the missing part IDs from the GitHub runner. `mobile-session.spec.ts` was flaky once in the same run and passed on retry
… PowerShell test in `packages/core/test/tool-shell.test.ts` ("PowerShell legacy: ordinary syntax reuses approvals: Write-Output "$(Write-Output hello)""). The previous round's ripgrep failures are gone. Cause: the first time pwsh starts on a machine it can take close to 5s, and the GitHub runner always starts it cold. These tests had two 5s limits: Bun's default per-test timeout and the `Effect.timeout(Duration.seconds(5))` in the `runPermissionCommand` helper. So the cold start used up the budget and the command was killed with SIGTERM. Warm pwsh takes about 0.13s. I hit the same failure locally on a cold first run (failed at 5118ms); later runs passed. To reproduce it reliably, I pointed SHELL_SCAN_PWSH at a wrapper that waits 6s before starting the real pwsh: the original file fails ("timed out after 5000ms") and the fixed file passes. Fix (test file only): the PowerShell tests in both loops now pass `{ timeout: 15_000 }`, like other tests in this file, and the helper's limit went from 5s to 10s. Raising only the test timeout still failed locally, because the helper cut it off at about 5s. Most of the diff is Prettier re-indenting the two PowerShell test calls; I put back three unrelated lines Prettier had also changed. Checks: `bun test test/tool-shell.test.ts` gives 108 pass, 0 fail, and `bun typecheck` in packages/core passes. I did not run the whole repo test suite
This was referenced Oct 5, 2026
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
1 participant
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.
Intent
Captain's ask: "Queue a fix to re-enable shuvcode test + typecheck CI".
Context: in the fork https://github.com/shuv1337/shuvcode the GitHub Actions workflows
test(.github/workflows/test.yml) andtypecheck(.github/workflows/typecheck.yml) are disabled manually. Their last runs failed (test: 2026-08-17 on a PR branch; the run logs have expired). As a result a PR showing "green" only means Socket Security passed, and merges rely on local tests alone. The goal is for both workflows to pass on the default branchintegration-v2and on PRs against it, so they can be turned back on and mean something.What Changed
testandtypecheckworkflows now run on pushes tointegration-v2instead of upstream'sv2. Thetestworkflow's concurrency group andE2E_ENABLEDguard also checkintegration-v2now. The upstream "Verify packed workerd SDK" step is removed, with a comment explaining why: the fork's@opencode/coredepends on private, source-only@shuvcode/*plugins that can't be packed.packages/cli/script/service-smoke.tsnow reads the service registration fromstate/shuvcodeand the failure log fromdata/shuvcode/log, which match the fork'sGlobal.appdirectories. It used to read fromopencode.packages/util/src/cross-spawn-spawner.tsadds anerrorlistener on the child's stdin. It ignores an EPIPE that arrives late, after a child exits without reading all of its input, so the run reports the child's exit code instead of crashing on an uncaught error. Any other error is still thrown if no other handler is attached. A newAppProcesstest inpackages/core/test/process/process.test.tscovers this: the child closes stdin and exits with code 3, and the test expects exit code 3.Risk Assessment
✅ Low: The change is small. It retargets the CI branch from v2 to integration-v2 and fixes the smoke-test paths to use the shuvcode XDG app name. The stdin EPIPE fix now defers to the active NodeSink: it returns while another error listener is attached and rethrows only when it is the sole listener, which matches the prior unhandled-error behavior. The new regression test checks observable behavior, the exit code.
Testing
I ran the two compiled-service smoke tests the way thetestworkflow does, with the Bun and Node builds, and both passed. I also ran the old smoke script against the same binary to show it failed before the path change. I ran the stdin regression test with and without the spawner fix and ran the wholeprocess.test.tsfile. All of these passed. I checked the workflow trigger changes by parsing the YAML files only. GitHub Actions has not run them, so that scenario is marked untested. The first smoke run failed only because my shell'sOPENCODE_CONFIG_DIRpointed at the user's personal shuvcode config, which needs a token file that doesn't exist in the smoke test's temp home. GitHub runners don't set that variable, so I unset it and re-ran. Two things couldn't be run here: the real GitHub Actions runs, which belong to the CI phase, and the case where a non-EPIPE stdin error arrives while the stream is still writing. I removed the build output I created (packages/cli/dist,dist-nodeand.cache). This change has no UI, so there are no screenshots.Evidence: Bun compiled service lifecycle smoke (pass)
Evidence: Node build service lifecycle smoke (pass)
Evidence: Base-commit smoke script against the same binary (fails: no registration under state/opencode)
Evidence: Stdin regression test without the spawner fix (uncaught EPIPE)
Evidence: Stdin regression test with the fix (pass)
Evidence: Full process.test.ts run
Evidence: Parsed workflow triggers and steps
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
packages/util/src/cross-spawn-spawner.ts:234- The newproc.stdin.on("error")listener is registered beforeNodeSink.fromWritableattaches its own listener, and it rethrows every non-EPIPE error. EventEmitter calls listeners in registration order, so any non-EPIPE stdin error that happens while the sink is still writing now throws out ofemitfirst. The sink'sonErrornever runs, so the error that used to become a typedPlatformError(fromWritable(stdin)) now becomes an uncaught exception that can crash the process. Example: anERR_STREAM_WRITE_AFTER_ENDorERR_STREAM_DESTROYEDerror during an active stdin stream, or a WindowsEOFpipe error. Fix: keep the listener from preempting the sink. Make it ignore EPIPE without rethrowing anything, or rethrow only when it is the sole remainingerrorlistener (proc.stdin.listenerCount("error") === 1). Either way the sink still maps errors that happen while it is attached..github/workflows/test.yml:121- The 'Verify packed workerd SDK' step is removed rather than fixed. The comment explains why: the fork publishes only CLI packages, and @opencode/core depends on source-only @shuvcode/* workspace plugins (confirmed in packages/core/package.json). This is a deliberate tradeoff that matches the goal of getting the workflows green. It does mean CI no longer checks @opencode/sdk packability.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
cd packages/cli && bun run script/build.ts --single --skip-install && env -u OPENCODE_CONFIG_DIR bun run script/service-smoke.ts(exit 0)cd packages/cli && bun run script/build-node.ts --single --skip-install --outdir=dist/node && env -u OPENCODE_CONFIG_DIR bun run script/service-smoke.ts --node(exit 0)Ran the base-commitservice-smoke.ts(readsstate/opencode) against the same compiled binary: it fails with 'Compiled service did not publish registration'cd packages/core && bun test test/process/process.test.ts: 28 passPut the base-commitcross-spawn-spawner.tsback temporarily and ranbun test test/process/process.test.ts -t "exits before reading stdin": fails with an uncaught EPIPE. Restored the fix: passesParsed.github/workflows/test.ymlandtypecheck.ymlwithBun.YAML.parseand checked the triggers, the concurrency group, the E2E gate and whether the SDK pack step is present✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Fork CI notes
Verify packed workerd SDK(bun turbo verify:package --filter=@opencode/sdk) is dropped fromtest.yml. The fork publishes only CLI packages, and its@opencode/coredepends on private, source-only@shuvcode/*plugins that have no packable build.ubuntu-latest): the unit job now installsripgrep. The PowerShell shell-tool tests get a longer timeout because pwsh starts cold on every GitHub runner.e2e/regression/timeline-history.spec.ts› "mounts every part once and in order while paging to the start" is sensitive to CPU starvation. In one PR run it failed all 3 attempts; the next run passed. Locally, 9 of 10 runs fail with 5 workers on 4 CPUs, and all 6 pass with 2 workers. When an older page lands under load, the viewport sometimes isn't kept anchored and skips rows. This is upstream app code that this PR doesn't touch.