Skip to content

fix: reject missing local paths and speed up local checks - #81

Merged
flexdinesh merged 3 commits into
mainfrom
fix-local-missing-path
Oct 21, 2026
Merged

flexdinesh merged 3 commits into
mainfrom
fix-local-missing-path

Conversation

@flexdinesh

@flexdinesh flexdinesh commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner
  • Behavior owner and resulting change:
    • cmd/diffx/local.go (acquireLocalInput): local mode rejects a checkout path that is not an existing directory before serving. Since fix: start local server before collecting workspaces #76/feat: stream local collection and render available snapshots #79 moved collection after startup, a typo such as diffx change started a foreground server that never exited, which hung TestCollectorOptionsRejectAmbiguousInputsBeforeStartup for Go's 10m timeout (or passed by accident when a developer's diffx was running). The test now isolates runtime/config/state and runs each case under a 10s deadline; TestLocalInputRejectsMissingCheckoutPathBeforeStartup covers missing, file and repeated paths. TestLocalInputDefaultsToCurrentDirectory now creates its --path fixture directories.
    • Faster checks:
      • Browser tests: global setup builds the web app and CLI once; each Playwright worker owns an in-memory fixture server behind a Vite preview (SPA fallback, API proxied), so specs run fullyParallel at 33% of cores with no shared review state and no fixed port (DIFFX_TEST_PORT removed). The 4000px full-render inline-comments test is marked test.slow().
      • mise.toml: suites after check:ci (contracts, JS, release, Go) run concurrently; check:push runs the Go suite once, with the race detector, instead of plain then race.
      • Go tests time out after 3m (5m race) so hangs fail fast.
    • TestRemoteRunShutdownClosesActiveEventStream flaked under load (3/40 on origin/main with -race): a keep-alive client could leave a spare, never-used connection that http.Server.Shutdown waits on for 5s. The test client now disables keep-alives; 80/80 pass under load.
  • Contracts/invariants affected; local/remote implications: local startup input validation only; later Sync of a removed path still reports a discovery diagnostic. Test/tooling changes otherwise; pnpm dev still starts its own fixture via go run.
  • Validation results; remaining risks:
    • Pre-push mise run check:push passed in 3m56s (previously ~12m), with 88/88 browser tests in 2.9m, while another project's Playwright suite loaded the machine (load avg ~30-50). A run at lower load with warm caches took 96s.
    • Risk: concurrent suites raise peak CPU; timing-sensitive tests may surface on slower machines. Worker count is capped at 33% for headroom.
  • Intentional architecture/design changes and rationale: docs/system.md documents the path rule; docs/development.md documents per-worker browser servers, concurrent suites, race-only Go run in pre-push and Go timeouts.
  • UI evidence: n/a.

- local mode validates each checkout path is a readable directory before startup
- typos like `diffx change` no longer start a server that never exits
- ambiguous-input test isolates runtime/config/state and fails at a deadline instead of hanging
- each Playwright worker owns a fixture server behind a Vite preview of one shared build
- browser specs run fully parallel at 33% of cores; slow full-render test marked slow
- mise runs post-build suites concurrently; check:push runs Go tests once, with race
- Go tests time out after 3m (5m with race) so hangs fail fast
A keep-alive client could leave an unused connection that http.Server.Shutdown
waits on for 5s, flaking the active-stream shutdown test under load.
@flexdinesh flexdinesh changed the title fix: reject missing local checkout paths before serving fix: reject missing local paths and speed up local checks Oct 21, 2026
@flexdinesh
flexdinesh marked this pull request as ready for review October 21, 2026 06:40
@flexdinesh
flexdinesh merged commit 7d406b4 into main Oct 21, 2026
2 checks passed
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.

1 participant