Skip to content

fix(watch-pr): exclude fork PRs from child stack walk and sanitize terminal lines - #1194

Open
mangeshraut712 wants to merge 1 commit into
cursor:mainfrom
mangeshraut712:fix/watch-pr-fork-children-sanitize
Open

mangeshraut712 wants to merge 1 commit into
cursor:mainfrom
mangeshraut712:fix/watch-pr-fork-children-sanitize

Conversation

@mangeshraut712

Copy link
Copy Markdown

Summary

This resolves both issues reported in #1193:

  1. Exclude fork PRs from child stack discovery (github.ts):
    orderStack previously built the children map from every open pull request (for (const pr of open)), allowing cross-repository (fork) PRs whose base ref matched an upstream branch to be indexed as children. Although byHead excluded fork PRs for downwards resolution, a fork PR targeting an upstream stack branch would still be traversed and pushed to up.

    • Filtered out pr.isCrossRepository when building the children map (if (pr.isCrossRepository) continue;).
    • Added defense-in-depth guard in visit: if (child.isCrossRepository) continue;.
    • Added regression test does not attach a fork PR as a child of a stack branch in github.test.ts.
  2. Sanitize terminal blocker output (render.ts):
    Review thread paths, author logins, and comment bodies rendered in blocker summaries could contain ANSI escape sequences (e.g. \x1b[2K, \r, or formatting sequences) and control characters.

    • Introduced sanitizeText to strip ANSI escape sequences (/\\x1b\\[[0-9;]*[a-zA-Z]/g) and C0/C1 control characters (/[\\x00-\\x1f\\x7f-\\x9f]/g).
    • Applied sanitization to path, authorLogin, and bodyLine in threadLine.
    • Sanitized bodyLine before slicing to at most 180 characters.
    • Added regression test sanitizes review thread paths, authors, and comment bodies in blocker output in cli.test.ts.
  3. Harness Generation:
    Regenerated harness/pstack/claude-code/ tree via node scripts/generate-pstack-harness-trees.mjs.

Verification

  • tsc --project watch-pr/tsconfig.json --noEmit --strict: 0 errors.
  • bun test watch-pr: 56/56 passing (160 expectations).
  • Full script test suite: 63/63 passing.
  • node scripts/generate-pstack-harness-trees.mjs --check: up to date.
  • node scripts/validate-plugins.mjs: all plugins validated successfully.
  • node scripts/check-pstack-docs.mjs: 0 problems.
  • git diff --check: 0 errors.
  • Independent code review completed and approved.

Fixes #1193

…rminal lines

Exclude cross-repository PRs from children in orderStack so fork PRs
cannot be attached to upstream stack branches during stack discovery.

Sanitize review comment body text, file paths, and author logins in
threadLine to strip ANSI escape sequences and C0/C1 control characters
before rendering to the terminal.

Fixes cursor#1193

Signed-off-by: Mangesh Raut <[email protected]>
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.

pstack watch-pr: fork PRs can join a discovered stack; review comment text printed unsanitized

1 participant