Skip to content

fix(browser): add bounded prompt commit diagnostics - #331

Closed
cheong-yi wants to merge 1 commit into
steipete:mainfrom
cheong-yi:fix/prompt-commit-diagnostics
Closed

fix(browser): add bounded prompt commit diagnostics#331
cheong-yi wants to merge 1 commit into
steipete:mainfrom
cheong-yi:fix/prompt-commit-diagnostics

Conversation

@cheong-yi

Copy link
Copy Markdown

Summary

  • add opt-in prompt-commit phase diagnostics gated by ORACLE_PROMPT_COMMIT_DIAGNOSTICS=1
  • report bounded candidate-selection, trusted-click dispatch, immediate-probe, commit-accepted, and commit-timeout states
  • keep prompt, probe, and session values out of diagnostics, and isolate diagnostic failures from submission behavior
  • add focused coverage for accepted, timeout, rejection, logger-failure, redaction, and fallback paths

Why

Oracle's prompt composer is the only layer that can observe the causal sequence between candidate selection, trusted click dispatch, the immediate post-click probe, and final commit acceptance or timeout. Bounded diagnostics make failed browser submissions attributable without exposing prompt or raw probe content.

Validation

Run with Node v24.15.0:

  • git diff --check
  • ./node_modules/.bin/oxfmt --check src/browser/actions/promptComposer.ts tests/browser/promptComposer.test.ts
  • ./node_modules/.bin/tsc --noEmit
  • ./node_modules/.bin/oxlint — 0 warnings, 0 errors
  • ./node_modules/.bin/vitest run tests/browser/promptComposer.test.ts --no-file-parallelism — 18/18 passed

Overlap and sequencing

At preparation time, #322 and #325 were both open, conflicting with current main, and modified both changed paths. This commit is based only on steipete/oracle:main at f13347973bf6e21e6ef45c0088183021dc2c9e20; it does not incorporate either PR head. If either PR lands or main changes, this branch should receive a fresh semantic conflict review before integration.

Changelog

No new changelog bullet is included. This is an opt-in diagnostic refinement of the existing redacted prompt-commit diagnostics noted in the 0.15.1 changelog; maintainers can request a separate release-note adjustment if preferred.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 7:39 AM ET / 11:39 UTC.

Summary
The PR adds opt-in, redacted diagnostic events around browser prompt candidate selection, click dispatch, immediate probing, commit acceptance, and timeout handling.

Reproducibility: no. The PR describes prior browser submission attribution gaps, but it provides no current-main failing reproduction or redacted real-browser before/after transcript.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 2 files affected; 73 production lines added, 2 removed. The runtime change is focused on the browser prompt composer, limiting the functional review surface.
  • Regression coverage: 417 test lines added. The PR exercises multiple diagnostic outcomes, but those tests remain supplemental to real browser behavior proof.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post a redacted real browser or terminal transcript with ORACLE_PROMPT_COMMIT_DIAGNOSTICS=1 enabled that shows event order and the resulting submission outcome.
  • After adding proof, update the PR body for a fresh review; if it does not trigger, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR supplies automated checks and focused unit tests only; before merge, provide redacted after-fix browser or terminal evidence showing diagnostic output while a real prompt submission still completes or times out as reported. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Automated tests do not demonstrate that the added post-click CDP evaluation and awaited diagnostic flow leave a real browser prompt submission unchanged.
  • [P1] The new environment variable and JSON event names may become an operator-facing diagnostic contract without maintainer agreement on their long-term support and documentation level.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the diagnostics strictly opt-in and redacted, obtain a redacted real browser run showing both emitted events and unchanged submission behavior, then explicitly accept or narrow the environment-variable and event-format contract.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The remaining blocker is contributor-provided real browser proof plus a maintainer decision on the new diagnostic configuration contract, not a narrow automated repair.

