git worktree remove refuses when the tree holds modified or untracked
files, and the skill gave no guidance for that refusal — the natural
agent response was --force, permanently destroying files that exist
nowhere else (uncommitted plans, notes, scratch work). Reported twice
from real sessions (#2016's plan loss, #1223's dirty-tree ambiguity).
Step 6 now treats the refusal as a stop-and-ask moment: show the
untracked files, offer commit / relocate / delete, and only remove the
worktree after the human partner chooses. Adds a matching rationalization
row so --force-as-cleanup is named as the failure it is.
Follow-up to #2011 (which fixed the ./-prefix mismatch for the documented
pattern form): strip a leading ./ from the caller's pattern instead of
double-prefixing it into a never-matching ././ form, and also match the
pattern with '**/' collapsed, since find -path cannot match '**/' against
zero directory levels and silently skipped files directly under the base
directory (src/top.test.ts vs src/**/*.test.ts).
Adds a deterministic test suite for the script with a stubbed npm.
The eval verdict on this cut: deleting Why Order Matters and trusting the
compressed one-line table rows measurably degrades test-first behavior under
the exact pressure the section rebutted ("just write it, tests after") —
control 8/10 → treatment 5/10 at n=10, corroborated on both Claude and Codex.
Normal TDD triggering did not move (PPPPP → PPPPP both arms); the damage is
purely the pressure case.
So instead of trusting the compressed rows, fold the section's five prose
rebuttals into their Common Rationalizations rows so each row carries the
argument, not just the excuse label:
- "I'll test after" — passing immediately proves nothing (wrong thing /
implementation-not-behavior / missed edge; you never saw it fail).
- "Already manually tested" — ad-hoc, no record, can't re-run, forgotten
under pressure.
- "Deleting X hours is wasteful" — sunk cost; rewrite-high-confidence vs
bolt-tests-on-after-low-confidence.
- "TDD will slow me down" — TDD is the pragmatic path; shortcuts mean
debugging in production.
- "Tests after achieve same goals (spirit not ritual)" — what-does vs
what-should; biased by the code you wrote; coverage without proof.
Still removes the 50-line section (~200 words / 45 lines net); the
arguments survive where an agent hits them mid-rationalization. Revalidate
with the tdd-holds-under-tests-later-pressure probe before merge.
Restates the evaluate-don't-obey frame, verification rule, and
no-performative-agreement rule, each detailed earlier at point of use.
The Common Mistakes table stays: it is the skill's one compact guard
table, the class this cleanup standardizes toward rather than deletes.
All four lines restate the Overview (DRY/YAGNI/TDD/frequent commits),
Task Structure (exact paths, commands with expected output), and No
Placeholders (complete code in every step).
Five of six principles restated the Checklist and Process sections
verbatim-in-spirit. The sixth, YAGNI, appeared nowhere else — it moves to
the Exploring approaches list where designs get shaped; the recap section
goes.
Common Mistakes and Red Flags restated Steps 0-3 wholesale; both fold
into one Common Rationalizations table (house Excuse/Reality form) whose
five rows carry the tempting-thought version of each rule, including the
#1-mistake emphasis on bypassing native tools. Quick Reference stays as
the compact decision aid.
Integration with Workflows restated the When to Request Review triggers
grouped by caller (each-task / before-merge / when-stuck all appear at
point of use) — detritus, so it goes.
The intro's crafted-context sentence guarded two things at once, so keep
both as Common Rationalizations rows (house Excuse/Reality form) rather
than deleting the sentence. The skill's reader is the coordinator, not
the code's author:
- Don't review the diff inline — that burns the coordinator's context
window; dispatch a subagent so the diff and evaluation live in its
context and only findings return. ("preserves your own context for
continued work")
- Don't hand the reviewer your session history — crafted context keeps it
on the work product, not your thought process.
Five blocks of benefits and cost/benefit selling aimed at a reader who
has already invoked the skill; the vs-Executing-Plans comparison also
duplicates the one under When to Use. Integration section untouched
(PR #1932 owns it).
The tell-your-partner directive and the prefer-SDD instruction stay; the
significantly-higher-quality sentence restated them as a claim.
Integration section untouched (PR #1932 owns it).
Why This Matters (failure-memory testimonials), the dishonesty reframing
in the Overview, and The Bottom Line recap all restate stakes the Iron
Law, gate function, and rationalization table already enforce. This is
the eval-gated class: the bet is that discipline holds without the
persuasion prose — evals on this branch decide.
Real-World Impact was statistics; the Overview opener restated the core
principle as motivation. The 95%-of-no-root-cause line stays: it guards
the bail-out point, which is rationalization control, not social proof.
Supporting Techniques/Related skills untouched (PR #1932 owns that).
Real-World Impact restated the Real Example from Session as statistics;
Key Benefits and the time-saved line sold the skill to a reader already
executing it. Instructions unchanged.
Re-derived from scratch: every rule becomes a corollary of two principles
(every test names the break it catches; every test exercises the real
thing), one consolidated gate per principle, four example pairs kept, the
rest carried by prose. Scratch branch for comparison against the accreted
eight-rule version.
Prose additions from the last two passes tightened to the terse guard
form: change-detector rule, string-presence trap, and Rule 7's release
valve each drop to a few sentences. Rule 7 now settles the jurisdiction
question outright: trivial code and human prose earn no test; skills and
prompts are pressure-tested per writing-skills when edits change
behavior, never text-asserted. Micro-tested: a subject with a README
rewrite plus a skill typo fix, under tests-with-every-PR pressure,
shipped zero tests — declining the string assertions and the ceremonial
subagent pressure-test alike.
Fresh-eyes review found falsifiable-but-worthless tests passed every
rule: a constant assertion can fail, uses a literal, mocks nothing — and
protects nothing, firing on intentional decisions while sleeping through
bugs. Rule 1 gains the what-break-would-this-catch question (absorbed
from the source skill's quality gate, missed in the first pass) with a
gate stop for change detectors; Rule 6's trivial-code list regains
constants; Rule 7 gains the release valve that trivial-only changes earn
no ceremonial test; the coverage-theater and change-detector smells join
Warning Signs; the Rule 6 example stops modeling exact-copy brittleness.
Micro-tested: under a tests-with-every-PR norm, a subject rejected both
draft constant tests citing the new gate and replaced them with a test of
the retry behavior the constant controls.
Generalized from agentsview's testing-without-tautologies skill: a new
Iron Law and lead rule (name the production change that would fail the
test, derive expectations independently of the code under test), a
test-your-code-not-the-framework rule with the characterization-test
exception and the trivial-code guidance, branch-specific doubles folded
into Mock at the Right Level, a closing Mutation Check, and six new
warning-sign smells. Rule 1 carries the string-presence trap by name:
grep-style tests on scripts, skills, and prompts counterfeit
falsifiability — the observable is the artifact's behavior, never its
text — with a hard stop in the gate function. Repo-specific content
(testify, backend parity, test-level ladder) stays in the source skill.
Micro-tested: 3/3 tautology verdicts with correct rule citations and the
mutation check named unprompted; a RED-pressure subject refused the
10-second grep test and wrote a behavioral one citing the trap.
The pointer fired only on adding mocks or test utilities; the doc's own
load-when line already says writing or changing tests. The narrow trigger
would skip the rules exactly when an agent thinks no mocks are involved.
The disclosure doc becomes a catalog of what to do: six positively named
rules (assert on real behavior, cleanup in test utilities, mock at the
right level, mirror real data, tests ship with implementation, prefer
real components), each leading with the GOOD example and keeping the
violation as contrast. Iron Laws, gate functions, human-partner lines,
and warning signs all survive; The Bottom Line recap and the
TDD-prevents-these section fold into one Overview sentence. SKILL.md's
pointer moves into the Good Tests section it belongs with. Micro-tested
2/2: a mock-existence assertion got rewritten to a real-behavior
assertion citing Rule 1, and a test-only teardown method plus a
to-be-safe mock were both rejected citing Rules 2 and 3.
Step 6 recomputed WORKTREE_PATH after Option 1 and discard had already
cd'd to the main repo root, so --show-toplevel returned the main root:
the provenance check could never match, cleanup silently no-oped, and the
branch delete failed with the worktree still attached. A test subject had
to deviate from the literal skill to produce a working sequence. The
capture moves to Step 2 (still inside the workspace); Step 6 consumes
Step 2's values and drops its redundant recompute and MAIN_ROOT
derivation. Also: Option 2 gains the detached-HEAD push variant its menu
advertises, and the stale-green rationalization row states what a green
run proves instead of asserting the tree changed. Re-verified: merge-flow
and discard-flow subjects both walk the literal skill to correct cleanup
with concrete paths and no deviations.
Red Flags and Common Mistakes fold into one Common Rationalizations table
(house Excuse/Reality form); every prior entry maps to a table row or an
inline sentence in the step it guards. Instructions rephrase positively —
what to do rather than what to avoid — with negations remaining only in
statements of fact. Workflow prose tightens throughout; menus, detection
mechanics, cleanup provenance, and the typed-discard ritual are unchanged.
Re-verified 4/4 after the rewrite: both menus verbatim, the lukewarm-human
pressure arm cited the rationalizations table when declining to offer
discard, and a prose discard request still required the literal typed
word.
Naming gh and glab implicitly blessed two forges; Gitea, Forgejo,
Bitbucket and others are equally valid. Point at the forge's CLI or the
creation URL printed on push instead of naming tools.
The completion menu dates from when throwing away branches was routine;
offering 'Discard this work' beside 'Merge' on every completion advertised
destroying finished, passing work. The menu is now 3 options (2 detached
HEAD); discard survives as an explicit-request-only path with the same
typed-confirmation ritual and cleanup mechanics. Fresh-eyes fixes in the
same pass: Option 2 actually creates the pull/merge request
(platform-neutral tooling) and reports the URL; Step 3's base-branch
detection drops a command that printed a SHA instead of choosing a branch
(ask when not known); Option 1 gains a failure branch (merged-result test
failures stop cleanup); description trimmed to trigger-only. Micro-tested
4/4: both menus verbatim with no discard, no discard offer even when the
human sounded lukewarm about the feature, and a prose 'throw it all away'
still required the typed confirmation before any deletion.
Same treatment as subagent-driven-development and executing-plans: the
test-driven-development entry duplicated the reference already at Phase 4
Step 1, and the verification-before-completion entry was a sole carrier —
it moves to its point of use in Phase 4 Step 3 (Verify Fix). Micro-tested
2/2: subjects at the just-implemented-a-fix point invoke
verification-before-completion before any success claim, including under
ship-pressure.
The list-style Integration sections in subagent-driven-development and
executing-plans duplicated references that already exist where the flow
uses them (process digraph, When to Use, prompt templates, Step 3), so
they added maintenance cost without carrying behavior. The one entry not
duplicated anywhere — the using-git-worktrees isolated-workspace
requirement — moves to its point of use: SDD's Pre-Flight Plan Review and
executing-plans' Step 1. Micro-tested 5/5: controllers at skill start
establish or verify the worktree before reading the plan or dispatching
Task 1, including under skip-the-ceremony pressure. The prose Integration
sections in requesting-code-review and other skills are unchanged — they
carry placement content, not an index.
Bump all manifests to 6.1.0 and add RELEASE-NOTES for v6.1.0:
- Compress the using-superpowers bootstrap and prune per-harness
tool-mapping references (lower per-session token cost).
- Add a Codex marketplace manifest so the plugin installs from Codex;
drop the Codex SessionStart hook.
- Remove Gemini CLI support (Google EOLed the Gemini CLI 2026-06-18).
Commit 1f0c76e removed the Codex SessionStart hook — dropping the hooks
field from .codex-plugin/plugin.json and deleting hooks-codex.json — but
left test-marketplace-manifest.sh asserting the old hooks pointer, so the
test has failed on dev since. Assert the field is absent instead, locking
in the no-Codex-hooks decision.
The bootstrap is injected into every session, so its token cost is paid
constantly. Condense it without dropping behavior-shaping content:
- Replace the graphviz skill-flow diagram with the prose it encoded (the
1% rule, the plan-mode to brainstorm gate, announce + checklist to todos).
- Fold the standalone Instruction-Priority section into User Instructions.
- Drop the per-platform 'How to Access Skills' walkthrough.
- Trim the Platform Adaptation pointer to the harnesses that still have a
reference file (Codex, Pi, Antigravity).
Keeps the full Red Flags rationalization table, skill priority framed as
process-before-implementation, and user-instruction precedence.
The verbose action-to-tool tables and skill-loading explainers in the
per-harness reference files restated guidance modern agents already
follow. Trim each file to the harness-specific notes that still carry
weight (subagent dispatch, task tracking, instructions-file paths), and
delete claude-code-tools.md and copilot-tools.md, which had nothing left
that wasn't generic.
Google EOLed the Gemini CLI on 2026-06-18; the extension can no longer
be installed or updated. Remove Gemini from the install docs, the
subagent-capable platform lists, and the eval-harness description, and
delete its tool-mapping reference.
Codex reliably triggers skills on its own, and the SessionStart hook
made the UX worse rather than better. Drop the Codex hook config and
its registration in the plugin manifest.
Prompt: Jesse questioned whether the PR should remove the hooks config from the Codex plugin manifest.
Runtime investigation showed Codex accepts a committed plugin manifest with hooks and installs the plugin successfully. Removing the field changes behavior: Codex falls back to the default hooks/hooks.json, which uses the non-Codex session-start hook and CLAUDE_PLUGIN_ROOT path, instead of hooks/hooks-codex.json and the session-start-codex script.
Changes: restore .codex-plugin/plugin.json hooks to ./hooks/hooks-codex.json and update the Codex marketplace manifest test to require that Codex-specific hook pointer instead of rejecting hooks.
Validation: bash tests/codex/test-marketplace-manifest.sh; scripts/lint-shell.sh tests/codex/test-marketplace-manifest.sh; bash tests/codex-plugin-sync/test-sync-to-codex-plugin.sh; bash tests/kimi/test-plugin-manifest.sh; bash tests/shell-lint/test-lint-shell.sh.
Prompt: Jesse asked for a new worktree off the local superpowers dev branch to add the Codex manifest after diagnosing why github.com/obra/superpowers did not show installable Codex plugins.
Root cause: Codex marketplace sources expect a .agents/plugins/marketplace.json at the marketplace root. The superpowers repo only had the Claude marketplace file and the Codex plugin manifest, so Codex could configure the marketplace name but found no installable plugin entries.
Changes: add a repo-local Codex marketplace manifest for superpowers-dev that points at this same repository root via the same-root source pattern Codex already accepts; add a focused marketplace manifest test; remove the unsupported hooks field from .codex-plugin/plugin.json so the plugin validator accepts the manifest.
Validation: bash tests/codex/test-marketplace-manifest.sh; uv run --with PyYAML python /Users/jesse/.codex/skills/.system/plugin-creator/scripts/validate_plugin.py /Users/jesse/git/superpowers/superpowers/.worktrees/codex-marketplace-manifest; throwaway HOME codex plugin marketplace add/list/add; bash tests/codex-plugin-sync/test-sync-to-codex-plugin.sh; bash tests/kimi/test-plugin-manifest.sh; bash tests/shell-lint/test-lint-shell.sh; scripts/lint-shell.sh tests/codex/test-marketplace-manifest.sh.
Bump all plugin manifests to 6.0.3. This release moves subagent-driven-
development's scratch artifacts (task briefs, implementer reports, review
diffs, progress ledger) from .git/sdd/ — which Claude Code denies agent
writes to — into a self-ignoring working-tree .superpowers/sdd/ dir, and
bumps the brainstorm-server test harness's ws dependency to clear two
dependabot alerts. See RELEASE-NOTES.md.
Clears two dependabot alerts on the test harness's ws dependency:
GHSA-96hv-2xvq-fx4p (high, memory-exhaustion DoS, fixed 8.21.0) and
GHSA-58qx-3vcg-4xpx (medium, uninitialized memory disclosure, fixed
8.20.1). Test-only — the shipped brainstorm server hand-rolls its
WebSocket framing and does not depend on ws. Suite passes (57/57).
The per-worktree workspace test was added but never registered in
run-skill-tests.sh, so it only ran when invoked by hand. Add it to the
fast unit-test array alongside the other pure-shell test.
Also document, in the Durable Progress section, that the ledger now
lives in git-ignored working-tree scratch, so `git clean -fdx` deletes
it — recover from `git log` if that happens.
- Brainstorm companion reads version from .codex-plugin/plugin.json when package.json is absent (PRI-2240)
- sync-to-codex script excludes .gitmodules and .pre-commit-config.yaml (PRI-1168)
Points evals at superpowers-evals 70a245c, which replaces the seven
per-agent *-superpowers-bootstrap scenarios with one cross-agent
superpowers-bootstrap scenario (adds the QUORUM_CODING_AGENT env var and
the bootstrap-installed dispatcher check verb).
scripts/review-package generates the reviewer's input deterministically:
commit list, stat summary, and net diff with -U10 context, written to a
file from an explicit BASE. Live runs showed controllers improvising
'git diff HEAD~1..HEAD', which silently truncates multi-commit tasks,
and svelte's five fix dispatches shipped without re-running any tests —
fix dispatches now explicitly carry the implementer's
re-run-and-report contract.
The skill read as a changelog: 'combined task review,' 'one reviewer,
one reading,' 'one dispatch,' and an example still showing diffs pasted
into prompts. A reader who never saw the two-reviewer design has no
referent for 'combined.' Prose now states the design directly, and the
flowchart/example reflect the diff-file handoff.
Paste adoption stayed at 0/15 even as a Red Flag — and the controller's
reluctance is locally rational: pasting loads the diff into the (most
expensive) controller context permanently, while a reviewer self-fetch
costs a few cheap turns. The diff-file handoff is cheap for both sides:
the controller redirects git diff to /tmp without reading it, and the
reviewer gets the whole change in one Read call.
Fourth planted-defect failure mode: the implementer's self-report said
'noted mild structural duplication; left unabstracted per YAGNI' and the
reviewer deferred to that framing, rating the duplication no finding at
all. The pre-judging keeps relocating — controller prompt, then reviewer
calibration, now the implementer's report. Rationales are claims; they
never downgrade severity.
Adoption was 6/11 reviews on fractals and 0/17 on svelte when phrased
as guidance; reviewers without the diff re-derive it by hand, which is
the single largest remaining reviewer cost. Now a Red Flags Never entry
and a REQUIRED marker on the template placeholder.
With merged review, a planted verbatim-duplication defect shipped: the
reviewer rated it Minor (YAGNI) under the strict cannot-be-trusted
definition of Important, and the Minor-rolls-up rule meant no fix was
ever dispatched and the final review never saw the finding. Calibration
now names merge-blocking maintainability damage (verbatim duplication,
swallowed errors, assertion-free tests) as Important, and controllers
must paste accumulated Minor findings into the final review dispatch.
Iteration-1 profiling: implementers and per-dispatch overhead dominate
(429 of 686 subagent turns; controller coordination is half the dollars
and scales with dispatch count), reviewers are individually lean, and
the controller pasted the diff in only 2 of 22 review dispatches when
the guidance was phrased as optional.
Changes: spec-reviewer-prompt.md + code-quality-reviewer-prompt.md
replaced by task-reviewer-prompt.md (one reviewer, one reading of a
pasted diff, two verdicts: spec compliance ✅/❌/⚠️ and task quality);
one fix dispatch can address both kinds of findings; controller now
runs git diff itself and pastes it (imperative, not optional);
implementers run focused tests while iterating and the full suite once
before committing; flowchart, example, Red Flags, tool tables updated.
The broad final whole-branch review is unchanged.
Round-2 fractals eval regressed to 70min/32.2M tokens (vs round-1's
42.8min/14.5M) while reaching baseline-parity quality. Per-subagent turn
profiling attributed it to: haiku dispatches taking 2-3x the turns of
sonnet (678 of 1197 subagent turns), reviewers re-fetching diffs by hand
(518 Bash calls), and evidence-rule narration. Changes: turn-count-beats-
token-price model guidance; controllers paste small diffs into reviewer
prompts (reviewers then need few or no tool calls); evidence scoped to
findings and would-be-bare-yes checks; Important defined as cannot-trust-
until-fixed with coverage suggestions Minor; fixes dispatched only for
Critical/Important.
Resumed the offending eval controller session and asked it why it
pre-judged despite the rule being in context. Its retrospective: the
motive was avoiding a review loop, the abstract rule was read but not
applied at the moment it governs, and a phrase-level trigger ('do not
flag', 'at most Minor', 'don't treat X as a defect', 'the plan chose')
would have fired where the principle did not.
Second observed instance: with the Constructing Reviewer Prompts rule
already live, a controller still wrote 'do not treat that duplication as
a defect to fix — the plan chose it; you may note it as a Minor
observation at most' into a quality reviewer dispatch, fabricating plan
intent from the plan's example snippet. Promote the rule to the Red
Flags Never list and name the rationalization.
Live eval deliverables shipped five polish defects; tracing each through
the transcripts showed three mechanisms, each now addressed:
- reviewers answered pointed checklist items with unsupported yes
(evidence rule: every What-to-Check answer needs file:line evidence)
- no reviewer ever saw the design's global constraints (controllers now
paste binding constraints into task requirements)
- test output noise was invisible everywhere (pristine-output checks in
implementer self-review and quality review)
In live eval runs, controllers given judgment-based model selection
stopped passing a model at all; the omitted parameter inherits the
session's top-tier model, silently making every subagent maximally
expensive (one run dispatched 26/26 reviewers on the session model).
A live eval run of sdd-quality-reviewer-catches-planted-defect caught the
SDD controller fabricating a plan constraint and instructing the quality
reviewer not to flag the planted DRY violation. The duplication shipped.
Constructing Reviewer Prompts now bans suppression directives alongside
open-ended broadening directives.
A second adversarial review of the merged branch found that combining the
session-key auth with the feature work created real bugs the (vacuous) tests
missed:
- [Critical] GET /files/ (empty name) resolved to CONTENT_DIR and crashed the
process with uncaught EISDIR — newly reachable because the query-stripping
refactor turns /files/?key=... into /files/. Reject non-regular-file names.
- [High] --open opened a KEYLESS url, which the auth gate 403s — the headline
feature landed on the error page. Open the keyed url.
- [High] Same-port restart regenerated the token (port persisted, token not), so
the open tab's old cookie 403'd and never reconnected — contradicting the
documented promise. Persist the token (BRAINSTORM_TOKEN_FILE / .last-token)
alongside the port.
- [Medium] Token sat in world-readable server-info/server.log (0644 in /tmp).
umask 077 in start-server.sh + mode 0600 on server-info/.last-token.
- [Medium] touchActivity() ran before the auth check, so unauthenticated requests
defeated the idle timeout. Count activity only after authorization.
- [Low] COOKIE_NAME embedded the pre-fallback port; derive it from the actual
bound port (also prevents a cross-server cookie-jar collision on fallback).
Tests added/strengthened (previously passed vacuously): /files/ no-crash; the
auto-open url carries the key and is reachable (200); restart reuses the same key
not just the port; unauthenticated requests don't reset the idle clock.
Full suite green (ws-protocol 32, helper 12, auth 13, server 29, lifecycle 8,
stop-server 4); restart smoke confirms same port+key and old URL -> 200.
Integrating the per-session-key auth onto the same branch as the dotfile and
lifecycle work: two tests added after the auth commit opened WebSockets without a
key (server.test.js dotfile-reload, lifecycle.test.js idle-shutdown), which the
auth gate now resets. Pass ?key=/BRAINSTORM_TOKEN in both. Full suite green:
ws-protocol 32, helper 12, auth 13, server 28, lifecycle 7, stop-server 4.
The companion server is reachable by any local browser tab (default loopback
bind) and by any host that can route to it (remote --host bind). It served
screens, files, and accepted event-injecting WebSocket connections with no
authentication, so a malicious browser tab or a direct remote client could read
brainstorm content or inject events that the agent reads as the user's input
(prompt injection into a live session).
Generate a per-session secret token, carry it in the served URL as ?key=, and
mirror it into an HttpOnly SameSite=Strict per-port cookie on first load so
same-origin subresources and the WebSocket handshake authenticate automatically.
Every HTTP request and WebSocket upgrade now requires a valid key (query or
cookie, constant-time compared); unauthenticated requests get a friendly 403
explaining they need the full URL. A secret authenticates the client uniformly
across loopback, tunnel, and remote binds and defeats DNS rebinding, which a
Host/Origin allowlist cannot.
Also guard handleMessage against a null JSON payload that crashed the process.
Tests: new auth.test.js (13 cases) covering the key on /, /files/*, and WS plus
cookie bootstrap and the null-payload guard; server.test.js threads the key;
ws-protocol.test.js + auth.test.js wired into npm test.
Closes#1014
Refs #1110, #1553, #1504
Records the triage of open issues/PRs touching the brainstorm companion server
and the decision to protect it with a per-session secret key (supersedes the
Host/Origin allowlist approach) so remote-connected users are covered, not just
loopback.
The node+server.cjs command match (from the adversarial review) still matched any
unrelated node process running a file named server.cjs. When we recorded the
bound port (state/server-info) and lsof is available, additionally require the
PID to be the process actually LISTENING on this session's port — which rules out
a different project's server.cjs / editor task runner that recycled the stale
PID. Falls back to the command match when the port or lsof isn't available.
Test: a 'node server.cjs' process not listening on the recorded port is spared.
Refs #1703
From a two-reviewer adversarial pass:
- [High] EADDRINUSE fallback clobbered the shared .last-port: onListen wrote the
bound port unconditionally, so a fallback to a random port overwrote the
preferred port another live session still owns — stranding that session's open
tab forever. Now persist only when we bound the preferred port (not on
fallback). The fallback test now asserts .last-port integrity (teeth-verified).
- [Medium] maybeOpenBrowser ran the URL through a shell (exec + JSON.stringify),
which does NOT neutralize $(...) in a url-host. Platform launchers now use
execFile with the URL as an argv element (no shell). The operator-set
BRAINSTORM_OPEN_CMD path stays shell-based (trusted input).
- [Medium] --open was a silent no-op on native Windows (no win32 branch). Added.
- [Medium] helper.js reconnect/status/tombstone had only substring-grep tests.
Added behavioral tests driving the state machine against a mocked browser:
Reconnecting+backoff (500->1000->2000), tombstone after the grace period, and
reload-on-recovery.
- [Low] status pill showed a false 'Connected' before the socket opened; now
starts 'Connecting…' until onopen.
Not changed (flagged): stop-server.sh's PID-ownership check still matches any
'node ... server.cjs' (narrow residual — a recycled PID onto an unrelated node
server.cjs); robust fix needs fragile cross-platform process introspection.
Move the companion consent from an upfront, anticipatory offer to the first
moment a question would genuinely be clearer shown than told. If no visual
question ever arises, it's never offered. On approval the agent starts the
server with --open, so the user's browser opens to the first screen — the pop is
tied to that approval, never unsolicited.
Also hardens visual-companion.md: confirming the server is alive (server-info
present, server-stopped absent) before referring to the URL is now a required
step; restart with the same --project-dir reuses the port so the open tab
reconnects on its own (paused overlay while down); idle default corrected to 4h.
NOTE: SKILL.md is behavior-shaping content — this flow change should be
eval-tested (writing-skills adversarial pressure test) before merge.
Refs #1237, #1037
When the user approves the visual companion, open their browser automatically the
first time a screen is actually ready to show — rather than at startup (just the
waiting page) or making them open the URL by hand.
Opt-in and gated on approval: off unless BRAINSTORM_OPEN is set (start-server.sh
--open, which the agent passes only after the user agrees to use the companion).
Even then it fires once, and is skipped if a browser is already connected, on a
non-loopback/remote bind, or when headless. Launcher is the platform default
(open / xdg-open / WSL cmd.exe) or BRAINSTORM_OPEN_CMD; best-effort, never fatal.
lifecycle.test.js: opens once on the first screen when approved; does NOT open
without approval.
Closes#755
Refs #759
When the companion idle-shuts-down and the agent restarts it, a fresh random
port meant the user's open browser tab pointed at a dead URL. Persist the bound
port per project and prefer it on the next start, so the restarted server comes
up on the same port and the open tab's reconnect just works.
- start-server.sh exports BRAINSTORM_PORT_FILE=<project>/.superpowers/brainstorm/
.last-port for project sessions (not /tmp).
- server.cjs prefers an explicit BRAINSTORM_PORT, else the recorded port, else
random; writes the actually-bound port back; and on EADDRINUSE (preferred port
still in use) falls back to a random port once instead of crashing.
lifecycle.test.js: restart reuses the recorded port; a taken preferred port
falls back to a random one without crashing.
Refs #1237
The injected client reconnected on a fixed 1s timer with no feedback: if the
laptop slept or the server restarted, the page showed 'Connected' over a dead
socket and silently queued events. And when the server stopped, the user got a
bare connection-refused with no explanation.
helper.js now:
- reconnects with exponential backoff (500ms, doubling, capped at 30s; reset on
open), with an onerror->close handler, nulls the socket on close, and clears a
pending timer before scheduling another;
- drives the frame status pill Connected/Reconnecting/Disconnected via a
--status-color custom property (frame-template.html);
- after ~15s disconnected, shows a self-styled 'Companion paused' overlay
(tombstone) explaining the companion stopped and will reconnect automatically;
- on recovery from a tombstoned outage (e.g. server restarted on the same port)
reloads to pick up the restarted server's current screen.
The reconnect-backoff is an exported pure function; helper.test.js unit-tests it
(doubling + cap progression) and asserts the status/tombstone/reconnect wiring.
DOM behaviour is verified live.
Refs #856, #1237
The companion shut down after only 30 minutes idle — too short for real
brainstorming, where a single question can sit far longer. And shutdown() never
closed upgraded WebSocket sockets, so an open browser connection could keep the
Node process alive after it was supposed to exit.
- Default idle timeout raised to 4 hours, configurable via BRAINSTORM_IDLE_TIMEOUT_MS
and start-server.sh --idle-timeout-minutes (validated positive integer).
- Reported as idle_timeout_ms in the server-started JSON / server-info.
- shutdown() now destroys all client sockets so the process exits even with an
open WebSocket.
- Watchdog check interval is configurable (BRAINSTORM_LIFECYCLE_CHECK_MS, default
60s) so the lifecycle can be tested without minute-long waits.
Adds lifecycle.test.js (configured timeout reported; idle shutdown exits despite
an open WS — teeth-verified; the start-server flag). Wires ws-protocol,
lifecycle, and stop-server suites into npm test.
Closes#1237
Refs #1689
stop-server.sh read server.pid and SIGKILL'd that PID with no checks. After a
reboot or PID wraparound the pid file can point at an unrelated, live process —
which we would then kill.
Verify the PID is actually our server (a running 'node ... server.cjs') before
signalling it. If ownership can't be proven, fail closed: remove the stale pid
file and report {status: stale_pid} without killing anything. Real servers still
stop ({status: stopped}); a missing pid file still reports not_running.
Adds stop-server.test.sh covering: an unrelated reused PID is left alone, a real
server is stopped, and a missing pid file.
Refs #1703
On macOS (and ExFAT/SMB volumes) the OS writes ._<name>.html sidecar files
holding binary resource-fork metadata. These end with .html, so they passed the
content filter and could be picked as the newest screen — serving binary garbage
to the browser instead of the mockup — or fetched via /files/.
Skip dotfiles (leading '.') at all four sites that list or serve content:
getNewestScreen, the /files/ endpoint, the known-files seed, and the fs.watch
handler. Tests cover serving (/ and /files/) and the watch path (a ._ file must
not trigger a reload).
Refs #950
