Repository navigation
fix(cli): accept --project-dir in schedule and webhook, retire their cwd lane - #4026
Conversation
…cwd lane `schedule`, `webhook`, and the TUI's project creation read the process working directory with no way to pass a directory in. Their tests therefore had to `Deno.chdir` through `withCwd`, which put four files in the serial `unit:cwd` lane and kept a cross-isolate lock in play for every run of the unit suite. Give each site the parameter the rest of the CLI already has: `schedule` and `webhook` take `CommonArgs.projectDir` (`--project-dir`, `--dir`, `-d`) and resolve `opts.projectDir ?? Deno.cwd()`; `createProject` accepts an optional `baseDir` on its context and `remoteProjectPath` / `setRemoteProjects` an optional `baseDir`, all defaulting to the working directory so no existing caller changes behaviour. The four tests pass the directory instead of entering it, drop the `withCwd` import, and return to `unit:parallel`; `unit:cwd` keeps only the files that test cwd itself or a literal "." argument. Constraint: Without `--project-dir` every command resolves exactly as before. Rejected: Pin "every withCwd importer is in UNIT_CWD_FILES" | withCwd holds a cross-isolate disk lock, so parallel-lane files already use it safely and the invariant is false today. Confidence: high Scope-risk: narrow Reversibility: clean Tested: the four migrated files, run-suite/suites pin tests (3/17 steps), test:unit:cwd (3 files, 112 steps), the four files under --parallel with five unrelated cli tests (14/277 steps), semantic disposition audit (2141/762, unchanged), test:layout, cwd-relative read audit (baseline unchanged), deno check/lint/fmt on every touched file. Not-tested: hosted CI and merge queue, pending push.
There was a problem hiding this comment.
kojiwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Why
schedule,webhook, and the TUI's project creation read the process working directory with no way to pass a directory in. Their tests therefore had toDeno.chdirthroughwithCwd, which kept four files in the serialunit:cwdtest lane and a cross-isolate disk lock in play for every unit-suite run.What
scheduleandwebhookaccept the project directory argument the rest of the CLI already has (CommonArgs.projectDir:--project-dir,--dir,-d), resolvingopts.projectDir ?? Deno.cwd(). Help text updated.createProjectaccepts an optionalbaseDiron its context;remoteProjectPath/setRemoteProjectstake an optionalbaseDir. All default to the working directory — no caller changes behaviour.withCwd, and return tounit:parallel.UNIT_CWD_FILESinscripts/test/run-suite.tsshrinks to the three files that are genuinely about cwd itself (skills/validatetests a literal".",platform/compat/processandtesting/cwdtest the primitives).Notes
shared-cwd.cli/commands/build/embedded-preset-flags.test.tsandcli/auth/login.test.tsstill usewithCwdfrom the parallel lane (safe under its disk lock, but the same parameter treatment applies).Verification
src/testing/cwd.test.ts+ run-suite/suites pin tests: pass.deno task test:unit:cwd(now 3 files): pass. The four files run under--parallel --trace-leaksalongside five unrelated cli tests: 14 passed (277 steps), no leaks.deno task lint:test-semantic-dispositions,deno task test:layout,deno task lint:cwd-relative-test-reads: ok.test:unit): passed on push.