Maintainer decision needed

  • Question: Should Oracle support ORACLE_PROMPT_COMMIT_DIAGNOSTICS=1 and its structured event names as a maintained browser-debug interface?
  • Rationale: The patch is technically narrow, but choosing a durable operator configuration and log schema is a product and supportability decision rather than a mechanical correctness fix.
  • Likely owner: steipete — Repository ownership and the browser diagnostic surface make this the best available routing candidate.
  • Options:
    • Adopt the bounded diagnostic contract (recommended): Keep the opt-in variable and stable redacted event schema after real-browser proof confirms it does not perturb prompt submission.
    • Keep diagnostics internal: Request a narrower implementation that reuses existing debug facilities without adding a new supported environment-variable interface.

Security
Cleared: The proposed diagnostic payload is bounded to static state labels, adds no dependency or permission surface, and explicitly excludes prompt, probe, and session values.

Review details

Best possible solution:

Keep the diagnostics strictly opt-in and redacted, obtain a redacted real browser run showing both emitted events and unchanged submission behavior, then explicitly accept or narrow the environment-variable and event-format contract.

Do we have a high-confidence way to reproduce the issue?

No. The PR describes prior browser submission attribution gaps, but it provides no current-main failing reproduction or redacted real-browser before/after transcript.

Is this the best way to solve the issue?

Unclear. The redacted, isolated implementation is a plausible approach, but maintainers should first decide whether this new environment-variable and event-schema surface is the desired supported solution.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against f13347973bf6.

Label changes

Label changes:

  • add P2: This is a bounded browser reliability and observability improvement without evidence of an urgent current-user outage.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR supplies automated checks and focused unit tests only; before merge, provide redacted after-fix browser or terminal evidence showing diagnostic output while a real prompt submission still completes or times out as reported. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded browser reliability and observability improvement without evidence of an urgent current-user outage.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR supplies automated checks and focused unit tests only; before merge, provide redacted after-fix browser or terminal evidence showing diagnostic output while a real prompt submission still completes or times out as reported. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Production diagnostic scope: The branch adds an opt-in ORACLE_PROMPT_COMMIT_DIAGNOSTICS=1 path that emits JSON containing only phase, sequence, status, and action kind, while its error paths explicitly avoid affecting submission behavior. (src/browser/actions/promptComposer.ts:23, 6afda9641c5e)
  • Focused coverage: The branch adds 417 test lines covering accepted, timeout, rejected probe, protocol exception, logger failure, redaction, and fallback paths for the new diagnostic flow. (tests/browser/promptComposer.test.ts:324, 6afda9641c5e)
  • Existing feature history: Current-main history already contains prompt-commit diagnostic work in the same production file, including commit 08c346a for browser submission debug logging, so this PR extends an active diagnostic surface rather than creating an unrelated subsystem. (src/browser/actions/promptComposer.ts:23, 08c346a)
  • Proof gap: The PR body reports formatter, typecheck, lint, and focused Vitest success, but it does not provide a redacted after-fix browser run or terminal transcript showing emitted diagnostics alongside an accepted or timed-out real submission. (6afda9641c5e)

Likely related people:

  • steipete: The repository owner is the most appropriate decision owner for accepting a permanent diagnostic environment variable and log-event contract on the browser submission path. (role: repository owner and recent browser-area contributor; confidence: medium; commits: f13347973bf6; files: src/browser/actions/promptComposer.ts)
  • LeoLin990405: Current file history associates this contributor with the earlier merged browser submission diagnostic work that this PR extends. (role: introduced adjacent diagnostic behavior; confidence: medium; commits: 08c346a; files: src/browser/actions/promptComposer.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete

Copy link
Copy Markdown
Owner

Thanks for the careful bounded diagnostics work. I am closing this rather than requesting changes because adding a new opt-in environment contract and permanent browser diagnostic vocabulary is a product and observability-policy choice, not a high-confidence bug fix. The current wave is limited to reproduced behavior fixes without new design surface. This is queued for Peter to decide whether Oracle should expose this diagnostic mode and which phase names and retention guarantees become supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants