Skip to content

fix: align windows lifecycle test with server layout#1592

Open
YOMXXX wants to merge 1 commit into
obra:devfrom
YOMXXX:fix/windows-lifecycle-server-cjs
Open

fix: align windows lifecycle test with server layout#1592
YOMXXX wants to merge 1 commit into
obra:devfrom
YOMXXX:fix/windows-lifecycle-server-cjs

Conversation

@YOMXXX
Copy link
Copy Markdown

@YOMXXX YOMXXX commented May 21, 2026

What problem are you trying to solve?

While verifying the brainstorm visual companion tests on current dev, I found that tests/brainstorm-server/windows-lifecycle.test.sh is no longer testing the current brainstorm server. It has drifted behind three implementation changes:

  1. The server entrypoint was renamed from server.js to server.cjs, but the lifecycle test still runs node .../server.js, which fails with MODULE_NOT_FOUND.
  2. The server now writes state under state/ (state/server-info, state/server.pid), but the lifecycle test still waits for .server-info and writes .server.pid at the session root.
  3. Current owner-PID startup validation treats a dead-at-startup owner PID as an invalid PID resolution, logs owner-pid-invalid, disables owner monitoring, and keeps the server running. The lifecycle test still expected that case to self-terminate after 60 seconds.

On current dev, before this fix, running bash tests/brainstorm-server/windows-lifecycle.test.sh on macOS produced:

=== Results: 0 passed, 3 failed, 3 skipped ===

After fixing only the entrypoint and state paths, the same test got past startup but exposed the stale owner-PID assertion:

=== Results: 6 passed, 2 failed, 3 skipped ===

What does this PR change?

This PR updates windows-lifecycle.test.sh to launch server.cjs, read state/server-info, write state/server.pid for the stop test, and assert the current owner-PID behavior.

It preserves the existing runtime code. The test now checks both sides of owner monitoring: dead-at-startup owner PIDs are logged as invalid and do not kill the server immediately, while a live owner process that exits after startup still causes lifecycle shutdown and logs owner process exited.

Is this change appropriate for the core library?

Yes. This is a core test-harness repair for the brainstorm visual companion server. It does not add dependencies, alter runtime behavior, introduce a new harness, or add project-specific configuration. It restores coverage for existing cross-platform lifecycle behavior.

What alternatives did you consider?

I considered only changing server.js to server.cjs, but that still left the test failing because the server no longer writes .server-info at the session root.

I considered only updating paths and changing the dead-at-startup PID expectation to "server survives", but that would remove the control that verifies owner monitoring still shuts down when a valid owner exits. Instead, this PR keeps both cases: invalid owner at startup survives, valid owner that exits shuts down.

I did not add a server.js compatibility wrapper because the accepted fix for the ESM/CommonJS issue was to use server.cjs directly.

Does this PR contain multiple unrelated changes?

No. All changes are in one test script plus the supporting spec/plan, and all address one problem: windows-lifecycle.test.sh had stale assumptions about the current brainstorm server entrypoint, state layout, and owner-PID semantics.

Existing PRs

PR #784 is related prior art for the server.js -> server.cjs rename. It was closed because the commit was cherry-picked to dev; this PR updates the lifecycle test that still referenced the old filename.

PR #879 is related prior art for owner-PID behavior. The current dev behavior supersedes the test's old assumption: invalid owner PIDs at startup are logged as owner-pid-invalid and monitoring is disabled, while valid owner-exit still shuts the server down.

I also searched for open/closed PRs matching windows lifecycle owner-pid server-info and brainstorm server state/server-info windows-lifecycle; no duplicate PRs were found.

Environment tested

Harness (e.g. Claude Code, Cursor) Harness version Model Model version/ID
Local shell tests zsh on macOS Darwin 24.6.0 Node.js v22.14.0 / npm 10.9.2

New harness support (required if this PR adds a new harness)

Not applicable. This PR does not add support for a new harness.

Clean-session transcript for "Let's make a react todo list"
Not applicable: no new harness support is added by this PR.

Evaluation

  • Initial prompt/problem source: local verification of current dev revealed windows-lifecycle.test.sh failed before exercising current lifecycle behavior.
  • RED baseline: bash tests/brainstorm-server/windows-lifecycle.test.sh produced 0 passed, 3 failed, 3 skipped because the test launched missing server.js.
  • Intermediate verification after entrypoint/state path repair: the test produced 6 passed, 2 failed, 3 skipped, showing the remaining stale owner-PID assertion.
  • Final verification after updating owner-PID assertions: the test produced 11 passed, 0 failed, 3 skipped on macOS.

Verification run locally:

npm ci --prefix tests/brainstorm-server
# added 1 package, audited 2 packages

bash tests/brainstorm-server/windows-lifecycle.test.sh
# Results: 11 passed, 0 failed, 3 skipped

npm test --prefix tests/brainstorm-server
# Results: 25 passed, 0 failed

node tests/brainstorm-server/ws-protocol.test.js
# Results: 31 passed, 0 failed

bash -n tests/brainstorm-server/windows-lifecycle.test.sh
git diff --check
git diff --cached --check
# no output

Rigor

  • If this is a skills change: I used superpowers:writing-skills and completed adversarial pressure testing (paste results below)
  • This change was tested adversarially, not just on the happy path
  • I did not modify carefully-tuned content (Red Flags table, rationalizations, "human partner" language) without extensive evals showing the change is an improvement

This is not a skills change. The first checkbox is intentionally not checked.

The test now covers both owner-PID branches instead of only the old happy path:

Invalid OWNER_PID at startup:
- server starts
- logs owner-pid-invalid
- stays alive immediately after startup

Valid owner exits after startup:
- server starts with live owner PID
- owner exits
- server self-terminates at lifecycle check
- log contains owner process exited

Human review

  • A human has reviewed the COMPLETE proposed diff before submission

@YOMXXX
Copy link
Copy Markdown
Author

YOMXXX commented May 21, 2026

Reviewer note

This PR is one of six small, independent PRs opened from the same triage pass. They can be reviewed and merged independently:

Focus for this PR: repair windows-lifecycle.test.sh so it tests current brainstorm server behavior: server.cjs, state/server-info, state/server.pid, and current owner-PID startup validation.

Review surface: test-only plus supporting plan/spec. No brainstorm server runtime files are changed.

Verification: before the fix the lifecycle test failed on current dev; after the fix it reports 11 passed, 0 failed, 3 skipped. server.test.js reports 25 passed; ws-protocol.test.js reports 31 passed.

Risk to check: whether the updated owner-PID assertions correctly reflect current behavior: invalid owner PID at startup survives with owner-pid-invalid, while a live owner that later exits still shuts the server down.

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