Guard the specs against platform-specific modifier chords

[wormeyman]

Part 1 of #208. Not Closes: that issue's part 2, the display panel editor's uncovered surface, is untouched here.

What it is

tests/spec-modifier-keys.test.ts reads every file in tests/*.spec.ts and flags Control+A and its four siblings (C, V, X, Z) where they appear as an argument to keyboard.press.

On macOS Control+A is the emacs "beginning of line" binding, not select-all. So nothing gets selected, the typed text lands beside the old text, and the assertion receives something like "New textOld text". That is #197, fixed in 402fbe31.

Why a text scan and not a test that drives something

The bug passes on Linux, and every runner this project has is ubuntu-latest: vp test and all four e2e shards.

When #208 was written, the reason this reached the default branch was that CI never ran Playwright at all. #223 and #225 have since closed that half, and it does not help here. A browser suite that gates every PR on four sharded runners will go green on this forever. The only thing that can see it is something reading the spec source.

The rule, which is not "never write Control"

A blanket ban would be wrong, and the measured usage says so. There are two classes:

• A chord reaching the app, through actions.ts, stays Control. type ModifierKey = 'Control' | 'Shift' | 'Alt' there, so the app has no Meta binding and Control is the only key that works.
• A chord reaching a focused DOM <input>, handled by the browser or the OS, wants ControlOrMeta, because that binding differs by platform.

A regex cannot tell those apart, so it flags both and the ALLOWLIST carries the distinction. An entry has to say which class it is and why. One entry today: chest-filters.spec.ts's Control+KeyZ, which drives the editor's own undo keybind with no DOM input focused.

Meta+ is flagged for the same five keys even though nothing uses it. It is the same mistake facing the other way.

Not flagged: keyboard.down('Control') / up('Control'), which four specs use to hold the modifier for the ctrl-drag copy gesture. That is a held modifier for a mouse gesture reaching the app's own handler, and it is correct as written in all of them.

Mutation-checked against the live instance

Dropping PR #222's tests/blueprint-grid-position.spec.ts into tests/ fails the guard, naming the real line:

1 spec chord(s) hardcode a platform modifier:

  blueprint-grid-position.spec.ts:97  Control+A

Removing it goes green again.

Two things the build turned up

It matches the .press( call form, not the bare text. The first draft matched the characters anywhere and immediately flagged display-panel-editor.spec.ts:160 - the five-line comment explaining this exact trap, sitting directly above its own corrected ControlOrMeta+A. Skipping comment lines does not work here, since this codebase writes block comments as bare indented lines with no leading asterisk. Matching the call form removes the whole question: prose can discuss the bug freely, and what gets flagged is the thing that actually runs.

The allowlist is checked in both directions. An entry matching nothing is a justification for code that is gone, and the next person reads a stale justification as a live one. Same membership check tests/entity-accessors.spec.ts uses on its fixture.

Why the class needs a guard rather than another comment

After 402fbe31 there is a five-line comment at display-panel-editor.spec.ts:159 explaining the whole trap, one line above the fix. A contributor then wrote Control+A in a new spec in a different file (#222, blueprint-grid-position.spec.ts:97). A comment at the scene of the last occurrence does not reach the next one.

Cost

Pure Node, so it runs under vp test inside checks in milliseconds rather than behind the browser job. playwright.config.ts pins a testMatch of spec files only, so Playwright does not also collect it.

vp check 0, vp test 184 passed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Mg2oe7JgL3xEzSoN5emTsd