feat(pilot): gate risky interact clicks with irreversible-action hook#1096
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d93d26b10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function classifyBrowserActionRisk(input: BrowserActionRiskInput): BrowserActionRisk { | ||
| const text = normalize(`${input.action} ${input.labelText ?? ''}`); | ||
| const evidence = CRITICAL_ACTION_WORDS.filter((word) => text.includes(word)); | ||
| const negated = SAFE_NEGATIONS.some((word) => text.includes(word)); |
There was a problem hiding this comment.
The negation check uses text.includes(word) for entries like "view" and "read", so labels containing larger words such as "review and pay" are treated as safe ("review" contains "view") even when critical evidence ("pay") is present. In pilot+contract-runtime mode this bypasses the irreversible-action hook for common payment/submit phrasing, which defeats the new safety gate for real high-risk clicks.
Useful? React with 👍 / 👎.
| const text = normalize(`${input.action} ${input.labelText ?? ''}`); | ||
| const evidence = CRITICAL_ACTION_WORDS.filter((word) => text.includes(word)); | ||
| const negated = SAFE_NEGATIONS.some((word) => text.includes(word)); | ||
| const mutatingClick = ['click', 'double_click'].includes(normalize(input.action)); |
There was a problem hiding this comment.
normalize(input.action) converts underscores to spaces, but the mutating-action allowlist still checks for "double_click". As a result double_click becomes "double click" and never qualifies as mutating, so destructive double-click actions are never classified as critical and are not gated by the irreversible-action hook.
Useful? React with 👍 / 👎.
Irreversible browser actions need a deterministic hook point before mutation, but ordinary OpenChrome clicks must remain fast and unchanged. This guards only keyword-classified critical click labels and only activates the contract gate when the pilot runtime is explicitly enabled. Constraint: Risky-action protection must not slow or alter normal OpenChrome interactions unless the pilot contract runtime is explicitly enabled. Rejected: Prompt for confirmation on every click | too noisy and harmful for long-running automation. Confidence: high Scope-risk: moderate Directive: Expand critical keyword coverage only with tests and keep default runtime pass-through behavior. Tested: npm test -- --runTestsByPath tests/harness/irreversible-action.test.ts; npm run build:src; npm run lint:tier Not-tested: Live irreversible-action denial against a real OpenChrome browser page. Co-authored-by: OmX <omx@oh-my-codex.dev>
shaun0927
force-pushed
the
feat/1003-irreversible-confirmation
branch
from
2d93d26 to
46c62b8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46c62b8509
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const axGuard = await guardIrreversibleBrowserAction( | ||
| { | ||
| toolName: 'interact', | ||
| action, | ||
| labelText: `${query} ${ax.role} ${ax.name}`, | ||
| pageUrl: page.url(), | ||
| }, |
There was a problem hiding this comment.
The new irreversible-action gate is only wired into the AX/CSS discovery branches, while the existing ref fast-path and nodeRef branch still execute page.mouse.click(...) directly. In pilot + contract-runtime mode, callers can bypass the safety hook by first resolving a destructive control and then invoking interact with ref/nodeRef, so high-risk actions like account deletion are not consistently blocked.
Useful? React with 👍 / 👎.
| ), | ||
| ), | ||
| ); | ||
| if (cssGuard.blocked) return cssGuard.blocked; |
There was a problem hiding this comment.
When cssGuard.blocked is returned, the function exits before cleanupTags(page, DISCOVERY_TAG) runs, leaving __elDiscIdx markers from discoverElements in the DOM. Those stale markers are later read by getTaggedElementRect(...), so a subsequent interact can resolve the wrong element index and click the wrong target after a denied risky action.
Useful? React with 👍 / 👎.
Address Codex P1 + P2 review findings: - P1: switch substring `includes` to a word-boundary regex match so negation tokens like "view"/"read" no longer mask critical evidence in phrases such as "Review and pay". - P2: align the mutating-click allowlist with the post-normalize form so `double_click` (normalized to "double click") is still treated as mutating and gated by the irreversible-action hook. Adds regression tests for both scenarios. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba1b6b475b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| toolName: 'interact', | ||
| action, | ||
| labelText: `${query} ${ax.role} ${ax.name}`, |
There was a problem hiding this comment.
Including ${query} in labelText lets the caller suppress irreversible-action gating by adding any SAFE_NEGATIONS token (for example cancel, view, or read) to the query while still targeting a destructive control, because classifyBrowserActionRisk() treats any negation match as non-critical. This creates a straightforward bypass of the new safety hook in both AX and CSS interact paths whenever query text and element text are mixed.
Useful? React with 👍 / 👎.
| ), | ||
| ), | ||
| ); | ||
| if (cssGuard.blocked) return cssGuard.blocked; |
There was a problem hiding this comment.
Returning immediately on cssGuard.blocked skips cleanupTags(page, DISCOVERY_TAG), leaving discovery marker properties in the page DOM for blocked critical clicks. Since this path comes after discoverElements()/tagging and the function explicitly documents cleanup to avoid stale properties, blocked actions now regress into persistent DOM pollution that can interfere with later element discovery behavior.
Useful? React with 👍 / 👎.
1 participant
Progress / Review status
Auto-refreshed 2026-05-13 — owner comments cleaned up to reduce review noise.
feat/1003-irreversible-confirmation→developba1b6b4— fix(pilot): match risk keywords on word boundariesOwner comment cleanup: 1 issue + 0 inline review comments deleted. Outstanding feedback from automated/external reviewers above is unchanged.
Summary
interactAX/CSS click paths with the pilot contract runtime only when a critical action is detected andOPENCHROME_PILOT+OPENCHROME_CONTRACT_RUNTIMEare enabled.Direction / necessity check
interactclicks to the existing before-irreversible contract gate.Validation
npm test -- --runTestsByPath tests/harness/irreversible-action.test.tsnpm run build:srcnpm run lint:tierPost-merge live verification with OpenChrome
Delete account; verify the click proceeds exactly as before.OPENCHROME_PILOT=1 OPENCHROME_CONTRACT_RUNTIME=1and register/enable a before-irreversible hook that denies critical actions.Delete accountfixture throughinteract; verify the result isisError: true, includes_irreversibleAction.status, action evidence, and does not mutate the page.Cancel delete dialog; verify it is not gated.hoveron destructive text; verify it is not gated.Closes #1003