Skip to content

fix(agent-core-v2): surface provider auth errors and align agent copy with v1 - #1631

Merged
sailist merged 5 commits into
MoonshotAI:mainfrom
sailist:fix/v2-provider-auth-copy
Jul 13, 2026
Merged

fix(agent-core-v2): surface provider auth errors and align agent copy with v1#1631
sailist merged 5 commits into
MoonshotAI:mainfrom
sailist:fix/v2-provider-auth-copy

Conversation

@sailist

@sailist sailist commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Related Issue

No linked issue — the problems are explained below.

Problem

Three independent issues, two in agent-core-v2 (the DI × Scope engine) and one in kap-server:

  1. When an OAuth-managed model returns 401, the engine forces a token refresh and retries. If the 401 persists, the account itself was rejected by the provider (plan or model unavailable) — but v2 surfaced AUTH_LOGIN_REQUIRED with a "send /login" prompt, sending users into a re-login loop that cannot fix the problem. The provider's actual rejection message was discarded, and full compaction wrapped the error as a generic COMPACTION_FAILED, hiding it further.
  2. v2's repeated-tool-call reminders hard-prohibited the call ("Do not call this exact same tool again") and echoed the tool name and arguments, which proved less effective than v1's redirect-style copy (v1 fix: rewrite repeated tool call reminders to redirect instead of prohibit #1518). Separately, a dismissed AskUserQuestion was documented as answering with the recommended option, while v1 (fix(agent-core): treat dismissed AskUserQuestion as no answer, not recommended pick #1550) treats dismissal as "no answer".
  3. kap-server's instance registry had a shutdown race: a heartbeat write already in flight when release() ran could complete its atomic rename after the unlink, recreating the just-removed instance file. This also made instanceRegistry.test.ts flaky in CI.

What changed

1. Surface provider rejection on post-refresh 401

  • Map a 401 that survives a forced token refresh to PROVIDER_AUTH_ERROR carrying the provider's sanitized message, instead of a misleading AUTH_LOGIN_REQUIRED re-login prompt (modelImpl.ts).
  • Propagate provider auth errors through full compaction instead of wrapping them as COMPACTION_FAILED (fullCompactionService.ts).
  • Export sanitizeStatusErrorMessage for reuse (errors.ts).

2. Align repeat-reminder and AskUserQuestion copy with v1

3. Drain in-flight heartbeat writes before unlinking the instance file

  • Count writes that passed the released check and have release() await them before unlinking, so an atomic rename can no longer recreate the file after removal (instanceRegistry.ts).
  • Harden the heartbeat test with a 1ms cadence plus a post-release re-check; verified the new test fails on the old code and passes deterministically with the fix.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue, or explained the problem above.
  • I have added tests that prove my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset.
  • Ran gen-docs skill, or this PR needs no doc update.

sailist added 3 commits July 13, 2026 23:17
- map a 401 that survives forced token refresh to PROVIDER_AUTH_ERROR
  carrying the provider's sanitized message, instead of a misleading
  AUTH_LOGIN_REQUIRED re-login prompt
- propagate provider auth errors through full compaction instead of
  wrapping them as compaction failure
- export sanitizeStatusErrorMessage for reuse
…th v1

- rewrite repeated tool call reminders to redirect instead of prohibit (port of v1 MoonshotAI#1518)
- simplify makeReminderText2 to (repeatCount), no longer echoing tool name and args
- treat dismissed AskUserQuestion as no answer, not the recommended pick (port of v1 MoonshotAI#1550)
- update toolDedupe test assertions and regenerate affected wire snapshots
@changeset-bot

changeset-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1111f65

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@moonshot-ai/kimi-code Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Jul 13, 2026

Copy link
Copy Markdown
pnpm dlx https://pkg.pr.new/@moonshot-ai/kimi-code@1111f65
npx https://pkg.pr.new/@moonshot-ai/kimi-code@1111f65

commit: 1111f65

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a9e527fa73

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

}

function toLoginRequiredError(error: APIStatusError): Error2 {
/**

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Move helper rationale to the file header

For files under packages/agent-core-v2, packages/agent-core-v2/AGENTS.md requires comments to live only in the top-of-file header and “never beside functions.” This newly added helper block comment sits next to toProviderAuthError, so it violates the local convention; please move the rationale into the file header or make the helper/test names carry it.

Useful? React with 👍 / 👎.

sailist added 2 commits July 13, 2026 23:31
…e instance file

- count writes that passed the released check and await them in release()
  so their atomic rename cannot recreate the file after unlink
- harden the heartbeat test: 1ms cadence plus a post-release re-check
  catches the recreate-after-unlink race (verified failing on old code)
…eader

Address Codex review: agent-core-v2 keeps comments in the top-of-file
header only, never beside functions
@sailist
sailist merged commit 2d874fb into MoonshotAI:main Jul 13, 2026
14 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 14, 2026
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