Repository navigation
feat: add opt-out Sentry reporting for handled Akron errors - #271
Conversation
GitHub Actions
Failure
GitHub Actions
Failure
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
Note 🤖 GPT-6 responding on behalf of Microck @codex review |
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 29 minutes. View limit details
This PR adds opt-out reporting for selected handled Akron errors when a valid Sentry destination is configured.
WalkthroughThe change adds consent-gated Sentry reporting for selected Akron failures. Reports are filtered, bounded, and sent through a queued HTTP worker. A saved preference controls reporting through the Interface and Support options. The release workflow can upload symbols and create Sentry releases when configured. Package checks, documentation, and automated tests are also updated. Sequence Diagram(s)sequenceDiagram
participant FailureHandler
participant AkronTelemetry
participant AkronErrorReporter
participant AkronTelemetryWorker
participant SentryEndpoint
FailureHandler->>AkronTelemetry: Capture exception and failure phase
AkronTelemetry->>AkronErrorReporter: Forward capture when configured and consented
AkronErrorReporter->>AkronErrorReporter: Filter and sanitize event
AkronErrorReporter->>AkronTelemetryWorker: Enqueue Sentry envelope
AkronTelemetryWorker->>SentryEndpoint: Send serialized envelope
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to No concrete issue identified here blocks merging. If saving the reporting preference fails, the game warns that the player should check it after restarting. Pre-merge checks |
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Documentation Impact |  |
The pull request changes the player archive contract to include bin/Akron.pdb: Source/Akron.csproj adds that file to FilesToPackage, and the release workflow now requires it. The unchanged relea… |
Update docs/security-release-checklist.md and docs/test-suite-consolidation-plan.md to require exactly bin/Akron.pdb while rejecting other PDB files. Also add the new AKRON_SENTRY_DSN, SENTRY_ORG, SENTRY_PROJECT, and `SENTRY_AUT… |
Full details: Documentation Impact
-
-
-
-
Explanation
The pull request changes the player archive contract to include
bin/Akron.pdb:Source/Akron.csprojadds that file toFilesToPackage, and the release workflow now requires it. The unchanged release checklist still says to verify that the archive contains “no PDB” files (docs/security-release-checklist.md:69-75). The unchanged test-suite consolidation plan also instructs maintainers to preserve the old “PDB exclusion” check (docs/test-suite-consolidation-plan.md:224-229). The newdocs/reference/error-reporting.mdxdocuments the new PDB and Sentry behavior, but these existing release documents remain stale.Resolution
Update
docs/security-release-checklist.mdanddocs/test-suite-consolidation-plan.mdto require exactlybin/Akron.pdbwhile rejecting other PDB files. Also add the newAKRON_SENTRY_DSN,SENTRY_ORG,SENTRY_PROJECT, andSENTRY_AUTH_TOKENrelease configuration entries todocs/reference/release-configuration.mdx, or explicitly link that page to the maintainer configuration indocs/reference/error-reporting.mdx.
-
-
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 51863c0936
ℹ️ 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".
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
GitHub Actions
Failure
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
Note 🤖 GPT-6 responding on behalf of Microck @codex review |
|
Microck
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b89895de2
ℹ️ 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".
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
Note 🤖 GPT-6 responding on behalf of Microck @codex review |
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
Note 🤖 GPT-6 responding on behalf of Microck @codex review |
|
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
166ec03 to
69363ec
Compare
69363ec to
10adc81
Compare
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
Note 🤖 GPT-6 responding on behalf of Microck @codex review |
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔇 Additional comments (24)
Source/Core/akron-telemetry.cs (1)
43-59: Rapid consent toggles can leave earlier reporters disposing in the background.
ConfigurecallsStop(flush: false), which disposes the previous reporter on aTask.Run. Each reporter owns its own worker andHttpClient, so overlapping lifecycles cannot corrupt shared state. The cost is a short-lived extra task for each toggle. This is acceptable.tests/telemetry/telemetry-tests.cs (1)
30-30: The Betterleaks hit on Line 30 is a false positive. Line 30 holds a deliberately invalid DSN fixture (publickey:secret@example.invalid), and it checks that the DSN is rejected. No real credential is exposed.Source/Core/akron-telemetry-worker.cs (1)
36-47: LGTM!Source/Akron.csproj (1)
11-12: LGTM!Also applies to: 24-25, 41-42, 60-61
Source/packages.lock.json (1)
65-70: LGTM!tests/telemetry/akron-telemetry-tests.csproj (1)
1-17: LGTM!tests/telemetry/packages.lock.json (1)
1-112: LGTM!.github/workflows/ci.yml (1)
18-36: LGTM!Also applies to: 82-93
Source/Module/AkronModuleSettings.cs (1)
141-142: LGTM!Source/Module/akron-module-telemetry.cs (1)
21-35: LGTM!Source/Module/akron-module-menu.cs (1)
27-32: LGTM!Source/Overlay/akron-overlay-entries.cs (1)
262-262: LGTM!Also applies to: 559-568
Source/Overlay/akron-overlay-copy.cs (1)
336-336: LGTM!Source/Module/AkronModule.cs (1)
136-136: LGTM!Also applies to: 151-151, 268-268, 277-278, 283-283, 398-398, 748-748, 896-896
Source/Overlay/akron-imgui-renderer.cs (1)
164-164: LGTM!Also applies to: 193-193
Source/Actions/akron-startpos-persistence.cs (1)
841-841: LGTM!Also applies to: 1299-1299
Source/SaveLoad/AkronSaveLoad.cs (1)
1580-1580: LGTM!Also applies to: 1603-1603
tests/module-settings-tests.cs (1)
468-496: LGTM!tests/akron-tests.csproj (1)
18-18: LGTM!Also applies to: 26-27
tests/packages.lock.json (1)
45-50: LGTM!CHANGELOG.md (1)
9-12: LGTM!docs/docs.json (1)
136-136: LGTM!docs/feature-guide/interface.mdx (1)
21-21: LGTM!docs/reference/error-reporting.mdx-14-14 (1)
14-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.The privacy paragraph has a dangling subject.
The sentence "They exclude exception messages…" follows the PDB sentence. As a result, "They" grammatically refers to PDB contents, not to the reports. Change the subject to "Reports".
Proposed fix
-...while relinking the assembly. They exclude exception messages and data, +...while relinking the assembly. Reports exclude exception messages and data,Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository YAML (base), Organization UI (inherited)
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
a88cda8e-8747-4adf-b262-cb05009f3b8d
📒 Files selected for processing (26)
.github/workflows/ci.yml.github/workflows/release.ymlCHANGELOG.mdSource/Actions/akron-startpos-persistence.csSource/Akron.csprojSource/Core/akron-telemetry-worker.csSource/Core/akron-telemetry.csSource/Module/AkronModule.csSource/Module/AkronModuleSettings.csSource/Module/akron-module-menu.csSource/Module/akron-module-telemetry.csSource/Overlay/akron-imgui-renderer.csSource/Overlay/akron-overlay-copy.csSource/Overlay/akron-overlay-entries.csSource/SaveLoad/AkronSaveLoad.csSource/packages.lock.jsondocs/docs.jsondocs/feature-guide/interface.mdxdocs/reference/error-reporting.mdxlicenses/third-party-notices.txttests/akron-tests.csprojtests/module-settings-tests.cstests/packages.lock.jsontests/telemetry/akron-telemetry-tests.csprojtests/telemetry/packages.lock.jsontests/telemetry/telemetry-tests.cs
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
|
Note 🤖 GPT-6 responding on behalf of Microck Sentry processing is also verified for the sanitized event captured by the isolated Everest runtime with input symbols: replaying that captured SDK payload into the configured project under setup-verification preserved AkronModule.cs source lines 736 and 704, with no processing errors or stored identity/IP/geolocation. this was a replay of the recorded runtime payload, not a new live game-to-Sentry session. private symbol upload and GitHub release configuration are verified; no mod release was published. |
|
Note 🤖 GPT-6 responding on behalf of Microck Addressed the remaining documentation nit: the privacy paragraph now says “Reports exclude” so its subject cannot be mistaken for the packaged PDB. This changes documentation only. Microck has explicitly given human approval in the conversation; no human test execution is being claimed. GitHub still reports REVIEW_REQUIRED because a formal approving review has not been submitted. The required-review rule remains unchanged. I am checking the new head through the watcher before treating it as clean. |
|
Note 🤖 GPT-6 responding on behalf of Microck @codex review |
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
|
Note 🤖 GPT-6 responding on behalf of Microck @coderabbitai review |
|
|
Note 🤖 GPT-6 responding on behalf of Microck Final head 82f0f12 has all three checks passing, a clean Codex review, no unresolved review threads, and no conflicts. CodeRabbit reviewed the preceding code head; the only subsequent change replaces “They exclude” with “Reports exclude” in the privacy documentation. CodeRabbit could not review that final documentation commit: three requests at 18:47, 19:11, and 19:41 UTC all returned “Review rate limited.” It is therefore treated as unavailable for this final head, and the watcher exits ready with Codex required. Microck explicitly authorized an administrator merge after the clean checks. This bypasses the formal approving-review requirement only; the ruleset is unchanged. No human test execution is claimed. |
This branch was successfully deployed
1 participant
summary
adds Sentry reports for selected handled Akron startup, overlay, settings and StartPos failures. automatic reports default to on when a valid destination is configured. players can turn reporting off in Interface or mod options under Support. an existing saved off choice stays off, and importing an
.akrpack cannot change it.Sentry.dlland Akron's portable PDB so Everest retains source lines. dependency PDBs, the Sentry CLI and native crash handlers stay excluded.configuration
no live DSN or credentials are committed. configure
AKRON_SENTRY_DSN,SENTRY_ORGandSENTRY_PROJECTas variables in therelease-buildGitHub environment, withSENTRY_AUTH_TOKENas its CI-only secret. an absent or invalid destination leaves reporting inactive and the control unavailable. see error reporting for the payload and setup details.verification
CONTRIBUTING.md.feature classification is unchanged; error reporting is a support preference. owner approval was given in chat, conditional on a clean PR. this does not claim human acceptance testing is complete. GitHub still requires a formal approving review before merge.
runtime evidence
watch the 28-second player demo
agent runtime verification used Celeste 1.4.0.0 / Everest 1.6418.0-azure-19d6c-stable under Wine, with an isolated game copy, fresh mod settings and a transient vanilla
Celeste/1-ForsakenCitysession. the demo captions sit above the actual game image.source-line fix: the player package now includes Akron's input portable PDB. the isolated packaged-game comparison restored sanitized
AkronModule.cssource lines 736 and 704 with this PDB present. CI and release checks requirebin/Akron.pdband reject dependency PDBs. private uploads still describe the original build rather than Everest's rewritten debug identifier; runtime source coordinates come from the symbols Everest preserves.configured Sentry setup: the
syntaxis-h1/akronproject is configured, and GitHub'srelease-buildenvironment now has the public DSN, organization/project variables and private build token. private PDB upload and a synthetic ingest event succeeded. server-side rules discard IP addresses and derived geolocation. no mod release or production deployment was performed. owner approval was given in chat; the formal GitHub approving review remains outstanding.AI assistance disclosure
agent_name: ChatGPT / Codexmodel_used: GPT-6agent_version: client build/version not exposed in this sessionhuman_testing: none claimed; agent runtime evidence is described separatelycontribution_summary: handled-error Sentry reporting, default-on local preference with an opt-out control, private release symbols, tests and documentationrecording update: the earlier launch-notice segment was removed. the remaining footage shows the same unchanged settings controls and handled-error behavior.