Repository navigation
[3310 by Claude Opus 5.5] Report privacy-safe browser checkpoints when signed RPM acceptance fails - #3311
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…o says which browser journey and checkpoint was reached last, plus a failure category from a fixed list. All the related test suites pass. I did not run the real container acceptance, and the cause of the failure in run 39559433992 is still unknown — nothing here fixes it.
What a failure looks like now
```json
{"signedRpmAcceptanceFailure":{"phase":"installed-acceptance","outcome":"exit","exitCode":1,
"stage":"browser-acceptance","browserJourney":"dashboard-profile-manager",
"browserCheckpoint":"paired","browserFailure":"dashboard-connected-timeout"}}
```
How it works
- Fixed lists: a new file, `apps/desktop/scripts/packaged-acceptance-checkpoints.mjs`, holds the ordered checkpoint names and the failure categories. The checkpoints cover setup, every visual journey (launched / prepared / captured / completed), the pairing, connection and socket steps on the dashboard, each CSP reload phase, every installed-RPM pairing journey, and each final-evidence gate.
- Browser runner (`run-packaged-acceptance.mjs`): it prints one checkpoint line at each of those points. If it fails, it prints one category line and re-throws the original error unchanged. The category is chosen by matching the start of the runner's own fixed error texts (or a Playwright timeout/closed-page error); anything else is `unclassified`. The error text itself is never printed. Lines are only printed for the installed-RPM run.
- Host side (`signed-rpm-acceptance-diagnostics.mjs`): it only accepts checkpoint and category lines while the container is in its browser stage. A line must be complete, at most 128 characters, and an exact match from the lists. A checkpoint is only accepted if it comes after the last one, so repeated or out-of-order lines are ignored. Only the first category counts, and nothing is accepted after it. Moving to a later container stage drops the browser fields.
- Rebuilding the diagnostic: the final diagnostic is rebuilt from allowed fields only, and journey and checkpoint are checked as a pair.
What stays the same
- The `signed-rpm-acceptance.json` success file is unchanged, and the new fields never reach it.
- Cleanup order, ownership refusal, and the rule that the original failure comes before cleanup failures are unchanged.
- No retries, timeouts, product assertions, test credentials or workflow/publication rules were changed.
Tests
- New `packaged-acceptance-checkpoints.test.mjs`:
- the checkpoint order matches the runner's journey lists, and every checkpoint the runner prints is on the list.
- a secret-bearing error turns into a category only.
- repeated, out-of-order, malformed, oversized and secret-bearing lines are ignored.
- `signed-rpm-acceptance-diagnostics.test.mjs`:
- checkpoint lines are only tracked during the browser stage, including a marker split across two writes and lines that arrive too early or too late.
- a known browser failure is reported correctly through a fake Docker that also prints secrets.
- the original failure stays first when cleanup also fails, every cleanup step still runs, and no evidence is written.
- a successful run that prints checkpoints produces evidence identical to a plain run.
- `packaged-acceptance-csp-reload.test.mjs`: some of its tests run parts of the runner's code directly, so they needed a stand-in for the new reporter. Those tests now also check which checkpoints are emitted on success and on the renderer-error failure. No assertions were removed.
Results
- `node --test` on the browser-approval, signed-RPM evidence, diagnostics, Linux preview workflow, checkpoints, CSP reload, acceptance-artifacts and signed-RPM context suites: 126 tests, 122 passed, 0 failed, 4 skipped. The skips are platform-specific tests (macOS-only or POSIX singleton cases) that `node --test` skips on this host.
- `npx tsx --test` on the account, stats and socket suites: 26/26 passed.
- `eslint --max-warnings 0` on the changed files is clean.
- Against an unchanged copy of the code, the 4 new diagnostics tests that check browser fields fail and the existing tests pass, so they do catch the missing behaviour.
I also updated `docs/docs/operations/linux-rpm-signing.md` to describe the new fields and add the new test file to the focused test command. No visual previews were made, since this only changes CI diagnostics.
|
/review codex-astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex-astraOverall EvaluationMerge-ready within the visible scope, conditional on the pending checks passing. The PR adds bounded browser progress diagnostics while preserving acceptance failures and authoritative success evidence. ✅ Privacy-safe fields — Both reporter and host validate checkpoint identifiers and failure categories against fixed lists; raw error messages never enter the structured diagnostic. ✅ Failure isolation — Browser markers cannot authorize success, and host cleanup failures remain secondary to the original acceptance failure. ✅ Focused regression coverage — Tests cover fragmented markers, stale checkpoints, secret-bearing input, cleanup precedence, and unchanged success-evidence fields. The supplied issue requirements are truncated, so this assessment covers the visible objective and changed-code invariants. No commands were run, as requested. The authoritative check summary reports no failures and five pending checks. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Strengthen checkpoint coverage assertionsIn S2: 🟢 Classify additional CSP failuresThe classifier in ScoreThe implementation is focused, preserves fail-closed behavior, and includes meaningful privacy and propagation tests. Static review found no merge blocker; pending checks and the unexecuted real-container path remain validation limits. Score: 8/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
AI Implementation Summary
Closes #3310
Branch:
3310/claude-opus-5-5-report-privacy-safe-brows-20261021-1252-2iaCommits: ✅ Changes committed (cf4df27)
AI Processing Completed
Execution Details:
Summary:
When signed-RPM browser acceptance fails, the diagnostic line now also says which browser journey and checkpoint was reached last, plus a failure category from a fixed list. All the related test suites pass. I did not run the real container acceptance, and the cause of the failure in run 39559433992 is still unknown — nothing here fixes it.
What a failure looks like now
{"signedRpmAcceptanceFailure":{"phase":"installed-acceptance","outcome":"exit","exitCode":1, "stage":"browser-acceptance","browserJourney":"dashboard-profile-manager", "browserCheckpoint":"paired","browserFailure":"dashboard-connected-timeout"}}How it works
apps/desktop/scripts/packaged-acceptance-checkpoints.mjs, holds the ordered checkpoint names and the failure categories. The checkpoints cover setup, every visual journey (launched / prepared / captured / completed), the pairing, connection and socket steps on the dashboard, each CSP reload phase, every installed-RPM pairing journey, and each final-evidence gate.run-packaged-acceptance.mjs): it prints one checkpoint line at each of those points. If it fails, it prints one category line and re-throws the original error unchanged. The category is chosen by matching the start of the runner's own fixed error texts (or a Playwright timeout/closed-page error); anything else isunclassified. The error text itself is never printed. Lines are only printed for the installed-RPM run.signed-rpm-acceptance-diagnostics.mjs): it only accepts checkpoint and category lines while the container is in its browser stage. A line must be complete, at most 128 characters, and an exact match from the lists. A checkpoint is only accepted if it comes after the last one, so repeated or out-of-order lines are ignored. Only the first category counts, and nothing is accepted after it. Moving to a later container stage drops the browser fields.What stays the same
signed-rpm-acceptance.jsonsuccess file is unchanged, and the new fields never reach it.Tests
packaged-acceptance-checkpoints.test.mjs:signed-rpm-acceptance-diagnostics.test.mjs:packaged-acceptance-csp-reload.test.mjs: some of its tests run parts of the runner's code directly, so they needed a stand-in for the new reporter. Those tests now also check which checkpoints are emitted on success and on the renderer-error failure. No assertions were removed.Results
node --teston the browser-approval, signed-RPM evidence, diagnostics, Linux preview workflow, checkpoints, CSP reload, acceptance-artifacts and signed-RPM context suites: 126 tests, 122 passed, 0 failed, 4 skipped. The skips are platform-specific tests (macOS-only or POSIX singleton cases) thatnode --testskips on this host.npx tsx --teston the account, stats and socket suites: 26/26 passed.eslint --max-warnings 0on the changed files is clean.I also updated
docs/docs/operations/linux-rpm-signing.mdto describe the new fields and add the new test file to the focused test command. No visual previews were made, since this only changes CI diagnostics.Detailed Logs:
b211eca3-3ab3-4e54-82e7-22fa2c434130Log files stored at:
/usr/src/app/logs/executions/issue-3310-2026-10-21T13-03-52-929Z-conversation.json/usr/src/app/logs/executions/issue-3310-2026-10-21T13-03-52-929Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3310.
💡 Need changes?
Comment on this PR to request refinements — the AI agent monitors comments and will update the implementation based on your feedback. Keep iterating until you're satisfied!