## Findings addressed - **Report bot edited the wrong comment.** The comment-lookup step matched on the marker body text with no author predicate, so it would silently PATCH a human's comment that happened to quote the marker (GitHub gates comment editing on write access, not authorship, so it never 403'd). Now constrained to `.user.login == "github-actions[bot]"`, the same identity `helm-prerelease.yml` already pins. - **A required check asserted facts about the whole webapp namespace.** `webappSymbols.test.ts` asserted that nobody anywhere in `apps/webapp` (walking locals, params, object keys) declares names like `createJWT`/`updateEnvVars`, so an unrelated PR naming a local variable failed a required check with a message pointing at nothing. Those negative self-tests move onto a package-owned fixture tree; the positive resolution assertions stay required (their absence rotted the tool before) but now name the list to edit. - **The suite ran twice on shared paths.** `obsmap` and `internal` path filters shared four generic paths (`package.json`, both lockfiles, `pr_checks.yml`), so any lockfile bump ran the observability-map suite in both jobs. Dropped from `obsmap` (where `internal` already covers them). The test that should have caught it only checked the package's own source path; it now asserts the two filters' path intersection is empty. - **PR-comment footer** reworded: it said the report gates nothing, which is true of the report but misled now that the tool's test suite does gate webapp PRs. Names both failure directions and where to read the rules. - **Nightly corpus** comment corrected (stale entry count; the failure-notification gap is documented, not silently implied). ## Review Two adversarial reviewers ran over the diff; both findings were verified and fixed: a hollow fixture assertion (a shared name satisfied either walker branch — now one name per declaration form, revert-confirmed) and a filter-intersection test that could be fooled by apostrophes in comment prose (now strips comment lines first). Full package suite green (877 passed), typecheck and format clean.
@internal/observability-map
Scores every webapp entry point on whether it could explain itself during an incident, and prints
the ones worth fixing. An entry point is a Remix loader or action under
apps/webapp/app/routes, 427 of them today.
The score is 19 out of 100. It is low because the webapp does not attach tenant identity to its failures: 11 of the 412 measured entry points name an environment, project, organization or user on a failure path. Everything else, when it breaks at 3am, gives you the route and the request id and nothing about whose request it was.
Every figure here comes from a run against the tree as it stands. How the scanner reaches a verdict, and what it refuses to decide, is in INTERNALS.md.
Running it
pnpm --filter @internal/observability-map run map # the whole tree
pnpm --filter @internal/observability-map run map --json # same, as JSON on stdout
pnpm --filter @internal/observability-map run map /api/v1/token # one route, with its check results
The whole-tree run also writes observability-map.json at the repo root, which --no-write
suppresses. Single-route mode takes either the route path the report prints (/api/v1/token) or the
file name (api.v1.token.ts). An exact match wins over the routes it is a prefix of, and an
ambiguous prefix warns and names the alternatives rather than picking one.
CI
A pull request touching apps/webapp/app/routes or this package gets a sticky comment scanning head
against the tip of the base branch, with the score, what changed, and the current fix list. Every
comment names the head commit it was rendered for, as a link to the compare range, because the
comment is edited in place across pushes and otherwise says nothing about which push it reflects. It
is report-only: nothing in the comment or its score fails the build or blocks a merge. Separately,
this package's test suite runs as a required check, on pull requests touching apps/webapp/app and
on any other package through the internal job, and fails when a symbol the tool references stops
resolving in one of the three trees it reads (apps/webapp/app, packages/plugins/src,
internal-packages/rbac/src), or when the first route with an anticipated sensitive segment lands.
Each failure names the list to update (src/webappSymbols.test.ts).
See .github/workflows/observability-map.yml.
The workflow runs on every pull request and applies the path list as a gate inside the job rather
than as a paths: filter on the trigger. GitHub evaluates one of those per workflow, so a pull
request whose diff stops matching never starts the workflow at all, and the comment an earlier push
left then stands for ever showing findings that are no longer in the diff. The case that matters is
a pull request touching a route and other files whose author reverts the route change and keeps the
rest, which still has a diff and still does not match. So a pull request with a comment and nothing
left to compare gets the comment reconciled to its resolved state without scanning anything, and one
with neither pays for a single cheap job that reads the paths and looks for a comment.
The base is github.event.pull_request.base.sha rather than a merge base, which reviewers have
reported as a bug twice. actions/checkout on a pull_request event checks out GitHub's test merge
commit, whose two parents are base.sha and the PR head, so the head tree being scanned already
contains everything on the base branch up to base.sha. Diffing that against base.sha isolates
the pull request's own work. A real merge base would leave the intervening base-branch commits in
the head tree and out of the base tree, and attribute all of them to the pull request.
What the score means
It is the mean score of the 412 entry points that had at least one applicable check, where an entry's score is the share of its applicable checks that passed.
One property of that definition will mislead you otherwise. Changing which routes a check applies
to moves the score without anything in the webapp changing. Widening the sensitive cohort from 26
routes to 67 gave auth-boundary 39 more routes to look at, 36 of which already passed it, and the
global went from 15 to 19 without a line of apps/webapp changing. Narrowing a check, or a refactor
that takes routes out of the denominator, runs the same way in reverse. A movement is evidence about
the codebase only once you have checked the CHECKS block below for an applicability change. Compare
fix lists, not scores.
The number is deliberately unflattering, and one platform change would move most of it. Nothing
central attaches a tenant: logger pushes { requestId, path, host, method } onto every line
through AsyncLocalStorage and forwards errors to Sentry, and the route builders log
logBoundaryError(message, error, url). If the auth path pushed environmentId through
trace(...), several hundred entry points would flip at once, and request-context would want
rethinking rather than celebrating.
What the number cannot tell you
request-context checks that a failure-path log names a tenant field. It does not check that the
value is real. Adding a synthetic environmentId: "obs-map" field to the first object argument of
all 139 in-catch log calls, with no other change, takes the global from 19 to 29 and the CONTEXT
figure from 11 to 98.
That is the tool verifying presence, not meaning, and it is not a bug to fix. Every check reads
syntax: a field name, a call, a binding reference. None can tell a genuine tenant id from a
hardcoded string with the right key. A reviewer owns whether the value behind the field is real, the
same way a Lighthouse accessibility score checks that an alt attribute exists and not that its
text describes the image. The number tells you where to look. It does not tell you what you will
find there.
What stops it being gamed
src/mutationCorpus.test.ts applies 53 semantics-preserving or handling-deleting rewrites to the
whole route tree in a temp copy and asserts three things for each: the published global does not
rise, the mean over the routes measured in both runs does not rise, and for a semantics-preserving
rewrite no individual route's score rises or drops out of the measured set. Every laundering shape a
reviewer has found is an entry in src/mutations.ts, and each entry says which kind it is.
Two entries are the ones the design turns on. Deleting every catch clause in the tree drops the
score from 19 to 8, so the metric does not pay you for removing error handling. Wrapping every body
in try { ... } catch (e) { throw e } leaves the global at 19 and raises no route, so it does not
pay you for adding error handling that does nothing either.
Two holes are open and the corpus says so. A catch over try { 0; } is refused, but canRaise
accepts any call, so try { String(0); } reads as real error handling: it takes the tree from 19 to
44 and raises 224 routes. Telling an inert call from one that can throw needs types the scanner does
not have. The second, dead-conjunction-instanceof-if, is a dead condition rather than a dead arm:
selectsADistinctPath folds the arm, and literalTruth treats && as always null on purpose so a
live guard is never read as dead, so widening that fold is a different rule needing its own
measurement. Both run as expected failures with the residual written out beside them, so the claim
is "51 rewrites are defended and here are the two that are not", never "unpaddable".
The corpus takes minutes rather than seconds, under two on CI's runner and closer to eight on a
laptop, so it is gated behind OBS_MAP_MUTATION_CORPUS=1
and runs as its own CI job rather than in pnpm test. Run it if you change this package:
OBS_MAP_MUTATION_CORPUS=1 pnpm --filter @internal/observability-map exec vitest run \
src/mutationCorpus.test.ts --disable-console-intercept
The five checks
- error-classification: does every catch clause decide what it caught, by branching on the error or by guarding a parse it can answer for. A clause that only rethrows decides nothing and is read as though there were no catch, so it neither passes nor fails.
- auth-boundary: does a route handling credentials, access control, sessions, billing or impersonation check who is asking.
- auth-scope: does a sensitive builder-wrapped route also narrow itself to the caller, in every
export, by declaring
authorizationor by filtering on the caller's own id. All nine route builders authenticate the request, but theirauthorizationoption is optional andapiBuilder.server.tsruns the RBAC gate insideif (authorization), so a route can be authenticated and scoped to nobody. That is the cross-org IDOR classapps/webapp/CLAUDE.mdnames. It applies to 19 routes and 17 pass; both failures resolve their target organization from the URL slug with no membership filter and put nothing but an ability check in front of it (_app.orgs.$organizationSlug.settings.sso/route.tsxin its loader,_app.orgs.$organizationSlug.settings.team/route.tsxin its action). The fix in each ismembers: { some: { userId } }on the lookup. - request-context: when this entry point's failure is reported, is the tenant named.
- audit-trail: does a sensitive mutation leave a record of who did it. Three routes do, all of
them impersonation paths reaching
prisma.impersonationAuditLog.createinmodels/admin.server.ts; the other 46 do not.
audit-trail is excluded from the score. The other four are in it.
auth-boundary, auth-scope and audit-trail only look at routes src/sensitivity.ts calls
sensitive, and that cohort is the fix list's primary sort key, so what goes in it decides what a
reader sees first. 67 routes are in it today: credentials and tokens, envvars, billing and the two
billing settings the bare billing segment does not match, impersonation, membership and invites
and roles and the team page, the login surface, API keys, and org or project deletion. Calling a
guard never makes a route sensitive, and src/webappSymbols.test.ts fails if a symbol or path
segment in the vocabulary stops resolving in the webapp.
What the score is made of
The check list describes a composite the number mostly is not, so the report discloses the shape instead of hiding it behind a weight. Today:
CHECKS
error-classification 166 applicable, 94 pass, 0 sole, global without it 10
auth-boundary 62 applicable, 59 pass, 0 sole, global without it 15
auth-scope 19 applicable, 17 pass, 0 sole, global without it 18
request-context 412 applicable, 11 pass, 223 sole, global without it 65
audit-trail 49 applicable, 3 pass, 0 sole, not in the score
sole says the most: 223 of the 412 measured entry points have exactly one applicable scored check,
so their score is 0 or 100 on a single boolean. Read the family bars with that in mind. They do not
compare families on observability in general; they mostly compare them on whether someone wrote a
tenant field into a catch log.
Weighting was considered and rejected, because a coefficient nobody can explain invites argument
about the number instead of about the finding. The block above is in the terminal report and in the
JSON as checkContributions.
Two findings are headlines, not list entries
audit-trail fails 46 of 49 and request-context fails 401 of 412. Printing either one per route
would bury the route-specific findings under the same sentence repeated hundreds of times, so both
are reported as a figure: the AUDIT and CONTEXT lines. 328 entry points fail nothing except
request-context and appear only in that figure, which leaves 76 in the fix list. An entry that
fails request-context and another scored check keeps both findings and stays in the list, so
/account/tokens still shows the whole picture. audit-trail does not count as "another" for this
purpose, being already a headline, so a route failing only request-context and audit-trail
collapses too (28 do today, all of them sensitive).
42 of those 328 are sensitive, so the CONTEXT line says how many. Read them out of
observability-map.json, where every entry keeps its full check results, rather than assuming the
fix list is the whole story.
request-context is still scored, unlike audit-trail. The gap it measures is real and the score
is meant to show it. Only the presentation collapses.
When a check declines to judge
The rule every applicability decision follows: would this evidence necessarily be visible in the body if it existed?
A log call inside a catch would be, because the catch is right there in the body being read, so its
absence is evidence of absence and request-context fails the route. A guard on work that happens
inside an imported helper would not be, because neither the work nor the guard is in the body, so
auth-boundary reports not-applicable with a detail saying it could not verify rather than accusing
the route of being unguarded. resources.impersonation.ts is the worked example: it calls
clearImpersonation, which authenticates and writes an audit row in app/models/admin.server.ts, a
file this tool never opens. Five of the 67 sensitive routes sit out for this reason.
The failure mode the rule exists to prevent is a fix list whose top three entries are all wrong, which is what a check asserting more than its evidence supports produces.
Not applicable is not a pass
An entry with no applicable scored check is measured: false, and it is left out of every mean the
report computes (src/score.test.ts: excludes an unmeasured entry point from the global mean,
excludes an unmeasured entry point from its family mean too). Its score field reads 100, a
placeholder for "nothing was measured here" that nothing averages, because the alternative of
letting unmeasured entries into the mean at 100 would let the tool look better the less it
understood. The header prints every count (412 measured, 15 unmeasured) so the denominator is
never hidden, and a family with nothing measured renders as not measured rather than as a full
green bar.
15 of the 427 routes are unmeasured because isTrivial rules them out before any check runs.
Trivial means a body of three statements or fewer, three or fewer calls, no try/catch, no builder
wrapping it, and nothing in the calls or the source naming a datastore or a service (prisma,
logger, fetch, redis, and the like). Parse the params, build a path, redirect: nothing there
for a check to find evidence in either way. Exclusion is a denominator exit, not a credit.
A route whose body is somewhere else is a different case. export { action } from "./handler.server"
and export const action = handleWebhook are not trivial: a redirect stub genuinely has nothing to
instrument, while a delegating route has work the scanner cannot see, and treating them alike would
delete a route from the metric whenever someone moved a body into a .server.ts file. Delegating
routes are counted apart from the unmeasured ones, listed on a DELEGATED line and carried in the
JSON as delegating, the same treatment a parse failure gets and for the same reason. There are
none in the tree today, which is exactly why the case needed writing down before someone wrote one.
Suppression
// obs-map-disable auth-boundary -- public by design, see ADR 12
The reason is mandatory: a suppression without one is ignored. The directive is read from comments only, so a string literal quoting it does not switch a check off.
It applies to the whole entry point, not to the line under it. Line scoping is not available,
because a finding is attached to an entry point and carries no line number to match against, which
is why the old obs-map-disable-next-line spelling is not honoured.
A suppression cannot raise a score. The suppressed check leaves the numerator and the denominator,
and the result is capped by what the entry would have scored unsuppressed, so suppressing a failing
check holds the number still rather than improving it (src/score.test.ts: does not raise the score when a failing check is suppressed). What you buy is removal from the worklist with a reason
on the record. The report prints how many suppressions are in force so the practice stays visible.
Known limits
Read these before trusting a specific verdict.
- One hop, same file only. If a loader delegates to a helper in the same file, that helper's
statements, catches and calls count as the route's. A helper's own helpers do not, and nothing
imported from another module is ever opened.
auth-boundaryapplies to 62 of the 67 sensitive entry points; the other 5 hand their work to an imported helper and are reported as unverified rather than unguarded. - A guard is matched by name, not by what it does. The accept-list is 29 names read off the
webapp, plus two
SOFT_GUARDS.src/webappSymbols.test.tsproves each one is declared somewhere; nothing proves the declaration it found is the guard we meant.authenticateAdminandauthenticatePlainRequestare local helpers inside one route file each, so a second route declaring its own no-op function of either name would be credited. - Two guard names are only checked as far as being read.
getUserandgetUserIdanswer with null instead of throwing, so calling one is not a boundary. They are credited only when the body binds the result and some condition reads it (EntryPoint.checkedCallees). What that cannot see is whether the test guards anything:if (!user) { logger.warn("anonymous"); }followed by the work reads the same as returning. authenticateandisAuthenticatedare unresolved on purpose. They are remix-auth's, and resolving them means reading a path insideapps/webapp/node_modules, which fails confusingly on an install-layout change. They are listed inEXTERNAL_GUARDSinstead, so the resolution test still rejects a name that is neither first-party nor listed.auth-scopeapplicable structurally impliesauth-boundarypass. The check only applies to a builder-wrapped route, andauth-boundarypasses any builder-wrapped route, so all 19 carry the sameauth-boundarydetail, "authenticated by the builder". That free point is a third or a quarter of each of their scores: the 19 average 59.7 as scored and 44.6 withauth-boundarytaken out, andsettings.team, a confirmed cross-org exposure, scores 25 rather than 0. Read the finding rather than the score.auth-scopecannot tell a caller-id filter from a caller-id actor argument.presenter.call({ userId: user.id })narrows the query;generatePortalLink({ organizationId, userId: user.id })records who asked. Both read as scoping. Separating them means following the argument into the callee, so the four helpers credited this way (ApiKeysPresenter,TeamPresenter,regenerateApiKey,DeleteOrganizationService, all of which domembers: { some: { userId } }and throw) were hand-read instead. No route in the tree passes on an actor argument alone.auth-scopereads property assignments in that export's own handler. A handler that pulls the id into a local first,const userId = user.id; ... { userId }, or that builds its filter in a same-file helper, scopes itself and is not seen, so it would be reported as unscoped.auth-scopereads the builder-wrapped exports and says nothing about the rest of the file. A route whose action is builder-wrapped and whose loader is a plainexport async function loaderis judged on the action alone, and the pass detail, "every builder-wrapped export has an authorization gate", is true of what it read while reading as a claim about the whole route. Ten routes in the tree mix the two, and the one sensitive enough for the check to run on is_app.orgs.$organizationSlug.settings._index/route.tsx, whose builder-wrapped action carries the pass and whose plain loader filters onmembers: { some: { userId } }. That was hand-read.- Three login-flow routes fail
auth-boundarycorrectly and unhelpfully./auth/sso,/api/v1/authorization-codeand/api/v1/tokenare unauthenticated by design: the caller is anonymous at that point, which is the whole purpose. The check's statement about them is true and there is nothing to fix, so they are candidates for a suppression comment with the reason on the record. - Loggers are matched by spelling. A call counts as logging when the callee reads
logger.*orlog.*. An aliased logger, one wrapped in a helper, orconsole.erroris invisible, so a route can be reported as recording nothing while it records plenty. - A catch that logs and rethrows reads as though it only rethrows. The clause evidence cannot say
whether a clause does anything besides rethrow, so
error-classificationwithholds credit rather than granting it and reopening the free-points path a singlelogger.errorline wide. - Only the first object-literal argument is read for identifier fields, and only its property
names.
logger.error("failed", ctx)wherectxis a variable contributes nothing, and neither does a second object. - A catch inside a per-item callback is not the route's.
items.map((item) => { try {...} })is a fresh boundary per element, so its clause is not read as the route's own error handling. The test is the method name, which cannot tellusers.mapfromResult.map. Being wrong there costs precision rather than points: a refused catch fails the route rather than excusing it. - A route that delegates only one of its two exports is judged on the other.
export { action } from "./x"beside a loader written in the file is not counted as delegating, so half the route is scored and half is invisible. try { String(0); }still buys a pass. The open corpus entry above, and the largest single hole known in the tool: measured live, it takes the tree from 19 to 44 and raises 224 routes.- A forged tenant field buys a pass too.
request-contextreads the field name, never the value, so a codemod writingenvironmentId: "obs-map"into every in-catch log call takes the global from 19 to 29. Unlike the entry above this one is not a bug to fix, since no syntactic check can tell a real tenant id from a constant, but it bounds what the number can mean either way. - The score is a mean of means over a heuristic. Read the fix list, the two headline figures and the CHECKS block. Watching the single number for small movements will mislead you.
Layout
scan.ts walks the routes directory and produces an EntryPoint per module, carrying only
body-scoped evidence. checks/ holds the five checks, each a pure function of an EntryPoint.
score.ts turns checks into an entry score and a report, report/ renders it, cli.ts is the
entry point. sensitivity.ts, triviality.ts and suppression.ts are the three inputs the checks
share.
Tests sit next to their subject in src/. Every check has a false-positive fixture, something it
must not flag, alongside the positive one. Keep that: most of the bugs this package has had were
checks that fired on the wrong thing, and a test that only proves the heuristic fires would have
caught none of them.