Skip to content

recognize common test-runner wrappers in the agent hook - #18

Open
dxnnystyles wants to merge 3 commits into
argahv:mainfrom
dxnnystyles:fix/test-runner-wrappers
Open

dxnnystyles wants to merge 3 commits into
argahv:mainfrom
dxnnystyles:fix/test-runner-wrappers

Conversation

@dxnnystyles

@dxnnystyles dxnnystyles commented Oct 22, 2026 •

Copy link
Copy Markdown

What this changes

recognizes common test-runner wrappers in the agent hook while keeping command text out of published events. closes #5.

Why

the agent heartbeat previously missed package-manager and environment wrappers around test commands, and separators inside quoted shell arguments could produce false positives.

How it was tested

  • swift test passes — not run; this change is limited to the Python hook

  • python3 -I Tests/test_bridge.py passes — 24 tests

  • bash Scripts/build.sh builds — not run; no macOS app code changed

  • If I added, moved or removed a Swift file: I ran python3 Scripts/generate_project.py and committed NotchFlow.xcodeproj — not applicable; no Swift files changed

  • Checked on a Mac — not run

  • Focused hook regression — 1 test passed; covers wrappers, continuations, heredocs, ANSI-C quotes, malformed tails, false positives, and command privacy

  • Python 3.9 grammar parsing passed for both changed files

  • git diff --check passed

Accessibility and motion

  • New controls have accessibility labels and work from the keyboard
  • Animations respect Reduce Motion; materials respect Reduce Transparency
  • Not applicable

Privacy and security

  • No new network requests, permissions or entitlements
  • No tokens, personal paths or private data in code, tests, screenshots or logs

@argahv argahv left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this. The quote-aware splitting is a real improvement, and the new negatives are good ones. A few things before it can go in:

  • npx playwright test is no longer detected (Scripts/notchflow.py:319). npx and bunx are now stripped before the TEST_RUNNERS match, so the command becomes playwright test, which isn't in the list (only npx playwright test is). It was labelled Tests before this change. Adding "playwright test" to TEST_RUNNERS, with a test case, fixes it.
  • One unbalanced quote turns off detection for the whole command (command_segments, line 281). Returning [] for malformed quoting means an apostrophe in a heredoc body or a comment, or the [:4000] cut landing inside a string, hides every test command, including ones before it:
    cat > t.py <<'EOF'
    # it's a test
    EOF
    pytest t.py
    The old line split found pytest t.py; this returns False. Returning the segments completed before the unterminated quote would keep your echo 'literal; pytest -q negative and still catch these.
  • Dead entries in TEST_RUNNERS. Because the package manager and npx are stripped first, "npm test", "npm run test", "pnpm test", "pnpm run test", "yarn test", "npx jest", "npx vitest" and "npx playwright test" can't match any more. Removing them keeps the list honest; it's how the Playwright case slipped through.
  • Optional, while you're in there: pnpm -F web test (-F is --filter), pnpm -C app test (-C is --dir), bun run test and env CI=1 time pytest are still missed.

Happy to pair on any of it. Just ask here.

@dxnnystyles

Copy link
Copy Markdown
Author

thanks for the careful review — restored npx playwright test, kept completed commands when a trailing quote is malformed, and removed the unreachable runner entries. i also covered pnpm -F/-C, bun run, and env CI=1 time while i was in there.

@argahv argahv left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the quick turnaround. Playwright is back, the dead entries are gone, and the extra wrappers all work. I ran the branch against main over a batch of commands, and two kinds of multi-line command that main labels Tests aren't labelled any more:

  • Line continuations (Scripts/notchflow.py:262). A \ at the end of a line is kept as an escaped character, so shlex turns the backslash-newline into a word of its own and the real command no longer starts the segment. These are Tests on main and not on this branch:
    cd app && \
      npm test
    CI=1 \
      pytest -q
    The same goes for npm run build && \ then npm test, and make lint; \ then cargo test. Outside single quotes the shell drops a backslash-newline pair entirely, so skipping both characters in command_segments instead of keeping them fixes it.
  • Apostrophes in a heredoc body (command_segments). My earlier example only passes because its apostrophe sits on a # line, which is skipped as a comment. Anywhere else in the body, the apostrophe opens a quote that never closes, and every command after it is dropped:
    cat > src/sum.test.js <<'EOF'
    // it's the sum test
    EOF
    npx jest
    A README heredoc containing What's new followed by go test ./... fails the same way. Skipping heredoc bodies would cover these: after <<WORD, <<'WORD' or <<-WORD, ignore lines up to the one that is just WORD. Please add both shapes to the runs list.
  • Small: echo $'it\'s'; pytest regressed too. Inside $'…' a backslash escapes the quote, so it needs its own quote state.
  • Small: assertNotIn(command, json.dumps(events)) can't fail for the multi-line commands, because json.dumps writes the newline as \n. Also checking json.dumps(command)[1:-1] would make it a real check.

@dxnnystyles

Copy link
Copy Markdown
Author

thanks for the extra cases! I added the line-continuation, heredoc, and ANSI-C quote regressions, restored Playwright after wrapper stripping, and removed unreachable runner entries. the privacy assertion now checks the JSON-escaped command; the focused test and full 24-test bridge suite pass.

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.

Recognise common test-runner wrappers in the agent hook (npm run test, uv run pytest, env CI=1 …)

2 participants