fix(telemetry): keep -32603 recovery failure generically fail-closed; document auth-required unreachability (#101) #102

Merged

adrian.tkacz merged 3 commits from fix/telemetry-auth-required into develop

2026-09-20 00:45:53 +00:00

kilo commented

2026-09-20 00:00:49 +00:00

Member

Fixes #101 by explicit audit decision: the telemetry_auth_required vocabulary stays deliberately UNREACHABLE — no structured auth discriminator exists in codex 0.154.0 (companion: Infrastructure/creatidy-autonomy#50).

Correction (second commit, `2293c89`)

The first iteration emitted auth_required after a failed bounded refresh/retry. Owner review correctly challenged that -32603 proves authentication failure. Source audit of pinned codex 0.154.0 (codex-rs/app-server/src/error_code.rs, request_processors/account_processor.rs) confirms the concern:

error() constructs every JSONRPCErrorError with data: None — only the integer code and free-text message exist, and free-text parsing is forbidden by the security contract.
get_account_rate_limits_response maps EVERY backend failure to the same internal_error(-32603): expired-token 401, 403, 5xx, timeout, empty snapshot. A failed refresh/retry does not distinguish auth from outage.
get_account_response discards the refresh outcome (refresh_token_if_requested(...) is fire-and-forget) — no error envelope ever carries refresh failure. Live probe (2026-09-20) matches: account/read {refreshToken:true} returned a result even though the retry then still failed -32603.

Resulting behavior (this PR's final state): failed refresh or failed retry → unknown/telemetry_unknown → eligibility unknown/telemetry_invalid — the pre-#102 fail-closed classification. Mislabeling a backend outage as auth-required would have caused pointless device-auth re-logins; accuracy was chosen over making the vocabulary reachable.

The one code-level auth-adjacent signal, -32600 ("…authentication required to read rate limits") for a missing/unusable local store, is unsafe as a discriminator (-32600 also covers unrelated invalid-request conditions, it is not the evidenced expired-token shape, and D-018 never refreshes on it).

What remains in this PR

Tests pinning the reverted classification and its rationale (refresh-error and retry-error stay generic; synthetic-secret non-appearance).
D-018 amendment in docs/decisions.md recording the audit, the evidence, and the exact conditions under which telemetry_auth_required may become reachable (a future codex generation with typed error data or a dedicated auth error code).
Operator-facing auth precision lives in the worker surface instead: autonomy-openai-maintain.prd exit 4 = auth_required (creatidy-autonomy#50/#51).

Validation

make check: 1183 tests OK (unittest discovery), basedpyright 0 errors / 0 warnings.
Note for reviewers: the branch contains one Merge branch 'develop' commit that appeared on the shared checkout during the review window (clean merge of owner-merged develop content, PRs #100/#103); it was kept rather than force-pushed, per the repository's no-history-rewrite rule.

Fixes #101 by explicit audit decision: the `telemetry_auth_required` vocabulary stays deliberately UNREACHABLE — no structured auth discriminator exists in codex 0.154.0 (companion: Infrastructure/creatidy-autonomy#50). ## Correction (second commit, 2293c89) The first iteration emitted `auth_required` after a failed bounded refresh/retry. Owner review correctly challenged that `-32603` proves authentication failure. Source audit of pinned codex 0.154.0 (`codex-rs/app-server/src/error_code.rs`, `request_processors/account_processor.rs`) confirms the concern: 1. `error()` constructs every `JSONRPCErrorError` with `data: None` — only the integer `code` and free-text `message` exist, and free-text parsing is forbidden by the security contract. 2. `get_account_rate_limits_response` maps EVERY backend failure to the same `internal_error(-32603)`: expired-token 401, 403, 5xx, timeout, empty snapshot. A failed refresh/retry does not distinguish auth from outage. 3. `get_account_response` discards the refresh outcome (`refresh_token_if_requested(...)` is fire-and-forget) — no error envelope ever carries refresh failure. Live probe (2026-09-20) matches: `account/read {refreshToken:true}` returned a result even though the retry then still failed `-32603`. **Resulting behavior (this PR's final state):** failed refresh or failed retry → `unknown`/`telemetry_unknown` → eligibility `unknown`/`telemetry_invalid` — the pre-#102 fail-closed classification. Mislabeling a backend outage as auth-required would have caused pointless device-auth re-logins; accuracy was chosen over making the vocabulary reachable. The one code-level auth-adjacent signal, `-32600` ("…authentication required to read rate limits") for a missing/unusable local store, is unsafe as a discriminator (`-32600` also covers unrelated invalid-request conditions, it is not the evidenced expired-token shape, and D-018 never refreshes on it). ## What remains in this PR - Tests pinning the reverted classification and its rationale (refresh-error and retry-error stay generic; synthetic-secret non-appearance). - D-018 amendment in `docs/decisions.md` recording the audit, the evidence, and the exact conditions under which `telemetry_auth_required` may become reachable (a future codex generation with typed error `data` or a dedicated auth error code). - Operator-facing auth precision lives in the worker surface instead: `autonomy-openai-maintain.prd` exit 4 = `auth_required` (creatidy-autonomy#50/#51). ## Validation - `make check`: 1183 tests OK (unittest discovery), basedpyright 0 errors / 0 warnings. - Note for reviewers: the branch contains one `Merge branch 'develop'` commit that appeared on the shared checkout during the review window (clean merge of owner-merged develop content, PRs #100/#103); it was kept rather than force-pushed, per the repository's no-history-rewrite rule.

kilo added 1 commit

2026-09-20 00:00:49 +00:00

fix(telemetry): classify evidenced Codex auth failure as auth_required (issue #101 )

ci / check (pull_request) Successful in 57s

Details

4707e5eb3a

After the bounded D-018 recovery sequence fails (provider-managed refresh
response or rate-limits retry carries a validated protocol error), the
observation now reports snapshot status/diagnostic auth_required and the
paired eligibility report carries reason telemetry_auth_required instead
of the ambiguous unknown/telemetry_invalid. Only the integer protocol
error code is ever read; error bodies, tokens and auth material stay
excluded, and routing stays fail-closed (auth_required -> state unknown).

Verified against the 2026-09-19 Precision PRD incident: rateLimits/read
returned -32603 (backend 401, expired access token) while the router
surfaced telemetry_invalid, costing a manual investigation. The
auth_required vocabulary already existed on every contract surface but
was unreachable because _run_session never emitted the status.

D-018's recovery-oracle wording is amended explicitly in docs/decisions.md
per the repo rule that documentation conflicts are resolved in place.

adrian.tkacz added 1 commit

2026-09-20 00:15:07 +00:00

Merge branch 'develop' into fix/telemetry-auth-required

ci / check (pull_request) Successful in 52s

Details

2bb46b6ec0

adrian.tkacz referenced this pull request from a commit

2026-09-20 00:28:25 +00:00

fix(telemetry): keep -32603 recovery failure generically fail-closed (issue #101 audit)

adrian.tkacz added 1 commit

2026-09-20 00:28:25 +00:00

fix(telemetry): keep -32603 recovery failure generically fail-closed (issue #101 audit)

ci / check (pull_request) Successful in 54s

Details

2293c8957e

Owner review correction on PR #102: -32603 is codex 0.154.0's GENERIC
internal error. Source audit (codex-rs/app-server/src/error_code.rs,
request_processors/account_processor.rs) shows every rate-limits backend
failure -- expired-token 401 and outage alike -- is flattened into
-32603 with the cause only in the free-text message; the JSON-RPC data
member is always absent; the account/read refresh outcome is discarded
by its handler and never surfaces as an error envelope. A failed bounded
refresh/retry therefore does not prove an authentication condition, and
free-text error parsing is forbidden by the security contract.

The previous commit's auth_required emission is reverted to the generic
fail-closed unknown/telemetry_unknown (eligibility telemetry_invalid).
telemetry_auth_required stays deliberately unreachable until a future
codex generation exposes a bounded, structured, non-secret auth-failure
signal. Tests pin the reverted classification and its rationale; D-018
records the audit outcome and the conditions under which the vocabulary
may become reachable.

Routing remains fail-closed in both cases; accuracy over vocabulary
reachability.

kilo referenced this pull request

2026-09-20 00:28:48 +00:00

fix(telemetry): classify evidenced Codex auth failure as auth_required / telemetry_auth_required #101

kilo changed title from ~~fix(telemetry): classify evidenced Codex auth failure as auth_required / telemetry_auth_required (#101)~~ to fix(telemetry): keep -32603 recovery failure generically fail-closed; document auth-required unreachability (#101)

2026-09-20 00:29:43 +00:00

adrian.tkacz approved these changes

2026-09-20 00:45:48 +00:00

adrian.tkacz merged commit e4391dfcfc into develop

2026-09-20 00:45:53 +00:00

adrian.tkacz deleted branch fix/telemetry-auth-required

2026-09-20 00:45:53 +00:00

adrian.tkacz referenced this pull request from a commit

2026-09-20 00:45:53 +00:00

Merge pull request 'fix(telemetry): keep -32603 recovery failure generically fail-closed; document auth-required unreachability (#101)' (#102) from fix/telemetry-auth-required into develop