Skip to content

fix(server): run cli.test.ts main() cases from an empty temp dir - #561

Merged
davidwkeith merged 1 commit into
mainfrom
claude/affectionate-curie-3ekeg3
Oct 23, 2026
Merged

davidwkeith merged 1 commit into
mainfrom
claude/affectionate-curie-3ekeg3

Conversation

@davidwkeith

@davidwkeith davidwkeith commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

main() calls loadDwkEnv() against process.cwd(), so a developer's local .env leaked into process.env during cli.test.ts (recorded in #560). The main describe block now chdirs into a fresh empty temp dir in beforeEach and restores the cwd in afterEach. The two .env cases already chdir into their own dirs and restore to that temp dir, so they're unchanged.

Reproduced first: with DWK_LEAK_PROBE=leaked in a repo-root .env and a probe in afterAll, the unfixed file read back PROBE=leaked. With this change it reads PROBE=undefined.

packages/server/CLAUDE.md's test-secrets bullet now describes the fix instead of the leak.

Packages affected

@dwk/server (test + docs only)

Checklist

  • Read the relevant spec(s) under spec/packages/ and updated them if
    behaviour changed — not applicable, no behaviour change
  • Added/updated colocated tests (src/*.test.ts)
  • Ran the local CI gate: pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm test — all five passed (257 test files, 3265 tests)
  • Added a changeset (pnpm changeset) if this touches a publishable
    package — not applicable, @dwk/server is private
  • Updated catalog.json / conformance/status.json if this adds a new
    mountable worker or changes conformance status — not applicable, no new worker

🤖 Generated with Claude Code

https://claude.ai/code/session_01CMCuDa11Qr39yxAQJNUXoH

main() calls loadDwkEnv() against process.cwd(), so a developer's local
.env leaked into process.env during cli.test.ts.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMCuDa11Qr39yxAQJNUXoH

@davidwkeith davidwkeith left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review against CONTRIBUTING.md and the diff: no blocking issues.

Conforms

  • The title is Conventional Commits (fix(server): …) with a lowercase subject.
  • The body keeps the template headings Summary, Packages affected and Checklist. The inapplicable items are left unchecked, each with a one-line reason.
  • No changeset is needed, since @dwk/server is private. No catalog or conformance update is needed.
  • The code change is correct. beforeEach and afterEach scope the chdir to the main block. The two .env cases restore into the per-test temp dir, and afterEach then restores the original cwd. workdir() is already defined at line 16.
  • The packages/server/CLAUDE.md update matches the new behaviour.

Suggestions

  1. The "Ran the local CI gate" item is unchecked because the full pnpm lint && format:check && typecheck && build && test sequence wasn't run. CONTRIBUTING expects the whole gate before merge. Please run it, or confirm CI is green, and tick the box.
  2. The afterEach restore of the cwd is correct. If workdir() or process.chdir throws in beforeEach, prevCwd is still set first, so that case is safe too. No change is needed there.

Generated by Claude Code

@davidwkeith
davidwkeith enabled auto-merge (squash) October 23, 2026 03:51
@davidwkeith
davidwkeith merged commit 55c839b into main Oct 23, 2026
9 checks passed
@davidwkeith
davidwkeith deleted the claude/affectionate-curie-3ekeg3 branch October 23, 2026 03:53

Copy link
Copy Markdown
Owner Author

Ran the full local gate (pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm test) on this head. All five steps passed: 257 test files, 3265 tests. I ticked the checklist box. Agreed on suggestion 2, so no change there.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants