Repository navigation
[3376 by Claude Opus 5.5] Intermittent desktop packaged-acceptance failure (Linux x64), seen twice in 24h: PR #3368 run 40798090873 (passed on rerun) and nightly run 40837471417 job 'Nightly desktop package / Validate unsigned linux-x64 package'. Check current upstream main first. - #3378
integry merged 3 commits into
Conversation
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…acing, and I fixed it in the acceptance harness without loosening the strict gate. The new tests pass, but I couldn't run `npm run desktop:acceptance` here, so it isn't yet proven against a real packaged run. Upstream main: I couldn't fetch, because the worktree's git directory is read-only. The local `origin/main` ref is the same commit as this branch (`700ce8273`), and nothing there already covers this. Why it's (b): - Intended behaviour: after activation, main switches the CSP and schedules a renderer reload. Until the new document loads, the old document's requests to the new endpoint are blocked on purpose (see the comment at `apps/desktop/src/main.ts:1993`). The new document then does the real current-user sync. The classifier already has an expected category for exactly this denial during `csp-reload-pairing`. - Why it flaked: the classifier only exempts a console CSP error if Chromium's CDP `Audits.issueAdded` event confirms a matching blocked fetch. That event arrives asynchronously, and on the failing runs the reload replaced the old document first. So the console error had no matching request, and the classifier correctly refused it. That fits the failing run's `requestFailures: []` and the single "Connecting to … violates" line. - Why I didn't change the product: reloading synchronously wouldn't help, because the old document keeps running until the new one loads. Fencing the renderer's requests would mean changing `propr-ui`, and this task is desktop-only. The change is in `apps/desktop/scripts/packaged-acceptance-csp-reload.mjs`: - New evidence record: when the old main-frame document is replaced by a new main-frame document in the same phase, the CDP observer records a `document-replaced-before-audit` entry. It does this only for fetches that were seen exactly once and never got an Audits issue. Closing the app, subframe documents and repeated fetches produce nothing. - Narrow acceptance: the classifier accepts that record only for the current-user `csp-connect-policy` console error, and only when there's no Audits-confirmed denial. Everything else is still required: exact URL, page and phase, the converged reload evidence, and the existing count limits. Demo-mode, "Fetch API cannot load" errors, other phases and unrelated errors still fail. - Diagnostics: the new record appears in the diagnostics as `failureClass: 'document-replaced'`. No retries were added, and the unexpected-error check is unchanged. Regression tests: I added three tests to `packaged-acceptance-csp-reload.test.mjs`. They use the fake CDP session and play events in the failing order: fetch, then document replacement, then no Audits issue. The failing nightly shape now counts as one expected CSP transition with zero unexpected errors, and an extra unrelated error still fails. A late Audits issue still works, and there are negative cases for each condition above. All three fail on the original code. Checks: - Desktop unit tests (`npm run desktop:test`): 1355 passed, 0 failed, 56 skipped. - `eslint --max-warnings 0 apps/desktop/`: clean. - `npm run desktop:acceptance` failed straight away. There's no packaged `out/propr-desktop-linux-x64` build, and the container has no Xvfb or keyring to run it. The real-Chromium test in that file is also skipped here, because it only runs in the provisioned CI job. A Linux x64 packaged-acceptance run in CI will be the real confirmation. This isn't a visual change, so there are no previews. I also updated my notes on this flake with the fix.
|
CI failed: Prove stock Chromium existing-target axe boundary Please investigate and fix this CI failure.
Failure evidence
|
|
CI failed: Finalize unsigned validation checksums Please investigate and fix this CI failure.
Failure evidence
|
|
✅ Applied the requested follow-up changes in commit 78229cd Processed 2 comments:
Summary of ChangesI fixed both CI failures; they had one cause. The real-Chromium test in the axe-boundary job showed a soundness bug in the previous commit. Locally, against the same Chromium as CI (151.0.7922.34), that test now passes. Why the job failed: the failing test was Why "Finalize unsigned validation checksums" failed: this was only a knock-on effect. The packaging matrix depends on the axe-boundary job, so it was skipped, and this job requires the matrix to succeed. Nothing in it needed changing. The fix (in
New tests:
Checks:
As before, I couldn't run the full packaged acceptance ( 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…mium test in the axe-boundary job showed a soundness bug in the previous commit. Locally, against the same Chromium as CI (151.0.7922.34), that test now passes. I fixed both CI failures; they had one cause. The real-Chromium test in the axe-boundary job showed a soundness bug in the previous commit. Locally, against the same Chromium as CI (151.0.7922.34), that test now passes. **Why the job failed:** the failing test was `exact observer preserves native denied evidence across real Chromium navigation`, with `6 !== 4` at `apps/desktop/scripts/packaged-acceptance-csp-reload.test.mjs:607`. The previous commit recorded "document replaced before its CSP report arrived" for every single fetch in the old page that had no denial report. That included fetches CSP had allowed and that got a normal response. So navigating from the allowed page back to the denied page added two false entries. In practice, that could have excused a console CSP error using a request that was never blocked. **Why "Finalize unsigned validation checksums" failed:** this was only a knock-on effect. The packaging matrix depends on the axe-boundary job, so it was skipped, and this job requires the matrix to succeed. Nothing in it needed changing. **The fix** (in `packaged-acceptance-csp-reload.mjs`): - The fetch wrapper the harness injects into the page now also logs when a watched fetch gets a response. The caller still gets the original promise, rejections included. - The observer marks those fetches as allowed, even if the test phase changed before the response came back. Allowed fetches never produce a "document replaced" entry. - The narrow acceptance rule from the previous commit is unchanged, so a genuinely blocked current-user fetch still counts. **New tests:** - An allowed fetch followed by a page replacement produces nothing, whether the response arrives in the same phase or a later one. - The wrapper logs the response only for watched URLs, and leaves rejected fetches unreported and still rejected. **Checks:** - The real-Chromium test (`PROPR_DESKTOP_REAL_CHROMIUM_BOUNDARY=1`, the CI command): 51 passed, 0 failed. - The same test against the previous commit fails here with `6 !== 4`, so the CI failure reproduces locally. - Desktop unit tests (`npm run desktop:test`): 1355 passed, 0 failed, 56 skipped. - `eslint --max-warnings 0` on the two changed files: clean. As before, I couldn't run the full packaged acceptance (`npm run desktop:acceptance`) here, so a CI run is still needed to confirm the original flake fix on a real Linux build. No previews, since nothing visible changed. PR: #3378 Comment by: @propr-dev[bot] (ID: 6495148508) Comment by: @propr-dev[bot] (ID: 6495149624) Model: Claude Opus 5.5
|
CI failed: Validate unsigned linux-x64 package Please investigate and fix this CI failure.
Failure evidence
|
|
✅ Applied the requested follow-up changes in commit 172a5eb Summary of ChangesI made a likely fix for this failure, but I can't prove it: the CI log doesn't say why the step failed, and I couldn't run the packaged lifecycle here. The failure isn't in the code this PR changed, and the real check is the next CI run. What failed: the earlier step, the packaged acceptance run this PR fixes, passed in this run on linux-x64. The step that failed was the later native install/deep-link lifecycle step, with Why I think it's the ZIP:
The likely cause: that ZIP second-instance launch was the only warm dispatch started without Changes:
Checks: the lifecycle suite (44/44), If 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
… the CI log doesn't say why the step failed, and I couldn't run the packaged lifecycle here. The failure isn't in the code this PR changed, and the real check is the next CI run. I made a likely fix for this failure, but I can't prove it: the CI log doesn't say why the step failed, and I couldn't run the packaged lifecycle here. The failure isn't in the code this PR changed, and the real check is the next CI run. **What failed:** the earlier step, the packaged acceptance run this PR fixes, passed in this run on linux-x64. The step that failed was the later native install/deep-link lifecycle step, with `Native lifecycle operation failed [stage:PROTOCOL_DISPATCH]` and no other detail. That's by design: the script hides child output, URLs and profile paths, so the cause has to be worked out from the code. **Why I think it's the ZIP:** - If a registration command for the deb or rpm package had failed, the error would name a more specific stage (`PROTOCOL_LAUNCH` and similar). A bare `PROTOCOL_DISPATCH` means the error came from launching a second app instance directly, which is how the ZIP has to be dispatched. It could also come from writing the package's launcher file, but that's less likely. - On Linux the packages run in the order deb, rpm, zip, and the step failed about 55 seconds in, which fits the ZIP being last. **The likely cause:** that ZIP second-instance launch was the only warm dispatch started without `--disable-gpu --password-store=gnome-libsecret`. Every other dispatch passes them, and so does the rewritten launcher line used for the deb and rpm. They were added everywhere in commit `45e8ce344` except here. Missing them under the CI's virtual display could plausibly make that instance miss its 15-second exit deadline now and then. That would fit this signature appearing once, only on x64: linux-arm64 passed the same step on the same commit, and none of the 10 earlier failed desktop runs I checked had it. **Changes:** - `apps/desktop/scripts/test-native-artifact-lifecycle.mjs`: the protocol-dispatch helper now takes the launch arguments and passes them to the ZIP launch, and the lifecycle passes them in. I also gave the helper an injectable dispatch function so the ZIP path can be tested. - `apps/desktop/scripts/test-native-artifact-lifecycle.test.mjs`: a new case checks that the ZIP launch gets both arguments and never registers an OS launcher. With the old call restored, it fails as expected. **Checks:** the lifecycle suite (44/44), `linux-preview-workflow.test.mjs` (17/17) and `release-workflow.test.ts` (29/29) pass, and eslint shows no warnings on both files. If `PROTOCOL_DISPATCH` comes back even with the arguments passed, treat it as a runner flake and rerun. There's nothing visual to preview. PR: #3378 Comment by: @propr-dev[bot] (ID: 6495494400) Model: Claude Opus 5.5
|
/review |
1 similar comment
|
/review |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationReady to merge within the supplied scope. The PR adds a narrowly constrained fallback for missing CSP audit evidence and preserves Linux launch arguments during ZIP protocol dispatch. No merge-blocking defect was identified by static review. ✅ Bounded classification — Replacement evidence applies only to the matching current-user connect-policy diagnostic, with page, phase, URL, uniqueness, and convergence checks. ✅ Negative coverage — Added tests cover fulfilled and duplicate fetches, cross-phase replacement, subframes, application closure, and unrelated errors. ✅ Launch consistency — Linux ZIP dispatch now receives the launch context arguments, with a focused forwarding assertion. The authoritative current-head status reports 29 passed checks and no failures or pending checks, including Linux x64 package validation. No commands were run. The supplied issue body is truncated, so objective validation is limited to its visible requirements. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Clarify the observer contractUpdate the introductory comments in ScoreThe changed behavior is tightly scoped, has substantial negative coverage, and passes the reported current-head checks. Static review found no required corrections. Score: 9/10 🤖 Review Details
Copy and edit to choose records from this review: 🤖 Review by ProPR |
AI Implementation Summary
Closes #3376
Branch:
3376/claude-opus-5-5-intermittent-desktop-pack-20261029-1103-efeCommits: ✅ Changes committed (bb9fe12)
AI Processing Completed
Execution Details:
Summary:
The flake is option (b), a gap in the classifier. The product isn't racing, and I fixed it in the acceptance harness without loosening the strict gate. The new tests pass, but I couldn't run
npm run desktop:acceptancehere, so it isn't yet proven against a real packaged run.Upstream main: I couldn't fetch, because the worktree's git directory is read-only. The local
origin/mainref is the same commit as this branch (700ce8273), and nothing there already covers this.Why it's (b):
apps/desktop/src/main.ts:1993). The new document then does the real current-user sync. The classifier already has an expected category for exactly this denial duringcsp-reload-pairing.Audits.issueAddedevent confirms a matching blocked fetch. That event arrives asynchronously, and on the failing runs the reload replaced the old document first. So the console error had no matching request, and the classifier correctly refused it. That fits the failing run'srequestFailures: []and the single "Connecting to … violates" line.propr-ui, and this task is desktop-only.The change is in
apps/desktop/scripts/packaged-acceptance-csp-reload.mjs:document-replaced-before-auditentry. It does this only for fetches that were seen exactly once and never got an Audits issue. Closing the app, subframe documents and repeated fetches produce nothing.csp-connect-policyconsole error, and only when there's no Audits-confirmed denial. Everything else is still required: exact URL, page and phase, the converged reload evidence, and the existing count limits. Demo-mode, "Fetch API cannot load" errors, other phases and unrelated errors still fail.failureClass: 'document-replaced'. No retries were added, and the unexpected-error check is unchanged.Regression tests: I added three tests to
packaged-acceptance-csp-reload.test.mjs. They use the fake CDP session and play events in the failing order: fetch, then document replacement, then no Audits issue. The failing nightly shape now counts as one expected CSP transition with zero unexpected errors, and an extra unrelated error still fails. A late Audits issue still works, and there are negative cases for each condition above. All three fail on the original code.Checks:
npm run desktop:test): 1355 passed, 0 failed, 56 skipped.eslint --max-warnings 0 apps/desktop/: clean.npm run desktop:acceptancefailed straight away. There's no packagedout/propr-desktop-linux-x64build, and the container has no Xvfb or keyring to run it. The real-Chromium test in that file is also skipped here, because it only runs in the provisioned CI job. A Linux x64 packaged-acceptance run in CI will be the real confirmation.This isn't a visual change, so there are no previews. I also updated my notes on this flake with the fix.
Detailed Logs:
5a7adb7a-c568-45fb-9051-85b8e2478f0aLog files stored at:
/usr/src/app/logs/executions/issue-3376-2026-10-29T11-13-25-335Z-conversation.json/usr/src/app/logs/executions/issue-3376-2026-10-29T11-13-25-335Z-output.txtLatest Conversation Messages
This PR was created automatically by ProPR after processing issue #3376.
💡 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!