Skip to content

fix(auth): honor pinned accounts during compaction - #1572

Open
feelsodev wants to merge 3 commits into
code-yeongyu:mainfrom
feelsodev:fix/pinned-compaction-auth
Open

feelsodev wants to merge 3 commits into
code-yeongyu:mainfrom
feelsodev:fix/pinned-compaction-auth

Conversation

@feelsodev

@feelsodev feelsodev commented Sep 10, 2026

Copy link
Copy Markdown

Summary

Direct authentication lookup now honors the stored pinned account when a caller does not explicitly select a credential slot. This fixes compaction using an exhausted default account while ordinary conversation successfully uses another pinned account.

The reported environment was OMO 5.0.0-0.beta.52, Senpi 2026.9.10, and openai-codex / gpt-6-astra. The fix belongs in Senpi's shared auth resolver rather than a patch to an installed JavaScript bundle.

Confirmed defect and account-selection policy

The reproduced defect is that compaction bypasses the ordinary request account-selection path and reads the flat default credential even while a usable account is pinned. Selecting the default account is valid when the normal selection or failover policy chooses it; the account being named default is not itself an error.

The intended consistency is for compaction using the same provider to respect the session's account-selection and failover policy. That does not require forcing every request onto an unavailable pinned account.

What this patch proves, and what remains to review

  • Proven: with a usable pinned login-6 and an exhausted default account, ordinary requests succeed while unpatched compaction selects default and fails. The same real-CLI scenario succeeds after this patch. Explicit request-key and slot overrides retain precedence.
  • Current scope: the patch uses the stored pin for direct auth lookup. It does not route compaction through the runtime's complete health-aware selection and failover machinery.
  • Blocked pinned account: ordinary requests filter unavailable slots in packages/coding-agent/src/core/credential-pool/failover.ts, using health information overlaid in rotation-stream.ts. The shared selector also explicitly tests fallback from a blocked pin in packages/ai/test/credential-pool-select.test.ts. The new direct-lookup fallback does not consult that health state. A real compaction scenario with an unavailable pin has not been verified.
  • Missing pinned account: this PR's tests require direct lookup to return no auth rather than silently use another account. This is a chosen direct-lookup behavior, not an expectation established by the original incident. Its consistency with ordinary pool fallback needs review.

The original healthy-pin defect is therefore independently demonstrated, but the passing regression suite should not be read as proof of full account-selection policy parity. Before merge, review the usable, blocked, and missing-pin cases together, keeping explicit per-request overrides authoritative.

Changes

  • Resolve explicit request keys first, then explicit slots, then the stored pin. Unpinned credentials retain their existing default behavior.
  • Reuse the existing slot projection and OAuth refresh path. Missing pinned slots do not silently switch to default or ambient credentials, and refreshing a selected slot preserves sibling credentials.
  • Add 11 deterministic auth regression cases. Keep every assertion in the existing default-slot refresh test, but explicitly request the default slot instead of relying on the behavior being fixed.
  • Add a reproducible real-CLI QA driver and update the source change tracker and unreleased package changelogs.

Verification

Failing-first auth regression

bun run --cwd packages/ai test -- test/auth-pinned-resolve.test.ts

Before the fix: 5 failed, 6 passed. Implicit OAuth/API-key pins, missing-pin isolation, and pinned OAuth refresh failed. Explicit key/slot overrides and unpinned defaults passed.

After the fix: 11 passed, 0 failed.

Real CLI compaction

node .agents/skills/senpi-qa/scripts/pinned-compaction-qa.mjs --self-test --expect green

The driver launches the repository CLI through RPC with a synthetic session and isolated OAuth storage. A localhost provider rejects the default account with HTTP 402 and accepts login-6. It does not replace the resolver, pool selection, compaction machinery, or RPC handlers.

Observable Before fix After fix
Ordinary prompt login-6 / HTTP 200 login-6 / HTTP 200
Compaction request default / HTTP 402 login-6 / HTTP 200
RPC compact success false true
Summary committed false true
Old context removed no compaction positive structural savings
Subsequent prompt succeeds succeeds and replays summary

The final fixture was run against the old behavior with --expect red before restoring the fix. RED passed 14 bug-signature assertions; GREEN passed 16 success assertions. Every run verified that the CLI process group, localhost server, and sandbox were removed and real auth was unchanged. No real provider calls or user conversations were used.

Other checks

After merging current main in b4d314b3f, the complete AI suite passed 2,716 tests with 0 failures (884 pre-existing opt-in skips), and static checks, the full build, the local changelog gate, and all 16 real-CLI compaction assertions passed again. Follow-up receipts are under local-ignore/qa-evidence/20260910-pinned-compaction/merge-followup/. The counts below describe the original pre-merge verification.

  • Complete AI suite: 2,696 passed, 0 failed, with 884 pre-existing opt-in skips; no skipped tests added.
  • Focused auth/pool/refresh suites: 22 passed.
  • Root static gate components passed: Biome, pinned dependencies, TypeScript import paths, shrinkwrap, install lock, Claude SDK platform lock, tsc --noEmit, and browser smoke. Biome ran read-only instead of using the root command's auto-write flag.
  • bun run build: passed all workspace phases.
  • Actual CLI smoke: 8/8 passed, including help, version, offline model listing, invalid-option handling, and real-auth isolation.
  • Final touched-file Biome checks, root compiler validation, driver syntax, and git diff --check: passed.

Sanitized local receipts are retained under local-ignore/qa-evidence/20260910-pinned-compaction/: red.txt, green.txt, cli-red.json, cli-green.json, auth-scoped-green.txt, ai-suite-green.txt, check-build-green.txt, final-static.txt, cli-smoke.txt, and README.md. Evidence is intentionally not committed under the repository's QA policy; the driver and assertions are included for reproduction.

Scope and remaining coverage

The production change is the stored-pin fallback plus an annotation using the existing pooled credential type. It applies to shared text/image auth lookup, not just the compaction caller.

The CLI scenario ran on macOS with synthetic OAuth and an OpenAI-compatible localhost endpoint. Credentialed Codex calls and the complete unrelated coding-agent test suite were not run. The LSP tool could not initialize with the native TypeScript installation; the actual TypeScript compiler and build passed.


Summary by cubic

Fixes direct auth resolution so compaction uses the stored pinned account instead of the flat default credential. Previously an exhausted default account blocked compaction while ordinary conversation used another pinned account; now the stored pin is honored whenever no credential slot is explicitly requested.

Bug Fixes

  • Resolves explicit request keys first, then explicit slots, then the stored pin.
  • Applies to shared text and image auth lookups, not just the compaction caller.
  • Unpinned credentials keep the existing default behavior; missing pinned slots no longer fall back to default or ambient credentials.
  • OAuth refresh targets the selected slot and preserves sibling credentials.
  • Adds 11 auth regression cases and a real-CLI QA driver reproducing the compaction failure.

Written for commit b4d314b. Summary will update on new commits.

Review in cubic

feelsodev and others added 3 commits September 11, 2026 00:43
Use the stored pin when no request slot is supplied, preserving explicit API-key and slot precedence. This keeps compaction on the account selected for ordinary requests and scopes OAuth refresh to that account.

Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Drive the real source CLI with isolated synthetic OAuth accounts and a localhost quota fixture. Assert compaction account selection, committed summary replay, and complete sandbox cleanup without real provider calls.

Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Preserve upstream auth changes and resolve change-tracker and changelog conflicts. Keep the pinned-account fix under Unreleased. Revalidated the complete AI suite, static checks, build, and isolated CLI compaction.

Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant