test(observability-map): name the environment on the queues page failure logs - #4485
test(observability-map): name the environment on the queues page failure logs#44851stvamp wants to merge 4 commits into
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Observability mapAs of Nothing in this pull request moves the report any more. The findings an earlier push reported are gone. Report only, nothing here gates the merge. The rules and their reasons: internal-packages/observability-map/README.md. |
a681f21 to
1cd8d80
Compare
7ee2a98 to
05e9507
Compare
…ure logs Throwaway PR to verify the observability-map CI comment reports an improvement. Not for merge.
1cd8d80 to
d3ce17d
Compare
27cf0b0 to
6d9c167
Compare
Reverts the route edit and touches a non-route file, so the diff is non-empty but matches neither watched path while a findings comment exists. Tests that the workflow still starts and reconciles the comment.
|
Test complete. All four states of the delta rule confirmed on the real runner, plus the stale-comment gap this exposed (fixed in 042c9c3) and its reconcile path. Closing; branch deleted. |
… entry points (#4455) A static observability scorer for the webapp's route entry points, Lighthouse-style. The idea comes from evlog's `map` command, but that tool has no Remix adapter and checks for its own logging API, so the idea is ported rather than the tool. It scans all 427 loader/action entry points in `apps/webapp/app/routes` with the TypeScript compiler API and scores each against five checks: error-classification, auth-boundary, auth-scope, request-context and audit-trail. Current output on the real tree is **19/100** over 412 measured entry points. ``` cd internal-packages/observability-map pnpm exec tsx src/cli.ts # terminal report pnpm exec tsx src/cli.ts --json # machine output pnpm exec tsx src/cli.ts api/v1/token # one entry, per-check detail ``` The two findings at the top of the fix list are real: `/auth/sso` and `/api/v1/authorization-code` mint or exchange credentials unauthenticated, and `/_app/orgs/:organizationSlug/settings/team` resolves its org from a URL slug and gates each mutating branch on an RBAC check alone, which per `apps/webapp/CLAUDE.md` is not the tenant floor on self-hosted. Decisions worth knowing, all with the reasoning in the README: - The score started at 83 during development and fell to 19. Every drop was a perverse incentive being removed, not a regression: routes were being paid for having no error handling, two checks were reading the same fact, suppressing a failure raised the score, and a no-op `catch (e) { throw e }` was worth 50 points a route. - **A mutation corpus is the tool's main defence.** 44 entries apply semantics-preserving edits to a copy of the real route tree and assert the score cannot rise, per route as well as globally, because a mean can hide one route going up by taking another down. One entry runs as a live expected failure: `try { String(0); }` with a deciding catch is a known open hole worth 19 to 44, and it is disclosed rather than quietly excluded. - `audit-trail` and `request-context` are reported as headline figures rather than one finding repeated hundreds of times. Both still count in full where they should. - A cohort change moves the number without anything in the codebase getting better. Widening the sensitive cohort from 26 to 67 took the global from 15 to 19 with no webapp change at all, so the report prints per-check applicability and what the global would be without each one. CI: a report-only job posts a sticky comment when a PR moves the report, and says nothing when it does not. The package's own tests gate through `pr_checks.yml`. The diff-scoped merge gate is still deferred until the report has been used in anger. 524 tests plus the corpus. No runtime or dependency changes to anything that ships. <!-- GitButler Footer Boundary Top --> --- This is **part 1 of 4 in a stack** made with GitButler: - <kbd> 4 </kbd> #4485 - <kbd> 3 </kbd> #4484 - <kbd> 2 </kbd> #4483 - <kbd> 1 </kbd> #4455 👈 <!-- GitButler Footer Boundary Bottom -->
1 participant
Throwaway. Case 1 of 3 in an end-to-end check of the observability-map CI
comment: a real improvement should be reported as one.
The two logger.warn calls on the queues page log { error } with no tenant,
while environment is already in scope. Adding environmentId is exactly the
change the tool asks for.
Locally verified: that route goes 0 -> 50, request-context fail -> pass.
Global stays 19, because one route in 412 cannot move the rounded mean.
Do not merge. Will be closed once the CI behaviour is observed.
This is part 4 of 4 in a stack made with GitButler: