iter-145: route click JS exceptions through the JSON error envelope - #185
Merged
Conversation
click.rs's two remaining bare-stderr-then-AppError::Exit(1) paths (a genuine JS exception during click, at the top-level attempt and inside the frame-scan retry) now route through AppError::User via a shared classify_click_exception() helper, matching eval.rs's iter-141 Theme E handling. The audit also found scroll.rs's "scroll until" timeout using the same bare-stderr-then-Exit(1) bypass; reclassified to AppError::Timeout. Adds a new xtask gate, check-error-envelope-paths, that fails CI when an eprintln! in crates/ff-rdp-cli/src/commands/ is immediately followed by a bare AppError::Exit(N) bypassing the envelope — wired into check-iteration-ready and the discipline CI job. The remaining ~40 eprintln! sites audited are legitimately stderr (progress/debug/warn-and-continue); annotating them with justification comments is deferred to iteration-148-stderr-path-annotations.md per iter-145's own escape hatch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ac-fidelity-check.sh only parses the first physical line of a checkbox; the backtick-quoted error_type/Internal evidence for unit_145_click_exception_maps_to_user_error_type was on the wrapped continuation line, so the check reported no evidence despite the test existing. Reflow to one line — no content change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… test
The aggregator's sub-check count moved to 11/11 in this PR (iter-145 added
check-error-envelope-paths) and the assertion condition was updated to
match, but the panic message text still said "[{i}/10]" — found during
/review-pr local pass.
…test flake iter-145 added a sixth live_145_* test file reusing the shared LiveFirefox teardown in tests/common/mod.rs, so Theme A's fix covers it automatically — no AC changes needed, just a note confirming coverage and flagging the firefox_with_daemon duplication as a follow-up candidate. Also record a one-off flaky unit-test race hit during iter-145's review (specs::types::tests::resolve_slot_longstring_grip_fetches_full_value racing transport.rs's FRAME_CAP_LOCK-guarded tests) — a different bug class than this plan's live-Firefox themes, noted so it isn't lost rather than folded into Theme A/B/C.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
click.rs's two remaining bare-stderr-then-AppError::Exit(1)paths (a genuine JS exception thrown during click, at the top-level attempt and inside the frame-scan retry) now route through the standard JSON error envelope (error_type: "User") via a sharedclassify_click_exception()helper — the same treatmenteval.rsgot in iter-141 Theme E.eprintln!undercrates/ff-rdp-cli/src/commands/(~42 sites). Found one more instance of the same bug class inscroll.rs's "scroll until" timeout (nowAppError::Timeout, matching every other timeout in the codebase). The remaining ~40 sites are legitimately stderr (progress/debug/warn-and-continue) — annotating them is deferred tokb/iterations/iteration-148-stderr-path-annotations.mdper iter-145's own escape hatch.check-error-envelope-paths, that fails when aneprintln!incrates/ff-rdp-cli/src/commands/is immediately followed by a bareAppError::Exit(N)bypass — wired intocheck-iteration-ready(now 11/11 sub-checks) and thedisciplineCI job, so this defect class can't silently reappear.CONTRIBUTING.md's gate list updated; iteration plan ACs ticked with evidence.Test plan
cargo fmt/cargo clippy --workspace --all-targets -- -D warnings/cargo test --workspace -qall cleancargo run -p xtask -- check-iteration-ready --plan kb/iterations/iteration-145-error-envelope-completeness.md --base origin/main→ 11/11 PASS (run withFF_RDP_LIVE_TESTS=1socheck-dogfood-scriptexecutes)FF_RDP_LIVE_TESTS=1 cargo test -p ff-rdp-cli --test live live_145 -- --include-ignored):live_145_click_js_exception_envelope,live_145_click_frame_scan_js_exception_envelope,live_145_click_element_not_found_unchanged— all passunit_145_click_exception_maps_to_user_error_type+classify_click_exceptioncoverage inclick.rscargo run -p xtask -- check-error-envelope-pathspasses on this branch (and its own unit tests cover the bug shape it's designed to catch, including a miniature of the pre-fixclick.rspattern)🤖 Generated with Claude Code## Claims vs code
<generated 2026-08-12T08:05:46Z by ralph-loop>
a→ ✅ matched in diffAppError::Exit→ ✅ matched in diffAppError::User→ ✅ matched in diffAppError::Timeout→ ✅ matched in diffframe-scan→ ✅ matched in diff