AIT-652: CLI stops shipping expected sev3 errors to Sentry - #80
Conversation
Sev3 CliErrors are expected user states the CLI already tells the user how to fix, and PostHog records them as cli_command_invoked.errorCode. Customer agent loops replayed CHANNEL_FORWARDING_DISABLED and AUTH_REQUIRED by the thousand and exhausted the Sentry org quota on 2026-09-13, silencing the pager for every service. NetworkError and ConfigWriteForbiddenError stay in Sentry: they describe the environment, not the user.
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: 3d6d040cd6
ℹ️ 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".
| if (err?.severity === 'sev3' && !SEV3_ENVIRONMENT_CODES.has(err.code)) { | ||
| return false; |
There was a problem hiding this comment.
When a CDN, load balancer, or API gateway returns a 500/502/503 before the backend application handles the request, mapApiError still converts that response into a sev3 ApiError (src/api/client.ts:328-334). This condition now drops it under the assumption that the backend already captured it, but in this scenario no backend exception exists, so an infrastructure outage disappears from Sentry and its pager. Preserve at least generic 5xx ApiErrors, or distinguish responses known to have been captured by the application.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Applied in 84b4907: the uncoded SERVER_ERROR wrapper (bare 5xx, no body code) stays in Sentry; coded 5xx bodies came from the backend, which captured its own side.
An edge or load-balancer 5xx never reaches the backend, so nothing else captures it. Only the uncoded SERVER_ERROR wrapper qualifies; coded 5xx bodies came from the backend, which captured its own side.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84b49076a1
ℹ️ 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".
| if (err?.severity === 'sev3' && !SEV3_ENVIRONMENT_CODES.has(err.code)) { | ||
| return false; |
There was a problem hiding this comment.
This condition drops every CliError that has not been explicitly allowlisted, but CliError defaults to sev3 and is also used for operational failures rather than expected user states. For example, ensureCloudflaredBinary throws it for a missing checksum manifest, download failures, and checksum mismatches, while Instagram publishing uses it when successful-looking Meta responses omit required IDs. Those failures previously reached Sentry and now disappear even though they indicate broken releases, dependencies, or integrations; restrict suppression to known expected codes or reclassify/allowlist these operational errors.
Useful? React with 👍 / 👎.
| // service. PostHog keeps them (`cli_command_invoked.errorCode`). | ||
| // SERVER_ERROR is the bare 5xx wrapper (no body code): an edge/load-balancer | ||
| // failure the backend never saw, so nothing else captures it. | ||
| const SEV3_ENVIRONMENT_CODES = new Set(['NETWORK_ERROR', 'CONFIG_WRITE_FORBIDDEN', 'SERVER_ERROR']); |
There was a problem hiding this comment.
Fresh evidence after the prior fix is that mapApiError only preserves SUPPORT_* codes: for any other 5xx response, line 334 of src/api/client.ts constructs ApiError without the response's code. Consequently, a backend response such as a coded 500 is relabeled SERVER_ERROR and passes this allowlist even though the backend handled and captured it, so repeated coded failures can still consume the Sentry quota this change is meant to protect. Preserve whether the 5xx body had a code and reserve this allowlist entry for genuinely uncoded responses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Stale re-anchor; addressed in 3ce611c (details.serverCode).
Codex round 2: - BINARY_* and PUBLISH_NO_* were bare sev3 CliErrors and would have been dropped; they are broken releases/integrations, so they are sev2 UnexpectedError now (same code, message, exit code). - mapApiError discards the body code on generic 5xx, so every backend 5xx looked like a bare edge 5xx. The code now rides in details.serverCode and the Sentry filter keeps only SERVER_ERROR without one.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ce611c2de
ℹ️ 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".
| return false; | ||
| } | ||
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
Fresh evidence after the earlier reclassification fix is that this blanket sev3 rejection still drops non-user auth failures: src/auth/login.ts:915-916 throws AuthError when WorkOS returns any non-2xx response (including a 5xx outage), and src/auth/mcp-credential.ts:198-201 uses it when a successful credential response is malformed. Both inherit AUTH_REQUIRED/sev3, so neither reaches Sentry even though they indicate dependency or backend failures rather than an expired session. Reclassify these throw sites or distinguish expected auth-state codes from operational auth errors in this filter.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Applied in 54931fb, plus the same pattern in the two SSE connect sites (channels-logs, sandbox logs).
Codex round 3: WorkOS device-auth refusal, malformed agent-credential response and SSE connect refusals all threw AuthError (sev3, dropped by the new filter). They are dependency/backend faults: UnexpectedError with their own codes, exit code 4 preserved. SSE transport failures are NetworkError.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54931fb4cd
ℹ️ 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".
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
Fresh evidence after the earlier reclassification is that this blanket rejection still drops dependency and integration failures at unchanged throw sites: when WorkOS's device-token poll returns a 5xx or another non-pending error, src/auth/login.ts:145 throws an AuthError, and when a successful media lookup omits its required URL, src/commands/whatsapp-media.ts:59-62 throws a ValidationError. Neither is an expected user state, and the external service/backend may not capture the malformed or failed exchange, so these failures now disappear from Sentry; reclassify these paths as sev2 or explicitly preserve their codes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Applied in 68e3cad (plus PLANS_EMPTY in billing.ts, same pattern). Declined/expired device codes stay AUTH_REQUIRED.
…log are sev2 Codex round 4. Declined/expired device codes stay AuthError; every other grant error is WORKOS_DEVICE_TOKEN_FAILED. NO_MEDIA_URL and PLANS_EMPTY are backend contract breaks, not user input. Exit codes preserved.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68e3cadea9
ℹ️ 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".
| return false; | ||
| } | ||
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
Fresh evidence after the earlier sev3 reclassifications is that this blanket rejection still drops operational failures classified as ValidationError: src/auth/login.ts:609-612 reports an empty or malformed OAuth scope-discovery response, while src/commands/billing.ts:458-464 reports inconsistent billing eligibility for a money-model-v2 subscription. Neither condition is caused by invalid user input, and their successful/handled upstream responses may produce no server exception, so this change makes those integration faults disappear from Sentry; reclassify these sites as sev2 or preserve their codes here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Applied in 2d20449 (plus WORKSPACE_ID_NOT_PUBLIC_ID in workspace.ts, whose message already said 'this is a bug').
… are sev2 Codex round 5 plus one self-found site. Backend contract breaks and an internal invariant were ValidationErrors (sev3, dropped by the filter). Exit codes preserved.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d20449e19
ℹ️ 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".
| return false; | ||
| } | ||
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
When an expired credential refresh succeeds but persisting the new token fails on a read-only config directory, safeWriteFileSync raises ConfigWriteForbiddenError (src/storage/path.ts:57-65), but both refresh callers replace every non-NetworkError with a sev3 AuthError (src/api/client.ts:133-141 and 162-171). This blanket rejection therefore drops precisely the config-write environment failure the allowlist claims to retain, leaving no Sentry event and misleading the user with “Session expired”; preserve that operational error through the refresh catches or otherwise classify it before filtering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Applied in 4964a60. ConfigWriteForbiddenError now propagates through both refresh catches; a malformed refresh body keeps the user-facing message but is captured to Sentry before wrapping.
… expired Codex round 6. saveCredentials on a read-only config dir threw ConfigWriteForbiddenError, which both refresh catches replaced with AuthError. The environment fault now propagates. A malformed 2xx refresh body keeps the Session-expired message for the user (login fixes it) but is reported to Sentry first, under its own code.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4964a60744
ℹ️ 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".
| return false; | ||
| } | ||
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
Fresh evidence after the auth-path reclassifications is the successful device-authorization path: src/auth/login.ts:938-945 destructures WorkOS's 2xx payload without validating expires_in. If a malformed response omits that field, pollForTokens computes a NaN deadline at line 65, skips polling, and throws the sev3 AuthError at line 158; this blanket rejection then drops the dependency contract failure even though WorkOS will not record it in this project's Sentry. Validate the initial payload and raise a sev2 error before filtering.
Useful? React with 👍 / 👎.
Codex round 7. A 2xx without device_code/user_code/expires_in/verification url would have produced a NaN deadline and a sev3 Session-expired. It is WORKOS_DEVICE_AUTH_MALFORMED (sev2, exit 4) now; a missing interval falls back to the RFC 8628 default.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 214f4c0a3b
ℹ️ 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".
| if (typeof device_code !== 'string' || !device_code || typeof user_code !== 'string' || | ||
| typeof expires_in !== 'number' || !Number.isFinite(expires_in) || | ||
| (typeof verification_uri !== 'string' && typeof verification_uri_complete !== 'string')) { |
There was a problem hiding this comment.
Fresh evidence in the applied guard is that it still accepts an empty user_code, an empty verification URL, and a zero or negative expires_in. In particular, a non-positive lifetime makes pollForTokens skip its loop and throw the sev3 timeout AuthError, which the new Sentry filter suppresses, while an empty verification_uri_complete wins over a valid verification_uri via ?? and leaves the user unable to authenticate. Treat these values as WORKOS_DEVICE_AUTH_MALFORMED by requiring non-empty strings and a positive expiry.
Useful? React with 👍 / 👎.
…expiry Codex round 8. Empty user_code / verification url and expires_in <= 0 are WORKOS_DEVICE_AUTH_MALFORMED too; an empty verification_uri_complete no longer shadows a valid verification_uri.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c85575df6
ℹ️ 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".
| function isRefreshEnvironmentFault(err: unknown): boolean { | ||
| if (err instanceof NetworkError) return true; | ||
| return (err as { code?: unknown } | null)?.code === 'CONFIG_WRITE_FORBIDDEN'; |
There was a problem hiding this comment.
When saveCredentials fails with anything other than CONFIG_WRITE_FORBIDDEN—for example ENOSPC/EIO, or an error from the unwrapped chmodSync in src/storage/secrets.ts—this predicate returns false, so both refresh callers replace the filesystem failure with a sev3 AuthError; the new Sentry filter then suppresses it and the user is incorrectly told the session expired. Fresh evidence beyond the earlier read-only-directory fix is that safeWriteFileSync rethrows these other filesystem errors unchanged and does not cover the subsequent chmod, so credential-persistence failures should be distinguished from refresh rejection based on which operation failed rather than this single-code allowlist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Applied in c683b15: refresh and persist are separate try blocks; a failed save keeps its typed AppError or becomes CREDENTIAL_WRITE_FAILED (sev2).
Codex round 9. Refresh and persist are separate steps now: a rejected refresh is Session expired, a failed save keeps its typed AppError or becomes sev2 CREDENTIAL_WRITE_FAILED (ENOSPC, EIO, chmod).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c683b15fdb
ℹ️ 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".
| return false; | ||
| } | ||
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
A malformed 2xx response from the device-token poll is still treated as a successful login: src/auth/login.ts:93-104 revokes the prior credential and saves data.access_token/data.refresh_token without checking them. If WorkOS omits either field, the next wizard API request sends Bearer undefined, maps the resulting 401 to a sev3 AUTH_REQUIRED, and this blanket rejection suppresses it, so the dependency contract failure both corrupts the credential store and leaves no Sentry event. Validate the poll response before revoking or saving credentials and raise a sev2 malformed-response error.
Useful? React with 👍 / 👎.
Codex round 10. A 2xx without access_token/refresh_token no longer revokes the previous session or writes undefined into the store; it is WORKOS_DEVICE_TOKEN_MALFORMED (sev2, exit 4).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f105756146
ℹ️ 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".
| return false; | ||
| } | ||
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
When there is no active workspace and /workspaces returns an uncoded 5xx from an edge or load balancer, listWorkspacesOrEmpty in src/commands/_helpers.ts catches the allowlisted SERVER_ERROR and converts it to an empty list, after which getDefaultWorkspaceId throws a sev3 ValidationError. This predicate now suppresses that replacement error, so the exact infrastructure failure the SERVER_ERROR exception is intended to retain disappears from Sentry; propagate the original operational error rather than filtering the resulting validation error.
Useful? React with 👍 / 👎.
| return false; | ||
| } | ||
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
When a successful /workspaces response omits the active workspace row or its organizationPublicId, resolveOrgPublicIdForWorkspace in src/commands/_helpers.ts throws a ValidationError even though this is a backend contract failure, not invalid user input. Billing and customer commands now have that failure suppressed here, and the successful backend request produces no server-side exception to replace it; classify this path as sev2 or explicitly retain its code.
Useful? React with 👍 / 👎.
…sev2 Codex round 11. listWorkspacesOrEmpty swallowed uncoded 5xx into an empty list, which then failed as a sev3 not-found. A workspace row without organizationPublicId is a contract break (WORKSPACE_ORG_MISSING); a row that is simply gone stays the user-facing ValidationError.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efb25f9ed9
ℹ️ 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".
| throw new AuthError(`SSE connect failed: HTTP ${res.status}`); | ||
| // AIT-652: non-401 refusal is a backend/edge fault; sev2 so it reaches | ||
| // Sentry. Exit code stays 4 as before. | ||
| const err = new UnexpectedError(`SSE connect failed: HTTP ${res.status}`, 'SSE_CONNECT_FAILED'); |
There was a problem hiding this comment.
When either logs SSE endpoint returns an expected client response such as 403 for insufficient workspace access or 429 for rate limiting, this branch promotes it to a sev2 UnexpectedError; shouldCaptureToSentry retains every sev2 error, so agents repeatedly reconnecting can again fill Sentry with the API rejections this change intends to suppress. Map non-401 4xx statuses to the existing sev3 error classes and reserve SSE_CONNECT_FAILED for 5xx or malformed successful responses; the mirrored branch in src/commands/sandbox/logs.ts needs the same distinction.
Useful? React with 👍 / 👎.
Codex round 12. 403/429 from the log stream are expected client responses (sev3 ApiError, exit 4 as before); SSE_CONNECT_FAILED is reserved for 5xx and body-less 2xx.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63ee4c44a3
ℹ️ 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".
| if (err?.severity === 'sev3') { | ||
| if (!SEV3_ENVIRONMENT_CODES.has(err.code)) return false; |
There was a problem hiding this comment.
Fresh evidence after the earlier workspace-to-organization fix is that customers list still bypasses resolveOrgPublicIdForWorkspace: src/commands/customers.ts:43-49 combines a missing workspace row and an existing row missing organizationPublicId, then throws a sev3 ValidationError. In the latter case a malformed successful /workspaces response is a backend contract failure, but this blanket sev3 rejection suppresses it, unlike the corrected billing and other customer paths. Split those cases or reuse the shared resolver so the existing-row contract failure remains sev2.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2455def96
ℹ️ 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".
| if (err instanceof AuthError || err instanceof NetworkError || | ||
| (err instanceof ApiError && (err.statusCode ?? 0) >= 500)) { |
There was a problem hiding this comment.
Fresh evidence after the refresh-persistence and workspace-discovery fixes is that, when no active workspace is stored (or --workspace is used), an expired token can refresh successfully and then fail to persist with CONFIG_WRITE_FORBIDDEN or CREDENTIAL_WRITE_FAILED; apiClient('/workspaces') throws that operational error, but this allowlist does not propagate it. The catch therefore returns [], getDefaultWorkspaceId replaces the real filesystem failure with a suppressed “not a member of any workspace” ValidationError, and the user receives incorrect guidance with no Sentry event. Propagate local/operational errors here and swallow only the explicitly benign API responses.
Useful? React with 👍 / 👎.
| const err = new UnexpectedError( | ||
| 'HookMyApp did not return an org credential for this machine. Run: hookmyapp doctor', | ||
| 'AGENT_CREDENTIAL_MALFORMED', | ||
| ); |
There was a problem hiding this comment.
Fresh evidence after reclassifying this response is that agent setup and the post-login setup both call resolveMcpToken in src/commands/agent.ts, whose blanket catch at lines 381-387 converts every error into undefined. Thus, when /agent/credentials returns a successful but malformed payload, this new sev2 error never reaches the top-level Sentry capture; setup continues without the credential and can report the client as configured. Capture this contract failure before the soft-failure wrapper or allow operational errors to escape it.
Useful? React with 👍 / 👎.
1 participant
Fixes AIT-652
Sev3 CliErrors are expected user states (session expired, forwarding disabled, validation) and already reach PostHog. Customer agent loops replayed them by the thousand and exhausted the Sentry org quota on 2026-09-13. Keep NetworkError and ConfigWriteForbiddenError in Sentry (environment faults); drop the rest of sev3.