Repository navigation
test: keep the opener test off the host; typecheck the GUI's Node side - #6
Merged
Merged
Conversation
Three pre-existing problems kept changes from being verified automatically: - gui/tsconfig.json did not allow .ts import extensions, so gui/src/main.ts failed with TS5097, and the root config only covered src/, so the GUI was never typechecked. The GUI config now allows them, the root config also covers the GUI's Node side (gui/*.ts), and `pnpm typecheck` runs both projects. - "openTaskBody prefers noto via override without EDITOR" spawned the real macOS `open`, which raised `spawn open ENOENT` after the test ended on Linux. openTaskBody takes an optional launcher for platform openers, and the test injects a recorder that asserts the exact command, argv, options and unref. - Add .github/workflows/ci.yml: install with a frozen lockfile, then typecheck and test on ubuntu-latest with Node 22. Default behaviour of the CLI, GUI and file model is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D7P3PhbidfM5fMi1uyWFk5
main now carries #5, which fixes the same three problems. Keep its CI workflow, packageManager pin, verify script and the launchDetached error listener as they are. On top of that, keep what this branch adds: - launchDetached takes the spawn function, and openTaskBody passes opts.spawn when a test injects one, so the opener test never starts a real process on any host. The recorder also asserts the error listener that keeps a missing opener from crashing the process. - The root config still typechecks the GUI's Node side (gui/*.ts). - launchDetached moves above openTaskBody's doc comment, which the insertion on main had detached from the function. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D7P3PhbidfM5fMi1uyWFk5
lr00rl
changed the title
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
2 participants
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.
This started as a fix for three problems that kept holt changes from being verified automatically. While it was open, #5 merged to
mainand fixed most of the same ground: thegui/tsconfig.jsonflag, the two-projecttypecheckscript, CI runningpnpm verifywith pnpm 11.18.0 and Node 22, and alaunchDetachedhelper whose'error'listener stops a missing opener from crashing the CLI or the GUI server.I merged
maininto this branch and kept all of that as is. Two gaps are left, and this PR now closes them. The defaults of the CLI, the GUI and the file model are unchanged.1. The opener test still starts a real process on the host
After holt#5, "openTaskBody prefers noto via override without EDITOR" passes on Linux because the launch no longer crashes, but it still starts a real
openprocess. On Linux that process fails asynchronously withENOENT. On macOS it runsopen -a <temp dir>/Noto.app <task>. So the test's behaviour still depends on the host.OpenTaskBodyOptionsalready has test overrides (notoApp,platform,home). This PR addsspawn?: SpawnDetached.launchDetachednow takes the spawn function as an argument, andopenTaskBodypasses itopts.spawn ?? spawn, which covers every detached launch, including the non-waiting$EDITORpath. The CLI and GUI callers do not set it.The test injects a recorder and checks more than before. It still checks
path,viaandopened, and it now also asserts that exactly one launch happened:open, args['-a', <Noto.app>, <task path>]{ detached: true, stdio: 'ignore' }'error'listener attachedunref()callThe
'error'listener is the protection holt#5 added. The test now guards it on every host, where before the protection was exercised only by a realENOENTon Linux. I confirmed the test fails on each of these changes: removingchild.on('error', …), changing the argv, or removingunref().2. The GUI's Node side is not typechecked
pnpm typecheckrunstsconfig.json, which coverssrc/**, andgui/tsconfig.json, which covers the browser bundlegui/src/**. The Node-side GUI files (gui/api.ts,config.ts,dev.ts,vite.config.tsand their tests) are in neither project. The rootincludenow addsgui/*.ts. These files typecheck with no source changes.Also
On
main,launchDetachedsits betweenopenTaskBody's doc comment and the function, so the doc comment no longer attaches toopenTaskBody. This PR rewriteslaunchDetachedanyway, so it moves above that comment.Verification
I replayed
main's CI steps on a clean clone of this branch:pnpm install --frozen-lockfilethenpnpm verifywith pnpm 11.18.0 on Linux and Node 22. Both typecheck projects pass, the tests pass 58/58, and the working tree stays clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01D7P3PhbidfM5fMi1uyWFk5