Wide events audit 2026-07-14 - #136
Closed
fgilio wants to merge 1 commit into
Closed
Conversation
Automated weekly wide-events logging audit artifact. Findings summary: 1 critical, 2 warning, 2 info. The deterministic Pest run could not complete this session (composer install could not authenticate against github.com) so the C1-C9 checks are unverified. The report also flags two updater listeners that hardcode rfa.outcome = completed in finally and one comment-write path that has no owner canonical info event.
📝 WalkthroughWalkthroughAdds a dated Wide Events audit report documenting the audit scope, a deterministic check failure, five findings, confirmed clean areas, and residual verification risks. ChangesWide Events audit
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@reports/wide-events/2026-07-14.md`:
- Line 5: Update the report’s “Every warning payload…” and “Storage stays local
and bounded” findings to explicitly describe inventory-based observations rather
than runtime guarantees, while preserving the existing note that deterministic
checks were blocked by Composer authentication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8cff08ba-85dc-4fe9-be94-ebbd52124789
📒 Files selected for processing (1)
reports/wide-events/2026-07-14.md
|
|
||
| ## Summary | ||
|
|
||
| - Deterministic checks: fail (environment could not install composer dependencies) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='reports/wide-events/2026-07-14.md'
echo '--- line count ---'
wc -l "$FILE"
echo '--- around lines 1-90 ---'
sed -n '1,90p' "$FILE" | cat -n
echo '--- grep for deterministic/assertions/warning payload/storage ---'
rg -n 'deterministic|assertions|warning payload|storage|retention|local' "$FILE"Repository: fgilio/rfa
Length of output: 12359
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE='reports/wide-events/2026-07-14.md'
sed -n '1,90p' "$FILE" | nl -baRepository: fgilio/rfa
Length of output: 188
Scope the clean findings more narrowly. The summary already says the deterministic checks were blocked by Composer auth, so “Every warning payload…” and “Storage stays local and bounded” should read as inventory-based observations, not runtime guarantees, until the missing test and dynamic-path coverage runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@reports/wide-events/2026-07-14.md` at line 5, Update the report’s “Every
warning payload…” and “Storage stays local and bounded” findings to explicitly
describe inventory-based observations rather than runtime guarantees, while
preserving the existing note that deterministic checks were blocked by Composer
authentication.
![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: e8818770ee
ℹ️ 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".
| - `app/Providers/NativeAppServiceProvider.php:161` — the `UpdateError` listener emits both a diagnostic `Log::error('updater.failed', ...)` with sanitized `message` and `stack` fields, and a canonical `Log::info('updater.failed')` from `finally` with `rfa.outcome = 'error'`. This is the exact pattern required by A13. | ||
| - All warning and error payloads across the audited files route raw exception text through `LogSanitizer::summary()` (e.g. `GetProjectStatusAction.php:32`, `LoadFileDiffAction.php:56`) or restrict themselves to safe fields (`exit_code`, `error_class`, `path`). No bare `$e->getMessage()`, `$e->getTraceAsString()`, or `$e->stderr` reached a payload value in the reviewed files. | ||
| - Every warning payload includes a stable `reason` code (`diff_process_failed`, `commit_range_list_failed`, `exclude_append_failed`, and so on) — C5 in the checklist. | ||
| - `config/logging.php:25` resolves the default channel to `daily`, whose `driver` is `daily` with `days` bounded by `LOG_DAILY_DAYS` (7). The remote channels (`slack`, `papertrail`, `syslog`, `errorlog`, `stderr`) exist as Laravel scaffolding but are never referenced by the default channel or the default stack expansion. Storage stays local and bounded. |
There was a problem hiding this comment.
This clean-area conclusion only checks the fallback in config/logging.php, but normal setup copies .env.example into .env (composer.json post-root and README), and .env.example:18-21 sets LOG_CHANNEL=stack, LOG_STACK=single, and LOG_LEVEL=debug. In that environment config('logging.default') resolves to stack/single, not bounded daily, so the audit masks the wide-events Storage issue it is supposed to surface.
Useful? React with 👍 / 👎.
fgilio
added a commit
that referenced
this pull request
Jul 21, 2026
…and context comment writes (#139) * feat(logging): canonical events for deep links, menu clicks, and context comment writes Address the warning-level findings from the 2026-07-14 and 2026-07-21 wide-events audits (PRs #136/#137): - HandleDeepLink now owns a canonical deeplink.opened event emitted on every terminal outcome (completed / rejected / error) with rfa.mode, rfa.route, rfa.path_hash, and project context. - HandleMenuItemClicked now owns a canonical menu.item.clicked event with rfa.menu_id, outcome (completed / cancelled / skipped / error), and duration; sub-handlers report their outcome instead of void. - The UpdateAvailable / UpdateDownloaded listeners no longer hardcode rfa.outcome = completed in finally: a catch now flips the outcome to error (with rfa.error_class and a stable reason) before rethrowing. - Context-page comment writes own a canonical context.comment.written event (completed / rejected / skipped / error); the rejected branch keeps its diagnostic warning, whose payload keys move to snake_case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3Jts3UrUJYfdJFjvqnJZy * fix(logging): truthful outcomes for swallowed picker, registration, and scan failures Address Codex review on #139: OpenProjectFromPathAction and OpenRepositoryDialogAction collapse distinct failure modes into null, which the new canonical events mislabeled. - OpenProjectFromPathAction's swallowed-Throwable branch now marks the owner Context (rfa.reason = project_registration_failed + rfa.error_class), so HandleDeepLink records error instead of rejected/not_a_project for database or filesystem failures. - OpenRepositoryDialogAction marks not_a_git_repository picks and registration failures via Context; HandleMenuItemClicked maps a null project to cancelled / rejected / error accordingly. - scan-directory now reports partial (with repos_found / registered / already_tracked / failed counts) when any registration failed, and completed only for clean scans. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3Jts3UrUJYfdJFjvqnJZy * fix(logging): skipRender on no-op comment writes, cover skipped/error outcomes Address CodeRabbit nitpicks on #139: - createComment now calls skipRender() on the rejected and skipped early returns — neither touches $comments, so the render was pure waste. - Tighten the rejected-branch test to assert the warning event name and payload shape, and add coverage for the skipped (no comment produced) and error (rethrown Throwable) canonical outcomes. The suggestion to hoist the canonical-event try/catch/finally into a shared helper is intentionally skipped: the wide-events skill's owner pattern is written inline at each logging owner by design, matching softRefresh() and the updater listeners. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3Jts3UrUJYfdJFjvqnJZy * refactor(logging): exception-derived rejection reasons and clearer null-project mapping Apply the fgilio-review branch audit: - OpenProjectFromPathAction now marks its own null causes via Context (path_not_found, not_a_git_repository), so the deep-link and dialog flows share one reason vocabulary instead of the deep-link relabeling not-a-repo as not_a_project (kept only as the addIf fallback). - Rename outcomeForDismissedPicker to outcomeForNullProject: it maps all three null-project outcomes, not just dismissal. - Hoist the twice-computed mode and path hash in HandleDeepLink into locals shared by Context and the diagnostic breadcrumb. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3Jts3UrUJYfdJFjvqnJZy * fix(logging): report used_cached_project only when the cache resolved the project A stale cached id falls back to the picker, but is_int($cachedId) still read true, so both the canonical menu.item.clicked event and the diagnostic breadcrumb attributed picker-based opens to the cache. Track whether cache resolution actually produced the project instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M3Jts3UrUJYfdJFjvqnJZy --------- Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
Automated weekly wide-events logging audit.
Artifact:
reports/wide-events/2026-07-14.mdThis PR is human-gated. It contains the report only.
Generated by Claude Code
Summary by CodeRabbit