Skip to content

fix(chatgpt-review): remove duplicate SELECTORS.responseActions key breaking clipboard copy - #665

Merged
BorisTyshkevich merged 1 commit into
mainfrom
fix/chatgpt-review-duplicate-selector-key
Aug 10, 2026
Merged

fix(chatgpt-review): remove duplicate SELECTORS.responseActions key breaking clipboard copy#665
BorisTyshkevich merged 1 commit into
mainfrom
fix/chatgpt-review-duplicate-selector-key

Conversation

@BorisTyshkevich

Copy link
Copy Markdown
Collaborator

Summary

Found live during the same /ship 639 dry-run smoke test as PR #664, after
that fix's diagnostics made the real captured response text visible for the
first time.

SELECTORS (skills/chatgpt-review/scripts/lib/browser.mjs) defined
responseActions twice: the properly-scoped, commented
['[role="group"][aria-label="Response actions" i]'] (added alongside
copyLatestAssistantMarkdown), immediately shadowed a few lines later by a
pre-existing, unrelated ['button[aria-label*="Good response"]', ...]
definition that was never removed when the new one was added. JS object
literals silently let the second key win with no error and no lint warning
(this repo has no ESLint config anywhere), so copyLatestAssistantMarkdown
has been querying the wrong selector since the day it was introduced,
always returning null, and every caller silently fell back to plain
.innerText() — which strips Markdown heading syntax.

This is the real root cause of the recurring
"invalid_response: The delimited plan is not complete Markdown with a heading" failures in plan-author mode across this project's history,
including 3 reproductions in a single live dry run today. Confirmed live
against a real open ChatGPT tab (chrome-devtools MCP):

  • the correct selector matched 3 [role="group"] elements, the last one
    containing exactly 1 real copy button;
  • the shadowing selector's own text ("Good response"/"Bad response")
    matched zero elements in ChatGPT's current UI — doubly dead, wrong
    and stale.

Fix

Delete the shadowing duplicate key. Added two regression tests:

  • SELECTORS.responseActions equals the scoped group selector directly
    (not the stale button selector) — independent of mechanics.
  • A static parse of the SELECTORS object literal's source text asserting
    no top-level key repeats — can't become vacuously self-consistent the way
    the existing mock-based tests (which read SELECTORS.responseActions[0]
    to build their own mocks) are.

Test plan

  • node --test skills/chatgpt-review/tests/*.test.mjs — 64/64 pass (62 + 2 new)
  • Root cause confirmed live against a real ChatGPT tab via chrome-devtools MCP

🤖 Generated with Claude Code

https://claude.ai/code/session_01LwFPT465eDJqYcRa8HGNLz

…reaking clipboard copy

SELECTORS defined `responseActions` twice: the properly-scoped, commented
`['[role="group"][aria-label="Response actions" i]']` (added alongside
copyLatestAssistantMarkdown), immediately shadowed a few lines later by a
pre-existing, unrelated `['button[aria-label*="Good response"]', ...]`
definition that was never removed. JS object literals silently let the
second key win with no error and no lint warning (this repo has no ESLint
config anywhere), so copyLatestAssistantMarkdown has been querying the wrong
selector since the day it was introduced, always returning null, and every
caller silently fell back to plain innerText() -- which strips Markdown
heading syntax.

This is the real root cause of the recurring "invalid_response: The
delimited plan is not complete Markdown with a heading" failures in
plan-author mode across this project's history, including 3 reproductions
in a single live dry run today (issue #639). Confirmed live against a real
open ChatGPT tab: the correct selector matched 3 groups with a real copy
button in the last one; the shadowing selector's own text ("Good response"/
"Bad response") matched zero elements in ChatGPT's current UI -- doubly
dead, wrong AND stale.

Fix: delete the shadowing duplicate key. Added two regression tests: one
asserting SELECTORS.responseActions holds the scoped group selector (not
the stale button selector) directly, and one statically parsing the
SELECTORS object literal's source text to assert no top-level key repeats --
independent of whatever runtime value the constant happens to hold, so it
can't become vacuously self-consistent the way the existing mock-based
tests are.

64/64 chatgpt-review unit tests pass (62 + 2 new).
@BorisTyshkevich
BorisTyshkevich merged commit 33cb52f into main Aug 10, 2026
8 checks passed
@BorisTyshkevich
BorisTyshkevich deleted the fix/chatgpt-review-duplicate-selector-key branch August 10, 2026 10:16
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