feat(auth): add Kimi Code OAuth login

[euxaristia]

Summary

• Adds Kimi Code as an OAuth login option alongside ChatGPT, using the device-code flow with Kimi's required X-Msh-* vendor-identity headers
• Introduces a dedicated internal/kimiidentity package to manage the per-device Kimi identity file, including lazy creation, abandoned/empty-file repair with an ownership-safe lock, and stripping the identity headers when a request retargets off the Kimi catalog endpoint
• Provider catalog, CLI (zero auth kimi), and TUI onboarding all route through the shared device-code flow, with the desktop Enter key routed to device code and polling cancelled on every quit path

Test plan

• go test ./internal/kimiidentity/... ./internal/oauth/... ./internal/providercatalog/... ./internal/tui/... ./internal/cli/...

Summary by CodeRabbit

• New Features

  • Added Kimi Code as an OAuth provider with device-code authentication, managed coding endpoints, aliases, and identity handling.
  • Added the dedicated zero auth kimi login command.
  • Expanded OAuth setup guidance for Kimi and xAI, including presets and subscription requirements.

• Bug Fixes

  • OAuth device login now cancels cleanly when exiting, navigating, or timing out.
  • Prevented stale login results and outdated provider headers from overwriting current settings.
  • Improved support for configured headers during OAuth requests.
  • Preserved custom headers when provider endpoints are overridden.

[coderabbitai]

Walkthrough

Kimi Code is added as a device-only OAuth provider. The change adds persistent identity headers, propagates provider headers through OAuth requests, supports cancellation and stale-attempt rejection in TUI flows, adds a CLI alias, and updates profile resolution and documentation.

Changes

Kimi Code OAuth

Layer / File(s)	Summary
Persistent Kimi identity internal/kimiidentity/*	Generates sanitized identity headers and persists a device UUID with concurrent creation and repair handling.
OAuth headers and preset internal/oauth/*	Adds ExtraHeaders to OAuth requests and defines the Kimi Code device-flow preset with approved endpoint handling.
Provider catalog and runtime descriptors internal/providercatalog/*	Adds Kimi Code as a device-only OAuth provider and generates runtime headers only during direct lookup.
Cancellable device login in TUI internal/tui/*	Tracks OAuth attempts, cancels polling, rejects stale results, and starts device login automatically for Kimi Code.
Profile header resolution internal/config/resolver.go, internal/config/resolver_test.go, internal/tui/provider_wizard.go, internal/tui/provider_wizard_oauth_test.go	Preserves runtime headers for canonical endpoints and removes X-Msh-* headers from retargeted profiles.
CLI and OAuth documentation internal/cli/auth.go, docs/oauth-subscriptions.md	Adds the kimi auth alias and documents Kimi Code OAuth behavior and configuration.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• Gitlawb/zero#708: Extends the same Kimi Code OAuth implementation across presets, catalog, identity headers, CLI, OAuth flows, and onboarding.
• Gitlawb/zero#725: Shares OAuth authentication, device-login, provider-resolution, and profile-persistence changes.

Suggested reviewers: gnanam1990, vasanthdev2004, anandh8x

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding Kimi Code OAuth login support.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 9

🧹 Nitpick comments (7)

internal/kimiidentity/kimiidentity_test.go (1)

270-274: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Fix the staleness threshold in this comment.

The comment says > 1s. The production threshold in repairAbandonedDeviceID is 2*time.Second. The 5-second backdate still works, but the wrong number here will mislead the next person who tunes the threshold.

♻️ Proposed comment fix

-	// Backdate lock file mtime to make it stale (> 1s)
+	// Backdate lock file mtime to make it stale (> 2s, the repair threshold)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/kimiidentity_test.go` around lines 270 - 274, Update
the comment above the lock-file timestamp backdating in the test to state that
the mtime is made stale by exceeding the 2-second threshold used by
repairAbandonedDeviceID; leave the 5-second backdate and test logic unchanged.

internal/oauth/presets.go (2)

174-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Tighten and clarify the host allowlist.

Two points.

First, the allowlist is partly redundant. strings.HasSuffix(host, ".kimi.com") already covers auth.kimi.com and api.kimi.com, so only the bare kimi.com case needs a separate test. The suffix tests are correct as written; the leading dot prevents a notkimi.com match.

Second, the domain set looks inconsistent. .moonshot.cn is approved, but bare moonshot.cn is not, and .moonshot.ai is absent even though the existing moonshot descriptor uses api.moonshot.ai. State the intent in a comment, or narrow the list to the hosts Kimi Code actually serves.

♻️ Proposed simplification

 	host := strings.ToLower(u.Hostname())
-	return host == "auth.kimi.com" || host == "api.kimi.com" || host == "kimi.com" || strings.HasSuffix(host, ".kimi.com") || strings.HasSuffix(host, ".moonshot.cn")
+	for _, domain := range []string{"kimi.com", "moonshot.cn"} {
+		if host == domain || strings.HasSuffix(host, "."+domain) {
+			return true
+		}
+	}
+	return false
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/oauth/presets.go` around lines 174 - 181, Update isCanonicalKimiHost
to remove the redundant auth.kimi.com and api.kimi.com checks, retaining the
bare kimi.com and dotted suffix checks. Clarify the intended allowlist with a
comment or narrow it to hosts Kimi Code actually serves; ensure moonshot.cn
handling is consistent with .moonshot.cn and include or exclude .moonshot.ai
based on the existing api.moonshot.ai usage.

---

162-172: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

One non-canonical override silently strips the headers from the canonical requests too.

providerExtraHeaders returns nil if any single supplied endpoint is non-canonical. Consider an operator who overrides only ZERO_OAUTH_KIMI_CODE_TOKEN_URL to a proxy and leaves the device endpoint at auth.kimi.com. The device-authorization request then goes to the canonical Kimi host without the X-Msh-* headers. Per the comment at Lines 183-190, Kimi answers 401. The user sees an authorization failure with no indication that an override caused it.

The fail-closed choice is right; do not send identity headers to an unapproved host. The problem is that the decision is all-or-nothing and invisible.

Two options, in order of preference:

1. Decide per endpoint. Attach the headers to a request only when that request's own host is canonical. This keeps login working when only the token URL is proxied.
2. If you keep the all-or-nothing rule, surface it. Emit a warning at login time that names the offending environment variable and states that vendor-identity headers are disabled.

Also document the behavior in docs/oauth-subscriptions.md so the override is not a trap.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/oauth/presets.go` around lines 162 - 172, Update
providerExtraHeaders to decide Kimi vendor-identity headers per endpoint rather
than disabling them globally when any override is non-canonical: return headers
only for requests whose own host is canonical, while preserving the fail-closed
behavior for proxied hosts. Ensure the endpoint-to-header decision is available
to each request, and document the mixed canonical/non-canonical override
behavior in docs/oauth-subscriptions.md.

internal/oauth/flow_test.go (1)

156-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a test that asserts the extra headers reach the wire.

Both updated call sites pass nil for the new extraHeaders parameter, which is correct. No test in this cohort verifies the positive case, though. TestResolveConfigKimiCodePreset checks only that Config.ExtraHeaders is populated; nothing proves applyExtraHeaders puts those headers on the outgoing request.

Add a test with an httptest server that records r.Header and assert the X-Msh-* values arrive. Cover PostToken at minimum, and ideally RequestDeviceCode and pollDeviceOnce too, since Kimi requires the headers on all three.

The repository guidelines require a regression test for behavior changes.

Also applies to: 260-260

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/oauth/flow_test.go` at line 156, Add regression coverage for
extra-header propagation using an httptest server that records incoming request
headers. Extend the PostToken test around its extraHeaders argument to pass
representative X-Msh-* values and assert they arrive on the server request; also
cover RequestDeviceCode and pollDeviceOnce if their test fixtures permit,
ensuring all Kimi-required flows verify the headers reach the wire.

Source: Coding guidelines

internal/oauth/providers.go (1)

80-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Pass the endpoint overrides as a slice for readability.

The call takes four same-typed positional arguments after name. A reader cannot tell the order without opening providerExtraHeaders. A single []string argument, or reuse of the already-computed cfg endpoint values, would read better.

Note that this line resolves the endpoint values a second time. Lines 75-78 already compute the same environment lookups.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/oauth/providers.go` at line 80, Update the ExtraHeaders construction
in the provider configuration to reuse the endpoint values already computed in
cfg on lines 75-78, passing them as a single ordered []string to
providerExtraHeaders instead of repeating positional envValue lookups. Adjust
providerExtraHeaders as needed to accept the slice while preserving endpoint
order.

internal/tui/onboarding.go (1)

848-857: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the device-only bypass rule into one shared helper. Both wizards independently implement the same condition to decide when to skip the browser OAuth flow for a device-only provider like Kimi Code, and each file's comment notes the other must be kept in sync manually.

• internal/tui/onboarding.go#L848-L857: replace the inline descriptor.OAuthDeviceFlow && (descriptor.OAuthDeviceOnly || oauthPreferDeviceFlow()) check with a call to a shared helper, e.g. useDeviceLogin(descriptor).
• internal/tui/provider_wizard_discovery.go#L46-L51: replace the identical inline check on provider with the same shared helper, so both wizards can never diverge on this rule.

♻️ Proposed shared helper

// useDeviceLogin reports whether the OAuth flow for this provider should go
// straight to device-code login instead of the browser flow: either the
// provider has no browser/loopback endpoint at all (OAuthDeviceOnly), or the
// environment prefers device flow (headless/SSH).
func useDeviceLogin(d providercatalog.Descriptor) bool {
	return d.OAuthDeviceFlow && (d.OAuthDeviceOnly || oauthPreferDeviceFlow())
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/onboarding.go` around lines 848 - 857, Extract the shared
device-login condition into a helper such as useDeviceLogin(d
providercatalog.Descriptor), preserving the OAuthDeviceFlow, OAuthDeviceOnly,
and oauthPreferDeviceFlow logic. In internal/tui/onboarding.go lines 848-857,
replace the inline check with this helper; make the same replacement for
provider in internal/tui/provider_wizard_discovery.go lines 46-51 so both
wizards use the centralized rule.

internal/providercatalog/catalog.go (1)

452-460: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

cloneDescriptor drops runtime headers for any descriptor that also has static CustomHeaders.

The else if makes the two header sources mutually exclusive. A descriptor with both static CustomHeaders and RuntimeHeaders silently loses its runtime headers. No current descriptor has both, so this is not a live bug. The doc comment on RuntimeHeaders presents the mechanism as provider-agnostic, so merge instead of choosing.

♻️ Proposed merge of both header sources

-	if descriptor.CustomHeaders != nil {
-		descriptor.CustomHeaders = copyStringMap(descriptor.CustomHeaders)
-	} else if withRuntimeHeaders && descriptor.RuntimeHeaders != nil {
-		descriptor.CustomHeaders = descriptor.RuntimeHeaders()
-	}
+	static := copyStringMap(descriptor.CustomHeaders)
+	if withRuntimeHeaders && descriptor.RuntimeHeaders != nil {
+		merged := descriptor.RuntimeHeaders()
+		for key, value := range static {
+			merged[key] = value
+		}
+		descriptor.CustomHeaders = merged
+	} else {
+		descriptor.CustomHeaders = static
+	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/providercatalog/catalog.go` around lines 452 - 460, Update
cloneDescriptor so withRuntimeHeaders merges RuntimeHeaders into any copied
static CustomHeaders instead of making the sources mutually exclusive. Preserve
static headers, add runtime headers when available, and retain nil behavior when
neither source exists.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/config/resolver.go`:
- Around line 1095-1110: Update the conditional guarding the custom-header
cleanup in the descriptor resolution flow to use the presence of
descriptor.RuntimeHeaders rather than len(descriptor.CustomHeaders). Preserve
removal of matching catalog headers and all X-Msh-* headers for non-canonical
endpoints, including when RuntimeHeaders produces an empty CustomHeaders map.

In `@internal/kimiidentity/kimiidentity.go`:
- Around line 71-77: Remove the exported ResetDeviceIDForTest seam from the
production file to satisfy unreachable-function checks, and replace it with a
lint-compatible test seam such as an unexported deviceIDPathOverride used by the
device-ID loading path. Update tests to drive isolation through that seam while
preserving production behavior and synchronization.
- Around line 194-216: Update createOrAdoptDeviceID’s successful os.Rename path
in repairAbandonedDeviceID to re-read path with readValidDeviceID (or the
established retry helper) and return the persisted device ID when available,
falling back to id only if necessary. Preserve the existing rename and
error-handling behavior; do not add lock-mtime refresh unless separately
required.
- Around line 53-56: Run gofmt on internal/kimiidentity/kimiidentity.go to
normalize the var-block alignment and remove the consecutive blank lines after
ResetDeviceIDForTest, ensuring the file passes make fmt-check.

In `@internal/oauth/providers_test.go`:
- Around line 90-101: Prevent device-ID test state from leaking across platforms
and test order by exporting a shared user-config redirect helper from
internal/kimiidentity, based on setUserConfigRoot, that redirects APPDATA, HOME,
and XDG_CONFIG_HOME to t.TempDir() and resets the cached ID via t.Cleanup. Use
this helper in internal/oauth/providers_test.go at lines 90-101 and
internal/oauth/presets_test.go at lines 191-197; the latter must no longer omit
HOME. Also apply the same helper to internal/providercatalog/catalog_test.go’s
TestKimiRuntimeHeadersOnlyOnGet.
- Around line 104-107: Run gofmt on the map literal passed to ResolveConfig in
the relevant test, correcting the spacing between the ZERO_OAUTH_ALLOW_PRESETS
key and its value to match the longest key. Then run gofmt -l ./... and ensure
no formatting drift remains.

In `@internal/providercatalog/catalog_test.go`:
- Around line 494-499: Real Kimi device IDs are written outside the test sandbox
because config-root isolation is incomplete. Add a shared helper modeled on
setKimiUserConfigRoot that sets XDG_CONFIG_HOME, APPDATA, and HOME to
t.TempDir() and resets the device ID before and after; use it at
internal/providercatalog/catalog_test.go lines 494-499, 458-489 before Get
calls, and at internal/tui/provider_wizard_oauth_test.go lines 624-642 before
mouseTestModel().

In `@internal/providercatalog/catalog.go`:
- Around line 263-290: Remove the unused ListByTransport, ValidTransport, and
ValidAPIFormat helpers from the change. Do not add replacement abstractions or
broaden the scope; only retain these helpers if a production caller is added and
uses them.

In `@internal/tui/provider_wizard.go`:
- Around line 2144-2155: Update the Kimi profile construction around
providercatalog.Get and the profile.CustomHeaders assignment to preserve the
re-resolved X-Msh-* identity headers expected by
TestProviderWizardProfileAppliesKimiRuntimeHeaders. Remove the provider.ID ==
"kimi-code" strip loop, leaving the cloned runtime descriptor headers in
profile.CustomHeaders.

---

Nitpick comments:
In `@internal/kimiidentity/kimiidentity_test.go`:
- Around line 270-274: Update the comment above the lock-file timestamp
backdating in the test to state that the mtime is made stale by exceeding the
2-second threshold used by repairAbandonedDeviceID; leave the 5-second backdate
and test logic unchanged.

In `@internal/oauth/flow_test.go`:
- Line 156: Add regression coverage for extra-header propagation using an
httptest server that records incoming request headers. Extend the PostToken test
around its extraHeaders argument to pass representative X-Msh-* values and
assert they arrive on the server request; also cover RequestDeviceCode and
pollDeviceOnce if their test fixtures permit, ensuring all Kimi-required flows
verify the headers reach the wire.

In `@internal/oauth/presets.go`:
- Around line 174-181: Update isCanonicalKimiHost to remove the redundant
auth.kimi.com and api.kimi.com checks, retaining the bare kimi.com and dotted
suffix checks. Clarify the intended allowlist with a comment or narrow it to
hosts Kimi Code actually serves; ensure moonshot.cn handling is consistent with
.moonshot.cn and include or exclude .moonshot.ai based on the existing
api.moonshot.ai usage.
- Around line 162-172: Update providerExtraHeaders to decide Kimi
vendor-identity headers per endpoint rather than disabling them globally when
any override is non-canonical: return headers only for requests whose own host
is canonical, while preserving the fail-closed behavior for proxied hosts.
Ensure the endpoint-to-header decision is available to each request, and
document the mixed canonical/non-canonical override behavior in
docs/oauth-subscriptions.md.

In `@internal/oauth/providers.go`:
- Line 80: Update the ExtraHeaders construction in the provider configuration to
reuse the endpoint values already computed in cfg on lines 75-78, passing them
as a single ordered []string to providerExtraHeaders instead of repeating
positional envValue lookups. Adjust providerExtraHeaders as needed to accept the
slice while preserving endpoint order.

In `@internal/providercatalog/catalog.go`:
- Around line 452-460: Update cloneDescriptor so withRuntimeHeaders merges
RuntimeHeaders into any copied static CustomHeaders instead of making the
sources mutually exclusive. Preserve static headers, add runtime headers when
available, and retain nil behavior when neither source exists.

In `@internal/tui/onboarding.go`:
- Around line 848-857: Extract the shared device-login condition into a helper
such as useDeviceLogin(d providercatalog.Descriptor), preserving the
OAuthDeviceFlow, OAuthDeviceOnly, and oauthPreferDeviceFlow logic. In
internal/tui/onboarding.go lines 848-857, replace the inline check with this
helper; make the same replacement for provider in
internal/tui/provider_wizard_discovery.go lines 46-51 so both wizards use the
centralized rule.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6fe85d3e-89da-4094-adb6-1e5d3b99e47c

📥 Commits

Reviewing files that changed from the base of the PR and between 8e26679 and ba6ed17.

📒 Files selected for processing (26)

• docs/oauth-subscriptions.md
• internal/cli/auth.go
• internal/config/resolver.go
• internal/config/resolver_test.go
• internal/kimiidentity/kimiidentity.go
• internal/kimiidentity/kimiidentity_test.go
• internal/oauth/device.go
• internal/oauth/flow.go
• internal/oauth/flow_test.go
• internal/oauth/manager.go
• internal/oauth/oauth.go
• internal/oauth/presets.go
• internal/oauth/presets_test.go
• internal/oauth/providers.go
• internal/oauth/providers_test.go
• internal/providercatalog/catalog.go
• internal/providercatalog/catalog_test.go
• internal/providercatalog/export_test.go
• internal/providercatalog/oauth_test.go
• internal/tui/model.go
• internal/tui/oauth_device.go
• internal/tui/onboarding.go
• internal/tui/onboarding_test.go
• internal/tui/provider_wizard.go
• internal/tui/provider_wizard_discovery.go
• internal/tui/provider_wizard_oauth_test.go

💤 Files with no reviewable changes (1)

• internal/providercatalog/export_test.go

[euxaristia]

Addressed CodeRabbit findings in 910eb37:

1. Gate X-Msh-* stripping on RuntimeHeaders (not only non-empty CustomHeaders).
2. Dropped exported ResetDeviceIDForTest (path-keyed cache + config-root isolation instead).
3. Re-read device id after repair rename / WriteFile fallback.
4. Isolate XDG_CONFIG_HOME/APPDATA/HOME in all tests that touch Kimi device id.
5. Removed unused ListByTransport / ValidTransport / ValidAPIFormat.

@coderabbitai full review

[coderabbitai]

@euxaristia: I will perform a full review of the changes, including the fixes in 910eb37.

✅ Action performed

Full review finished.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 59 minutes.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/providercatalog/catalog_test.go (1)

482-503: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Assert that listing does not create a device ID.

The test only checks that listing descriptors have no CustomHeaders. A regression could invoke RuntimeHeaders, discard the returned headers, and still persist the Kimi device ID.

After All() and OAuthProviders(), assert that the isolated config root has no Kimi device-ID file before calling Get("kimi-code").

As per coding guidelines, **/*_test.go requires a regression test for behavior changes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/providercatalog/catalog_test.go` around lines 482 - 503, Extend
TestKimiRuntimeHeadersOnlyOnGet after the All() and OAuthProviders() listing
checks to assert that the isolated config root contains no persisted Kimi
device-ID file. Use the test’s existing isolation and device-ID storage symbols,
and keep the assertion before Get("kimi-code") so listing cannot create or
persist the device ID.

Source: Coding guidelines

🧹 Nitpick comments (1)

internal/kimiidentity/kimiidentity.go (1)

65-79: 🩺 Stability & Availability | 🔵 Trivial

Run identity handling under the race detector.

This package uses goroutines and shared identity-file repair paths, so run internal/kimiidentity with -race in an environment where Go can enable the race detector.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/kimiidentity.go` around lines 65 - 79, Run the
internal/kimiidentity package tests with Go’s race detector enabled, using an
environment/toolchain where race detection is supported; verify that DeviceID
and the shared identity-file repair paths complete without reported data races.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/kimiidentity/kimiidentity_test.go`:
- Line 13: Add a regression test in the test flow around IsolateDeviceIDStorage
that uses two temporary config roots sequentially, calls DeviceID after
switching roots, and verifies each root receives its own device ID rather than
reusing the cached value from the first root.

---

Outside diff comments:
In `@internal/providercatalog/catalog_test.go`:
- Around line 482-503: Extend TestKimiRuntimeHeadersOnlyOnGet after the All()
and OAuthProviders() listing checks to assert that the isolated config root
contains no persisted Kimi device-ID file. Use the test’s existing isolation and
device-ID storage symbols, and keep the assertion before Get("kimi-code") so
listing cannot create or persist the device ID.

---

Nitpick comments:
In `@internal/kimiidentity/kimiidentity.go`:
- Around line 65-79: Run the internal/kimiidentity package tests with Go’s race
detector enabled, using an environment/toolchain where race detection is
supported; verify that DeviceID and the shared identity-file repair paths
complete without reported data races.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5df72e9e-020b-4def-a02f-2272ed0acc15

📥 Commits

Reviewing files that changed from the base of the PR and between ba6ed17 and 910eb37.

📒 Files selected for processing (11)

• internal/config/resolver.go
• internal/config/resolver_test.go
• internal/kimiidentity/export_test.go
• internal/kimiidentity/kimiidentity.go
• internal/kimiidentity/kimiidentity_test.go
• internal/oauth/presets_test.go
• internal/oauth/providers_test.go
• internal/providercatalog/catalog.go
• internal/providercatalog/catalog_test.go
• internal/tui/provider_wizard.go
• internal/tui/provider_wizard_oauth_test.go

💤 Files with no reviewable changes (1)

• internal/tui/provider_wizard.go

🚧 Files skipped from review as they are similar to previous changes (5)

• internal/config/resolver.go
• internal/config/resolver_test.go
• internal/tui/provider_wizard_oauth_test.go
• internal/oauth/providers_test.go
• internal/oauth/presets_test.go

[coderabbitai]

@euxaristia: I will perform a full review of pull request #851. I will verify the device-ID cache isolation and that catalog listings do not create the Kimi device-ID file.

✅ Action performed

Full review finished.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 52 minutes.

[Vasanthdev2004]

Thanks for the very thorough work here — the OAuth plumbing itself is in good shape and I checked the parts that usually go wrong on this path:

• Redirects are refused on all four credential-bearing POSTs (RequestDeviceCode, pollDeviceOnce, PostToken, and Refresh via PostToken), so the new ExtraHeaders can't ride a 307 to another origin.
• I drove a throwaway httptest TLS server through all four and confirmed the X-Msh-* values actually reach the wire — that part works.
• providerExtraHeaders fails closed when an endpoint is overridden off a canonical Kimi host, and resolving a retargeted kimi-code profile through the real config.Resolve entry point does drop the identity headers while keeping user-supplied ones.
• The kimi alias still resolves to moonshot, gofmt/go vet are clean, and ./internal/{kimiidentity,oauth,providercatalog,config,tui} all pass (the only internal/cli failure is the pre-existing symlink-privilege one).

That said, I'd like changes before this merges. The problems are all in the device-identity lifecycle, not the protocol work.

1. First-run onboarding mints the Kimi device id for every user. internal/tui/onboarding.go:117 — setupOAuthProviderOptions filters the provider list by calling providercatalog.Get(option.ID) on every option, and Get now runs RuntimeHeaders → kimiidentity.DeviceID(). That function is reached from setupMethodOptions (onboarding.go:541), which is called from the render path setupMethodLines (onboarding.go:1665) as well as advanceSetup (onboarding.go:827). So the "How do you want to connect?" screen creates <UserConfigDir>/zero/kimi-device-id on first paint, before anyone has picked a provider.

I proved it with a temp config root: after All() and OAuthProviders() the file does not exist, and after one setupOAuthProviderOptions call it does. That's the exact invariant cloneDescriptor's own comment claims to protect ("merely enumerating providers never mints ~/.config/zero/kimi-device-id for users who never touch Kimi"). The filter only needs descriptor.OAuth, which the listing descriptors already carry — use OAuthProviders()/All() here, or add a headers-free lookup. Please add a test that walks the method + provider screens and asserts the file was never created; right now nothing would catch this coming back.

2. The wizard bakes the hostname and device UUID into config.json, and the resolver ignores them anyway. internal/tui/provider_wizard.go:2145 copies the runtime headers into profile.CustomHeaders, and applyProviderWizard persists that profile through config.UpsertProvider. I upserted a wizard-built profile into a temp config and read it back:

"customHeaders": {
  "X-Msh-Device-Id": "b5fd6ed8-50fb-4eec-8195-87571e6c8f60",
  "X-Msh-Device-Model": "windows amd64",
  "X-Msh-Device-Name": "Vasanth",
  ...
}

X-Msh-Device-Name is os.Hostname(). Two problems with persisting it. First, it's dead weight: applyCatalogDescriptor at internal/config/resolver.go:1074 deliberately skips stored x-msh-* values so the freshly minted ones win, so nothing ever reads the persisted copy. Second, it puts a machine hostname and a stable device UUID into a file people paste into bug reports, and the strip in resolver.go:1112 only fires while catalogId is present — resolving a profile with those headers, catalogId removed and baseURL: https://proxy.example.test/v1 forwards X-Msh-Device-Id and X-Msh-Device-Name to that host untouched. Note zero auth kimi (EnsureCatalogProvider) does not persist them, so the CLI and TUI already disagree. Simplest fix: keep the wizard's in-memory profile for the discovery call, but don't write RuntimeHeaders-derived headers to disk — the resolver re-attaches them at resolve time.

3. Both new guards in applyCatalogDescriptor are unproven. I removed the x-msh- prefix delete (resolver.go:1112-1116) and the stored-header override guard (resolver.go:1074-1076) and ran go test ./internal/config/ -count=1 — fully green. TestApplyCatalogDescriptorStripsKimiIdentityFromRetargetedProfile (resolver_test.go:1577) passes only because Require("kimi-code") returns a CustomHeaders map containing every X-Msh-* key, so the pre-existing EqualFold catalog-key loop already deletes both of the profile's headers. The only new code that test exercises is the widened else if condition. Please add: a case where the descriptor has RuntimeHeaders but empty CustomHeaders (the case the prefix branch exists for), and a case where a stale persisted X-Msh-Device-Id must lose to the freshly minted one on the canonical endpoint. Both should go red if the corresponding line is deleted.

4. Two existing tests were deleted rather than updated.

• internal/providercatalog/catalog_test.go — TestListByTransportPreservesCatalogOrder is gone (it was at line 364 on main), along with its export_test.go helper. That test pinned per-transport catalog ordering and the TransportOpenAICompatible/TransportAnthropicCompatible alias normalisation. Adding kimi-code into the openai-compatible run breaks its wantIDs list, so the fix was one line: add "kimi-code" after "chatgpt". Deleting the only ordering guard to make a new descriptor fit isn't a trade I want to make. (Swapping ValidTransport/ValidAPIFormat for inline maps in TestCatalogDescriptorsExposeRequiredDefaults is fine — that keeps the per-descriptor invariant.)
• internal/oauth/providers_test.go — TestEnvKey was replaced in place by TestResolveConfigKimiCodeStripsExtraHeadersOnEndpointOverride. envKey is still production code and is exactly what maps kimi-code → ZERO_OAUTH_KIMI_CODE_*; put the old test back alongside the new one.

One thing that needs a decision, not a code change. internal/providercatalog/catalog.go:158 says the bearer is accepted directly "so no client spoofing is involved", while internal/kimiidentity/kimiidentity.go:30-37 says X-Msh-Platform must be kimi_code_cli because the coding/v1 endpoint runs a client whitelist. Those can't both be true, and the second one is the operative claim — we'd be presenting Zero as Moonshot's own CLI to get past an allowlist. That's an owner call and I'd like it made explicitly before this lands, same as we've done for other provider presets. Related: your own comments say the header names and values are reverse-engineered from kimi-cli and "should be confirmed against a real login before this ships" (and X-Msh-Version currently ships the literal string "unknown"). Has anyone run a real Kimi Code login end to end against this branch? If not, that's the last gate.

Smaller things, none blocking:

• internal/oauth/presets.go:162-172: overriding one endpoint (say only ZERO_OAUTH_KIMI_CODE_TOKEN_URL) drops the identity headers from all requests, including the ones still going to auth.kimi.com, which by your own comment means a 401 the user can't diagnose. Either decide per endpoint, or surface which env var disabled them.
• internal/oauth/presets.go:174-180: the allowlist takes .moonshot.cn but not bare moonshot.cn and not .moonshot.ai, while the existing moonshot descriptor uses api.moonshot.ai. Add a line saying that's deliberate or tighten the list.
• No test asserts ExtraHeaders reach the wire. I verified by hand that they do, but given the entire feature hinges on it, an httptest server recording r.Header across RequestDeviceCode / pollDeviceOnce / PostToken / Refresh is worth having.
• internal/kimiidentity/kimiidentity.go:168: the 2s lock-staleness threshold is never refreshed while the holder works, so a stalled holder can have its lock broken mid-publish and the two racers can walk away with different ids than what's on disk. Narrow, and it needs an already-corrupt file to reach — but consider touching the lock mtime while working.
• docs/oauth-subscriptions.md documents the X-Msh-* requirement but not that one of those headers is the user's machine hostname. Worth saying out loud since it goes to a third party on every completion.

Happy to re-review as soon as 1-4 are addressed.

Everything in one pass as usual, nothing held back for a second round. The two deleted tests are the ones I'd push back on hardest — a guard removed to make a new descriptor fit is the kind of thing that's invisible in six months, and both look like one-line updates rather than deletions.

[euxaristia]

Addressed Vasanth review items 1-4:

1. Onboarding no longer mints kimi-device-id on paint. setupOAuthProviderOptions filters via OAuthProviders() listing clones instead of Get() / RuntimeHeaders. Regression: TestSetupMethodAndOAuthProviderScreensDoNotMintKimiDeviceID.

2. Wizard does not persist X-Msh-* into config.json. In-memory / discovery profiles still carry runtime headers; stripRuntimeIdentityHeaders runs before UpsertProvider. Resolver re-attaches on canonical endpoints (matches CLI zero auth kimi).

3. applyCatalogDescriptor guards covered. Prefix strip with empty CustomHeaders + set RuntimeHeaders; stale persisted device id loses to fresh runtime headers on the canonical endpoint.

4. Restored deleted tests. TestListByTransportPreservesCatalogOrder (with kimi-code in openai-compat order) and TestEnvKey.

Owner decision still open: catalog comment says bearer is accepted with "no client spoofing" while X-Msh-Platform must be kimi_code_cli for the managed-endpoint allowlist. Real end-to-end Kimi Code login on this branch also still wanted as the last gate.

[Vasanthdev2004]

Approving. Re-checked on d41e06fd and all four are properly closed — I verified rather than read.

Onboarding no longer mints the device id. Filtering via OAuthProviders() listing clones instead of Get()/RuntimeHeaders is the right fix — it removes the call rather than guarding it, so there's no path left that reaches DeviceID() from a render.

The wizard no longer persists identity headers. stripRuntimeIdentityHeaders is genuinely pinned: I disabled it and TestStripRuntimeIdentityHeadersDropsXMshOnly goes red with the persist copy still carrying X-Msh-Device-Id, X-Msh-Device-Name and X-Msh-Platform — while correctly keeping X-User-Agent. That last part matters; a strip that took everything would have passed a weaker test.

Both deleted tests are back. TestListByTransportPreservesCatalogOrder returns with kimi-code added to its expected list, which was the one-line fix, and TestEnvKey sits alongside the new endpoint-override test rather than replacing it. Thanks for restoring them rather than arguing the point — the catalog-ordering guard is exactly the kind of thing nobody misses until it's needed.

internal/oauth, internal/providercatalog and internal/config all green here. The single internal/tui failure is TestAltScreenTranscriptScrollKeepsFooterFixed, which fails on origin/main for me too and passes in CI — a symlink/terminal quirk on my box, unrelated to this.

[euxaristia]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 28 minutes.

[coderabbitai]

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/cli/auth.go`:
- Around line 606-612: Inspect newAuthManager to verify the AllowPresets value
used by auth login, then update the conflicting provider descriptions to match
that shipped behavior. In internal/cli/auth.go lines 606-612, correct the stale
Kimi or xAI help text; in docs/oauth-subscriptions.md lines 84-109, align both
the xAI opt-in and Kimi opt-out statements with the same result.

In `@internal/kimiidentity/kimiidentity.go`:
- Around line 164-175: The stale-lock recovery in the lock acquisition flow must
not reclaim locks solely based on the two-second mtime threshold. Update the
logic around os.ReadFile, os.Stat, and os.Rename to use a renewable lease or
another ownership protocol that proves the holder is dead before reclamation,
failing closed when ownership or permission cannot be verified; preserve atomic
lock replacement only after that proof.
- Around line 116-138: Update the device-ID publication flow around
readValidDeviceIDWithRetry and repairAbandonedDeviceID to write the complete ID
to a temporary file, check WriteString, Sync, and Close errors, then atomically
replace the destination while holding the repair lock. Apply the same atomic
publication and failure propagation to the initial creation path, returning an
error or otherwise signaling failure instead of an ID that was not persisted;
preserve post-publication rereading for race convergence.
- Around line 186-193: Update the deferred cleanup in the lock-acquisition flow
around lock.WriteString, lock.Sync, and lock.Close so failures from lock.Close
or removing lockPath are propagated to the caller or otherwise prevent a
successful device ID result. Only remove lockPath when its contents still match
ownerToken, preserving pre-existing or concurrently replaced locks, and ensure
cleanup failure causes the repair operation to fail closed.
- Around line 87-111: Update loadOrCreateDeviceIDAt and the helpers it invokes,
including readValidDeviceID and createOrAdoptDeviceID, to perform all device-ID,
lock, temporary-file, and rename operations relative to an opened
configuration-root handle using traversal-resistant, no-follow operations and
platform reparse-point protections. Remove direct path-based os.ReadFile,
os.MkdirAll, os.OpenFile, os.WriteFile, and os.Rename usage for these
operations; do not rely on pre-open EvalSymlinks, and preserve the existing
race-safe create-or-adopt behavior.

In `@internal/oauth/manager.go`:
- Around line 201-205: Update the token persistence flow around ProviderKey and
m.store.Save to serialize context cancellation with the commit, using an attempt
generation or shared lock so cancellation and token persistence are mutually
exclusive. Ensure a device-login attempt that is canceled before commit cannot
overwrite stored authentication state.

In `@internal/oauth/oauth.go`:
- Around line 117-126: Add request-level httptest regression coverage for
applyExtraHeaders across device authorization, device polling, code exchange,
and refresh flows. Assert Kimi ExtraHeaders are received by each HTTP handler,
and verify a provider without ExtraHeaders sends none; keep the existing
configuration-resolution tests intact.

In `@internal/oauth/presets.go`:
- Around line 174-180: Restrict isCanonicalKimiHost to an explicit allowlist of
approved OAuth endpoint hosts instead of accepting arbitrary *.kimi.com or
*.moonshot.cn subdomains. Preserve URL parsing and case normalization, then add
tests covering every approved host and representative unapproved subdomains to
verify identity headers are only applied to verified endpoints.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 341e6d92-2d2c-42df-9aca-d2d7586569b4

📥 Commits

Reviewing files that changed from the base of the PR and between 8e26679 and d41e06f.

📒 Files selected for processing (27)

• docs/oauth-subscriptions.md
• internal/cli/auth.go
• internal/config/resolver.go
• internal/config/resolver_test.go
• internal/kimiidentity/export_test.go
• internal/kimiidentity/kimiidentity.go
• internal/kimiidentity/kimiidentity_test.go
• internal/oauth/device.go
• internal/oauth/flow.go
• internal/oauth/flow_test.go
• internal/oauth/manager.go
• internal/oauth/oauth.go
• internal/oauth/presets.go
• internal/oauth/presets_test.go
• internal/oauth/providers.go
• internal/oauth/providers_test.go
• internal/providercatalog/catalog.go
• internal/providercatalog/catalog_test.go
• internal/providercatalog/export_test.go
• internal/providercatalog/oauth_test.go
• internal/tui/model.go
• internal/tui/oauth_device.go
• internal/tui/onboarding.go
• internal/tui/onboarding_test.go
• internal/tui/provider_wizard.go
• internal/tui/provider_wizard_discovery.go
• internal/tui/provider_wizard_oauth_test.go

[euxaristia]

CodeRabbit follow-up (c038f96)

Addressed open review findings on Kimi OAuth identity / presets:

Fixed

1. Preset opt-in docs/help — CLI help + oauth-subscriptions.md now match AllowPresets: true on zero auth login for both xAI and Kimi.
2. Rooted device-id storage — load/create/repair use os.OpenRoot on the config root + relative ops under zero/.
3. Atomic device-id publish — complete temp + checked write/sync/close + exclusive claim + rename; no direct WriteFile fallback.
4. Dead-PID lock reclaim — repair locks reclaimed via lockutil.ReclaimStaleLock + process liveness, not mtime.
5. Cancel vs token commit — CompleteDeviceLogin serializes ctx.Err() check with store.Save under deviceCommitMu.
6. ExtraHeaders httptest tests — device auth, poll, exchange, refresh + negative cases.
7. Identity host allowlist — only auth.kimi.com and api.kimi.com.

Skipped

8. Lock cleanup failure fail-closed — DeviceID is string-only; leftover lock after successful publish is recoverable via valid-id read / dead-PID reclaim. Ownership-checked remove retained.

Verify

go test ./internal/kimiidentity/ ./internal/oauth/ -count=1
go test ./internal/cli/ -count=1 -run "TestAuth|TestWriteAuth|TestNewAuth|TestRunAuth"

@coderabbitai full review

[coderabbitai]

Rate Limit Exceeded

@euxaristia have exceeded the limit for the number of chat messages per hour. Please wait 13 minutes and 16 seconds before sending another message.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/kimiidentity/process_alive_windows.go`:
- Around line 10-26: Make process liveness checks fail closed. In
internal/kimiidentity/process_alive_windows.go, update processAlive so pid <= 0
remains false, but OpenProcess failures and GetExitCodeProcess failures return
true; only a successfully queried process with a non-STILL_ACTIVE exit code
should return false. In internal/kimiidentity/process_alive_posix.go, update
processAlive to return false only for syscall.ESRCH, while treating nil and
syscall.EPERM—and other inconclusive errors—as alive.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: da252ea1-5e47-464a-9ce7-6d81bbbdbe70

📥 Commits

Reviewing files that changed from the base of the PR and between d41e06f and c038f96.

📒 Files selected for processing (11)

• docs/oauth-subscriptions.md
• internal/cli/auth.go
• internal/kimiidentity/kimiidentity.go
• internal/kimiidentity/kimiidentity_test.go
• internal/kimiidentity/process_alive_posix.go
• internal/kimiidentity/process_alive_windows.go
• internal/oauth/device_test.go
• internal/oauth/flow_test.go
• internal/oauth/manager.go
• internal/oauth/presets.go
• internal/oauth/providers_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• internal/cli/auth.go
• internal/kimiidentity/kimiidentity.go
• docs/oauth-subscriptions.md

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/kimiidentity/process_alive_windows.go`:
- Line 24: Update the process liveness logic around OpenProcess to request
SYNCHRONIZE and query termination with windows.WaitForSingleObject(handle, 0)
instead of interpreting the exit code 259. Return dead for WAIT_OBJECT_0, live
for WAIT_TIMEOUT and query errors, while preserving handle cleanup. Add a
Windows regression test covering a child process that exits with code 259.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 10a3cd12-b5ef-4090-9a65-9571343e2da3

📥 Commits

Reviewing files that changed from the base of the PR and between c038f96 and 4ddd0c6.

📒 Files selected for processing (2)

• internal/kimiidentity/process_alive_posix.go
• internal/kimiidentity/process_alive_windows.go

[euxaristia]

Addressed the Windows processAlive major (exit code 259 / STILL_ACTIVE ambiguity) in f3abea4: liveness is now SYNCHRONIZE + zero-timeout WaitForSingleObject, with fail-closed defaults preserved and a Windows regression test for exit code 259.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/kimiidentity/process_alive_windows_test.go`:
- Around line 12-33: Update TestProcessAliveExitCode259IsDead to open and retain
a SYNCHRONIZE handle for the child process before calling cmd.Wait. Wait on that
handle to confirm termination, keep it open while asserting processAlive(pid) is
false, then close the handle and call cmd.Wait afterward while preserving the
ExitError code-259 validation.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: db2cb8dc-abe2-46e8-bf07-23fb48db4cf1

📥 Commits

Reviewing files that changed from the base of the PR and between 4ddd0c6 and f3abea4.

📒 Files selected for processing (2)

• internal/kimiidentity/process_alive_windows.go
• internal/kimiidentity/process_alive_windows_test.go

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate relevant files =="
fd -a 'process_alive_windows(test)?\.go$' . || true

echo "== file outline =="
for f in $(fd 'process_alive_windows(test)?\.go$' .); do
  echo "--- $f"
  wc -l "$f"
done

echo "== process_alive_windows_test.go =="
test -f internal/kimiidentity/process_alive_windows_test.go && cat -n internal/kimiidentity/process_alive_windows_test.go

echo "== process_alive_windows.go =="
test -f internal/kimiidentity/process_alive_windows.go && cat -n internal/kimiidentity/process_alive_windows.go

echo "== all processAlive usages =="
rg -n "processAlive|ProcessAlive|STILL_ACTIVE|WAIT_OBJECT_0|ERROR_INVALID_PARAMETER" .

Repository: Gitlawb/zero

Length of output: 8745

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Behavioral probe: demonstrate the Go exec test-process lifetime semantics without repository changes.
# We cannot execute repository code, but Go's "cmd.Wait closes handles" behavior is documented and
# can be observed by checking whether a fresh "go tool" invocation can open the child PID immediately
# after cmd.Wait() returns and cmd.Process has not been assigned elsewhere.
#
# This small standalone probe must fail fast on non-Windows; the repository implementation uses
# Windows APIs, so do not treat the probe result as a platform-independent proof.
if [ "$(go env GOOS)" = "windows" ]; then
  cat > /tmp/probe_wait_pid_lifetime.go <<'GO'
package main

import (
	"fmt"
	"os"
	"os/exec"
	"syscall"
)

func main() {
	cmd := exec.Command("cmd", "/C", "exit 259")
	if err := cmd.Start(); err != nil {
		fmt.Fprintf(os.Stderr, "Start: %v\n", err)
		os.Exit(1)
	}
	pid := cmd.Process.Pid
	if err := cmd.Wait(); err == nil {
		fmt.Fprintln(os.Stderr, "Wait unexpectedly succeeded")
		os.Exit(1)
	}
	proc, err := os.FindProcess(pid)
	if err == nil {
		handle := proc.Phandle
		fmt.Printf("find: err=%v pid=%v handle=%v\n", err, pid, handle)
	} else {
		fmt.Printf("find: err=%v pid=%v\n", err, pid)
		h, err := syscall.OpenProcess(syscall.SYNCHRONIZE, false, uint32(pid))
		fmt.Printf("syscall: err=%v pid=%v handle=%v\n", err, pid, h)
	}
	os.Exit(0)
}
GO
  set +e
  go run /tmp/probe_wait_pid_lifetime.go
  status=$?
  set -e
  echo "probe exit status=$status"
else
  echo "probe skipped: GOOS=$(go env GOOS); repository implementation is Windows-only"
fi

Repository: Gitlawb/zero

Length of output: 219

---

🌐 Web query:

Go exec.Cmd Wait Windows process handle documentation

💡 Result:

In Go's os/exec package, Cmd.Wait is the standard method for waiting for an external process to exit and cleaning up its associated resources [1][2]. When running on Windows, this process involves both waiting for the process to terminate and ensuring that I/O operations (stdin, stdout, and stderr) are fully closed [1][2]. Core behavior on Windows: 1. Process Termination: Cmd.Wait internally calls os.Process.Wait, which uses the Windows API WaitForSingleObject (specifically with the process handle) to block until the process exits [3]. Once the process terminates, it also retrieves the exit code using GetExitCodeProcess and process timing information via GetProcessTimes [3]. 2. Resource Cleanup: Cmd.Wait is responsible for releasing resources associated with the Cmd struct, such as closing file descriptors (pipes) used for standard I/O [1][2]. Even if the process itself has terminated, Cmd.Wait may continue to block until all copying operations to stdin or from stdout/stderr have finished [2]. If these I/O operations are not handled or if they hang, Wait will not return, which is a known point of divergence from os.Process.Wait (which only cares about process exit) [4]. 3. Handle Management: On Windows, every os.Process contains a handle to the process, which is managed internally [3]. The os/exec package ensures this handle is acquired and released correctly during the waiting process [3]. The os/exec implementation on Windows is specifically designed to handle these handles via handleTransientAcquire and handleTransientRelease to maintain correct system resource state [3]. 4. Error Handling: Cmd.Wait returns an *ExitError if the process exits with a non-zero status code [1][2]. It also handles specific Windows error codes related to pipe I/O (such as ERROR_BROKEN_PIPE or ERROR_NO_DATA), ensuring that operations like stdin writes do not cause spurious errors if the process exits unexpectedly [5]. In summary, while os.Process.Wait is a low-level synchronization primitive targeting the process handle, exec.Cmd.Wait is a higher-level wrapper that manages the lifecycle of the process and its associated I/O streams [2][4]. It is required to be called after Start to prevent resource leaks [1].

Citations:

• 1: https://pkg.go.dev/os/exec?GOOS=windows
• 2: https://go.dev/src/os/exec/exec.go?m=text
• 3: https://github.com/golang/go/blob/master/src/os/exec_windows.go
• 4: os/exec: Inconsistent behaviour in exec.Cmd.Wait golang/go#20730
• 5: windows: exec.Cmd.Wait returns an error when the process exits without reading stdin golang/go#20445

---

Keep a liveness handle open before asserting the terminated-process branch.

cmd.Wait() closes the process handle resource held by cmd.Process, so processAlive(pid) can take the ERROR_INVALID_PARAMETER return path instead of the intended WAIT_OBJECT_0 signal path. Create the SYNCHRONIZE handle before waiting, wait on that handle to signal, keep it open during processAlive(pid), then call cmd.Wait() after the assertion.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/process_alive_windows_test.go` around lines 12 - 33,
Update TestProcessAliveExitCode259IsDead to open and retain a SYNCHRONIZE handle
for the child process before calling cmd.Wait. Wait on that handle to confirm
termination, keep it open while asserting processAlive(pid) is false, then close
the handle and call cmd.Wait afterward while preserving the ExitError code-259
validation.

Source: Coding guidelines

[euxaristia]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 3

♻️ Duplicate comments (1)

internal/kimiidentity/process_alive_windows_test.go (1)

16-33: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

The test does not exercise the WAIT_OBJECT_0 branch it claims to cover.

This was raised in a previous review and is still present. cmd.Wait() releases the process handle that cmd.Process holds. The _ = cmd.Process statement at Line 29 is a no-op; it retains nothing. After Wait returns, the PID can be freed, so processAlive(pid) returns false through the ERROR_INVALID_PARAMETER path in OpenProcess, not through WaitForSingleObject returning WAIT_OBJECT_0. The test therefore passes without proving that exit code 259 is handled correctly, and it can flake if the operating system recycles the PID onto a live process before the probe.

Open a SYNCHRONIZE handle before cmd.Wait(), keep it open across the assertion, and call cmd.Wait() after the assertion. The retained handle keeps the PID reserved and forces the wait path.

💚 Proposed fix: retain a SYNCHRONIZE handle across the probe

 	pid := cmd.Process.Pid
-	err := cmd.Wait()
+	// Hold an independent handle so the PID cannot be freed or recycled while
+	// we probe; this forces the WaitForSingleObject path in processAlive.
+	handle, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid))
+	if err != nil {
+		t.Fatalf("OpenProcess: %v", err)
+	}
+	defer windows.CloseHandle(handle)
+
+	err = cmd.Wait()
 	var ee *exec.ExitError
 	if !errors.As(err, &ee) || ee.ExitCode() != 259 {
 		t.Fatalf("Wait: %v (want ExitError with code 259)", err)
 	}
-	// Keep a live reference so the process object is still openable by PID
-	// while we probe (Go retains a handle until Process is released).
-	_ = cmd.Process
 	if processAlive(pid) {
 		t.Fatalf("processAlive(%d) = true after exit code 259; want false (dead)", pid)
 	}

Add the import:

 import (
 	"errors"
 	"os"
 	"os/exec"
 	"testing"
+
+	"golang.org/x/sys/windows"
 )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/process_alive_windows_test.go` around lines 16 - 33,
Update TestProcessAliveExitCode259IsDead to open and retain a Windows
SYNCHRONIZE handle for the child process before calling cmd.Wait(), using the
existing process-aliveness implementation’s expected handle type. Probe
processAlive while that handle remains open so the check reaches the
WAIT_OBJECT_0 path, then call cmd.Wait() afterward and close the retained handle
reliably; remove the ineffective _ = cmd.Process statement.

Source: Coding guidelines

🧹 Nitpick comments (6)

internal/providercatalog/catalog.go (1)

416-430: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Remove RuntimeHeaders from listing copies.

cloneDescriptor(descriptor, false) leaves RuntimeHeaders callable. A catalog or TUI consumer can invoke the callback and create Kimi's persistent device identity during listing. Clear the callback when withRuntimeHeaders is false. Keep it only on Get and Require results.

Proposed fix

 func cloneDescriptor(descriptor Descriptor, withRuntimeHeaders bool) Descriptor {
 	descriptor.AuthEnvVars = append([]string{}, descriptor.AuthEnvVars...)
 	descriptor.SupportedAPIFormats = append([]APIFormat{}, descriptor.SupportedAPIFormats...)
 	descriptor.Aliases = append([]string{}, descriptor.Aliases...)
+	if !withRuntimeHeaders {
+		descriptor.RuntimeHeaders = nil
+	}
 	if descriptor.CustomHeaders != nil {
 		descriptor.CustomHeaders = copyStringMap(descriptor.CustomHeaders)
 	} else if withRuntimeHeaders && descriptor.RuntimeHeaders != nil {
 		descriptor.CustomHeaders = descriptor.RuntimeHeaders()
 	}
 	return descriptor
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/providercatalog/catalog.go` around lines 416 - 430, Update
cloneDescriptor to clear RuntimeHeaders when withRuntimeHeaders is false, while
preserving it for Get and Require results. Ensure listing copies returned by All
and OAuthProviders cannot invoke the callback or create persistent device
identity.

internal/kimiidentity/kimiidentity_test.go (2)

157-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The adopt-the-winner test depends on a fixed timing margin.

The simulated winner sleeps 30ms before writing. readValidDeviceIDWithRetry gives up after 200ms. That leaves 170ms of slack. Under -race on a loaded CI runner, four goroutines plus scheduler jitter can consume that slack, and a worker then returns its own freshly minted id instead of winner, which fails the test.

Signal the write instead of sleeping: have the workers start first, then write the winner from the main goroutine. Or reduce the sleep to a few milliseconds. Either removes the dependency on absolute wall-clock margins.

As per coding guidelines, "run affected concurrent code under the race detector."

♻️ Proposed fix: shrink the timing dependency

 	done := make(chan struct{})
+	started := make(chan struct{})
 	go func() {
 		defer close(done)
-		time.Sleep(30 * time.Millisecond)
+		<-started
+		time.Sleep(5 * time.Millisecond)
 		_, _ = f.WriteString(winner + "\n")
 		_ = f.Sync()
 		_ = f.Close()
 	}()
 
 	const workers = 4
 	ids := make([]string, workers)
 	var wg sync.WaitGroup
 	wg.Add(workers)
 	for i := range workers {
 		go func(i int) {
 			defer wg.Done()
 			ids[i] = loadOrCreateDeviceIDAt(path)
 		}(i)
 	}
+	close(started)
 	wg.Wait()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/kimiidentity_test.go` around lines 157 - 196, Update
TestLoadOrCreateDeviceIDAdoptsWinnerAfterEmptyCreate to remove the fixed 30ms
timing dependency: start the worker goroutines, then signal or perform the
winner write from the main goroutine so readValidDeviceIDWithRetry observes it
deterministically. Keep all workers required to return winner, and run the
affected concurrent test with the race detector.

Source: Coding guidelines

---

371-383: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a non-ASCII case to the sanitizer test.

The realistic input for asciiHeaderValue is os.Hostname() on a machine with a non-ASCII hostname. The current cases cover printable ASCII, C0 control characters, and the all-control fallback. They do not cover multi-byte runes, which is the main class this function strips.

Add a case with mixed ASCII and non-ASCII, and a case that is entirely non-ASCII so the "unknown" fallback is proven for that input class.

💚 Proposed additional cases

 	if got := asciiHeaderValue("\x01\x02"); got != "unknown" {
 		t.Fatalf("got %q, want unknown", got)
 	}
+	if got := asciiHeaderValue("büro-laptop"); got != "bro-laptop" {
+		t.Fatalf("got %q, want bro-laptop", got)
+	}
+	if got := asciiHeaderValue("主机名"); got != "unknown" {
+		t.Fatalf("got %q, want unknown", got)
+	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/kimiidentity_test.go` around lines 371 - 383, Add
non-ASCII coverage to TestAsciiHeaderValueStripsNonPrintable: verify mixed ASCII
and multi-byte characters are sanitized to the retained ASCII content, and
verify an entirely non-ASCII input returns "unknown". Keep the existing
printable, control-character, and all-control cases unchanged.

internal/tui/provider_wizard.go (2)

1403-1422: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Centralize the x-msh- prefix.

The literal "x-msh-" now appears here and twice in internal/config/resolver.go (Lines 1075 and 1114), across two packages. A future vendor-header change must update all three sites. Export a constant and a predicate from the package that owns the identity headers (internal/kimiidentity or internal/providercatalog) and call it from both.

The clone-before-delete behavior itself is correct and well covered by TestStripRuntimeIdentityHeadersDropsXMshOnly.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/provider_wizard.go` around lines 1403 - 1422, Centralize the
X-Msh header prefix and detection logic in the owning identity-header package by
exporting a prefix constant and predicate. Update stripRuntimeIdentityHeaders
and both resolver locations to use that shared predicate instead of local
"x-msh-" literals, while preserving the existing clone-before-delete behavior
and case-insensitive matching.

---

389-389: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The subtitle lists device code for Kimi Code only.

docs/oauth-subscriptions.md Line 40 lists device code for xAI, Kimi Code, and Hugging Face. Align the wizard subtitle so the two descriptions agree.

✏️ Proposed fix

-			subtitle: "No API key to copy — one-click browser login (OpenRouter, xAI, ChatGPT, Hugging Face) or device code (Kimi Code).",
+			subtitle: "No API key to copy — one-click browser login (OpenRouter, xAI, ChatGPT, Hugging Face) or device code (xAI, Kimi Code, Hugging Face).",

As per coding guidelines: "Ensure PR descriptions, help text, and comments match shipped behavior".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/provider_wizard.go` at line 389, Update the subtitle in the
provider wizard’s no-API-key option to list device-code authentication for xAI,
Kimi Code, and Hugging Face, matching the supported behavior documented in
docs/oauth-subscriptions.md.

Source: Coding guidelines

docs/oauth-subscriptions.md (1)

113-118: 📐 Maintainability & Code Quality | 🔵 Trivial

The documentation admits the X-Msh-* header set is unverified.

Lines 116-118 tell readers to verify the headers against a real login. The PR objectives list an end-to-end Kimi Code login test as an open item. Until that test exists, a header-name or ordering mistake reaches users as a login failure with no local signal.

A hermetic test against a stub authorization server would cover the contract without a real Kimi account: assert the exact X-Msh-* header names and values on the device-authorization, poll, exchange, and refresh requests. Do you want me to draft that test, or open an issue to track it?

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/oauth-subscriptions.md` around lines 113 - 118, The documentation
currently leaves the X-Msh-* header contract unverified; add a hermetic OAuth
test using a stub authorization server that exercises device authorization,
polling, code exchange, and refresh, asserting the exact header names and values
on each request. Remove the unresolved verification warning only once this
coverage is in place.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/cli/auth.go`:
- Around line 606-613: Update the provider help text in internal/cli/auth.go
around the zero auth login documentation to remove provider-specific preset
opt-in claims and describe the global AllowPresets behavior for all OAuth
presets. Also update docs/oauth-subscriptions.md lines 54-63 and the referenced
lines 124 and 137 to use consistent global opt-in wording; do not imply that
presets require selecting particular providers or commands.

In `@internal/kimiidentity/kimiidentity.go`:
- Around line 288-299: Update reclaimDeadRepairLock to keep all lock operations
rooted in the provided *os.Root instead of constructing a path with root.Name().
Replace the lockutil.ReclaimStaleLock usage or add a rooted wrapper so rename,
read, restore, and removal use root.Rename, root.ReadFile, and root.Remove,
while preserving the existing lockHolderAlive and fail-closed behavior.

In `@internal/tui/onboarding_test.go`:
- Around line 2157-2160: Update the type-assertion diagnostics at
internal/tui/onboarding_test.go:2157-2160,
internal/tui/provider_wizard_oauth_test.go:252-255, and
internal/tui/provider_wizard_oauth_test.go:295-298: assign cmd() to an untyped
raw variable, assert raw against the expected setupOAuthMsg or
providerWizardOAuthMsg type, and format raw with %T on failure so diagnostics
report the actual command type.

---

Duplicate comments:
In `@internal/kimiidentity/process_alive_windows_test.go`:
- Around line 16-33: Update TestProcessAliveExitCode259IsDead to open and retain
a Windows SYNCHRONIZE handle for the child process before calling cmd.Wait(),
using the existing process-aliveness implementation’s expected handle type.
Probe processAlive while that handle remains open so the check reaches the
WAIT_OBJECT_0 path, then call cmd.Wait() afterward and close the retained handle
reliably; remove the ineffective _ = cmd.Process statement.

---

Nitpick comments:
In `@docs/oauth-subscriptions.md`:
- Around line 113-118: The documentation currently leaves the X-Msh-* header
contract unverified; add a hermetic OAuth test using a stub authorization server
that exercises device authorization, polling, code exchange, and refresh,
asserting the exact header names and values on each request. Remove the
unresolved verification warning only once this coverage is in place.

In `@internal/kimiidentity/kimiidentity_test.go`:
- Around line 157-196: Update
TestLoadOrCreateDeviceIDAdoptsWinnerAfterEmptyCreate to remove the fixed 30ms
timing dependency: start the worker goroutines, then signal or perform the
winner write from the main goroutine so readValidDeviceIDWithRetry observes it
deterministically. Keep all workers required to return winner, and run the
affected concurrent test with the race detector.
- Around line 371-383: Add non-ASCII coverage to
TestAsciiHeaderValueStripsNonPrintable: verify mixed ASCII and multi-byte
characters are sanitized to the retained ASCII content, and verify an entirely
non-ASCII input returns "unknown". Keep the existing printable,
control-character, and all-control cases unchanged.

In `@internal/providercatalog/catalog.go`:
- Around line 416-430: Update cloneDescriptor to clear RuntimeHeaders when
withRuntimeHeaders is false, while preserving it for Get and Require results.
Ensure listing copies returned by All and OAuthProviders cannot invoke the
callback or create persistent device identity.

In `@internal/tui/provider_wizard.go`:
- Around line 1403-1422: Centralize the X-Msh header prefix and detection logic
in the owning identity-header package by exporting a prefix constant and
predicate. Update stripRuntimeIdentityHeaders and both resolver locations to use
that shared predicate instead of local "x-msh-" literals, while preserving the
existing clone-before-delete behavior and case-insensitive matching.
- Line 389: Update the subtitle in the provider wizard’s no-API-key option to
list device-code authentication for xAI, Kimi Code, and Hugging Face, matching
the supported behavior documented in docs/oauth-subscriptions.md.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f7f6e70d-177e-4d1b-9757-381d2554214c

📥 Commits

Reviewing files that changed from the base of the PR and between 8e26679 and f3abea4.

📒 Files selected for processing (31)

• docs/oauth-subscriptions.md
• internal/cli/auth.go
• internal/config/resolver.go
• internal/config/resolver_test.go
• internal/kimiidentity/export_test.go
• internal/kimiidentity/kimiidentity.go
• internal/kimiidentity/kimiidentity_test.go
• internal/kimiidentity/process_alive_posix.go
• internal/kimiidentity/process_alive_windows.go
• internal/kimiidentity/process_alive_windows_test.go
• internal/oauth/device.go
• internal/oauth/device_test.go
• internal/oauth/flow.go
• internal/oauth/flow_test.go
• internal/oauth/manager.go
• internal/oauth/oauth.go
• internal/oauth/presets.go
• internal/oauth/presets_test.go
• internal/oauth/providers.go
• internal/oauth/providers_test.go
• internal/providercatalog/catalog.go
• internal/providercatalog/catalog_test.go
• internal/providercatalog/export_test.go
• internal/providercatalog/oauth_test.go
• internal/tui/model.go
• internal/tui/oauth_device.go
• internal/tui/onboarding.go
• internal/tui/onboarding_test.go
• internal/tui/provider_wizard.go
• internal/tui/provider_wizard_discovery.go
• internal/tui/provider_wizard_oauth_test.go

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "--- lockutil implementation ---"
fd -t f . internal/lockutil --exec cat -n {} \;

echo "--- other ReclaimStaleLock callers ---"
rg -n -C5 'ReclaimStaleLock' --glob '*.go'

echo "--- os.Root.Name usages ---"
rg -n -C3 '\.Name\(\)' internal/kimiidentity

Repository: Gitlawb/zero

Length of output: 30966

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "--- kimyidentity relevant sections ---"
fd -t f 'kimiidentity.go$' . --exec sh -c 'wc -l "$1"; ast-grep outline "$1" --match "openDeviceIDDir" --view expanded; sed -n "1,360p" "$1"' sh {} \;

echo "--- lockutil package files ---"
fd -t f . internal/lockutil | sed 's#^\./##' | sort

Repository: Gitlawb/zero

Length of output: 14103

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "--- Go os.Root docs if available ---"
go doc os.Root 2>/dev/null || true
echo "--- Go stdlib source doc snippets if available ---"
go env GOROOT
grep -n "func .*Root.*Name\\|type Root struct\\|func \\(\\*Root\\) Rename\\|func \\(\\*Root\\) ReadFile\\|func \\(\\*Root\\) Remove" "$(go env GOROOT)/src/os/root.go" 2>/dev/null || true
sed -n '1,220p' "$(go env GOROOT)/src/os/root.go" 2>/dev/null || true

echo "--- read-only behavioral text analysis of lockutil ---"
python3 - <<'PY'
from pathlib import Path
for p in ["internal/lockutil/reclaim.go", "internal/lockutil/reclaim_other_test.go", "internal/lockutil/reclaim_windows_test.go"]:
    text = Path(p).read_text()
    print(p)
    for needle in ["ReclaimStaleLock", "os.Rename", "RemoveLockFile", "isLive(reclaimed)", "restoreLockFile", "os.ReadFile"]:
        if needle in text:
            print("  contains", needle)
    # simple parser: print ReclaimStaleLock and surrounding implementation
    lines = text.splitlines()
    for i,l in enumerate(lines):
        if "func ReclaimStaleLock" in l:
            print("  ReclaimStaleLock:")
            for j in range(max(0,i-5), min(len(lines), i+35)):
                print(f"    {j+1}: {lines[j]}")
PY

Repository: Gitlawb/zero

Length of output: 12518

---

Keep lock reclamation inside *os.Root.

ReclaimStaleLock receives a plain path, uses os.Rename, then passes the moved name into os.ReadFile before restoring/removing it. root.Name() is the path string from when the root was opened, so rebuilding root.Name() + lockName lets the reclaim operations pass through symlink/reparse-point components again. Perform the reclaim with root.Rename, root.ReadFile, and root.Remove or add a rooted Lockutil wrapper; do not rely on filepath.Join(root.Name(), lockName).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/kimiidentity.go` around lines 288 - 299, Update
reclaimDeadRepairLock to keep all lock operations rooted in the provided
*os.Root instead of constructing a path with root.Name(). Replace the
lockutil.ReclaimStaleLock usage or add a rooted wrapper so rename, read,
restore, and removal use root.Rename, root.ReadFile, and root.Remove, while
preserving the existing lockHolderAlive and fail-closed behavior.

Source: Coding guidelines

[euxaristia]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/kimiidentity/process_alive_windows_test.go`:
- Around line 65-68: Update the test around the process termination assertion to
open a SYNCHRONIZE process handle before cmd.Process.Kill, wait on that handle,
and invoke processAlive(pid) while the handle remains open. Close the handle
only after the liveness assertion, then call cmd.Wait; follow the existing
handle-management pattern used by TestProcessAliveExitCode259IsDead.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: dd40f6ff-11c1-4979-8d67-29550113a8fa

📥 Commits

Reviewing files that changed from the base of the PR and between f3abea4 and 4c5df13.

📒 Files selected for processing (4)

• internal/kimiidentity/kimiidentity.go
• internal/kimiidentity/process_alive_windows_test.go
• internal/tui/onboarding_test.go
• internal/tui/provider_wizard_oauth_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• internal/tui/provider_wizard_oauth_test.go
• internal/tui/onboarding_test.go
• internal/kimiidentity/kimiidentity.go

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the liveness handle open before the post-wait assertion.

Line 66 calls cmd.Wait() before Line 68 calls processAlive(pid). On Windows, cmd.Wait() can close the process handle held by cmd.Process, so processAlive(pid) can receive ERROR_INVALID_PARAMETER and pass through the invalid-PID branch. The test can then pass without exercising WAIT_OBJECT_0 for a terminated process.

Open a SYNCHRONIZE handle before Kill, wait on that handle, call processAlive(pid) while the handle remains open, then close it and call cmd.Wait(). This is the same unresolved handle-lifetime issue previously reported for TestProcessAliveExitCode259IsDead.

#!/bin/bash
set -euo pipefail

sed -n '49,71p' internal/kimiidentity/process_alive_windows_test.go
rg -n -C 3 \
  'OpenProcess|WaitForSingleObject|processAlive\(pid\)|cmd\.Wait\(\)' \
  internal/kimiidentity/process_alive_windows_test.go \
  internal/kimiidentity/process_alive_windows.go

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/process_alive_windows_test.go` around lines 65 - 68,
Update the test around the process termination assertion to open a SYNCHRONIZE
process handle before cmd.Process.Kill, wait on that handle, and invoke
processAlive(pid) while the handle remains open. Close the handle only after the
liveness assertion, then call cmd.Wait; follow the existing handle-management
pattern used by TestProcessAliveExitCode259IsDead.

Source: Coding guidelines

[coderabbitai]

Actionable comments posted: 3

♻️ Duplicate comments (2)

internal/kimiidentity/kimiidentity.go (1)

288-302: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Keep repair-lock reclamation inside the *os.Root handle.

Line 289 rebuilds a path from root.Name(). root.Name() is the path string captured when the root was opened, so it is not traversal-resistant. lockutil.ReclaimStaleLock then performs os.Rename, and the callback at Line 295 performs os.ReadFile on that reconstructed path. A symlink or reparse point on a component can redirect the rename, the read, and the restore outside the configuration root. That defeats the rooted design used by every other operation in this file.

Perform the reclaim with root.Rename, root.ReadFile, and root.Remove, or add a rooted variant of lockutil.ReclaimStaleLock that accepts an *os.Root plus a name.

As per coding guidelines, "Do not rely on pre-open EvalSymlinks followed by open for containment; bind containment at open/use time with rooted or handle-relative traversal-resistant APIs."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/kimiidentity.go` around lines 288 - 302, The
reclaimDeadRepairLock flow must remain rooted in the provided *os.Root instead
of reconstructing paths from root.Name(). Replace or extend
lockutil.ReclaimStaleLock so reclamation uses root.Rename, root.ReadFile, and
root.Remove for every lock operation, including the lockHolderAlive callback,
while preserving the existing stale-lock and fail-closed behavior.

Source: Coding guidelines

internal/kimiidentity/process_alive_windows_test.go (1)

22-32: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Both Windows liveness assertions probe a PID whose handle is already released. cmd.Wait() releases the process handle Go holds, so windows.OpenProcess can return ERROR_INVALID_PARAMETER and processAlive returns false from the invalid-PID branch instead of the WAIT_OBJECT_0 branch. Both tests then pass without exercising the terminated-process path they exist to cover.

• internal/kimiidentity/process_alive_windows_test.go#L22-L32: open a SYNCHRONIZE handle before cmd.Wait(), keep it open across the processAlive(pid) assertion, then close it; remove the no-op _ = cmd.Process statement.
• internal/kimiidentity/process_alive_windows_test.go#L65-L70: open a SYNCHRONIZE handle before cmd.Process.Kill(), wait on that handle, assert processAlive(pid) while it stays open, then close it and call cmd.Wait().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/kimiidentity/process_alive_windows_test.go` around lines 22 - 32,
Update both liveness tests in
internal/kimiidentity/process_alive_windows_test.go: lines 22-32 should open and
retain a SYNCHRONIZE handle before cmd.Wait(), use it during processAlive(pid),
then close it and remove the no-op cmd.Process reference; lines 65-70 should
open the handle before cmd.Process.Kill(), wait on it, assert processAlive(pid)
while retained, then close it before calling cmd.Wait().

Source: Coding guidelines

🧹 Nitpick comments (3)

internal/tui/provider_wizard_oauth_test.go (1)

224-225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Two cancellation tests redirect only XDG_CONFIG_HOME. os.UserConfigDir reads APPDATA on Windows and $HOME/Library/Application Support on macOS, so these tests are hermetic on Linux only. The file already defines isolateKimiDeviceIDStorage, which sets all three variables.

• internal/tui/provider_wizard_oauth_test.go#L224-L225: replace the bare t.Setenv("XDG_CONFIG_HOME", t.TempDir()) in TestProviderWizardEscCancelsDeviceLoginPoll with isolateKimiDeviceIDStorage(t) and keep the explicit ZERO_OAUTH_TOKENS_PATH.
• internal/tui/provider_wizard_oauth_test.go#L270-L271: apply the same replacement in TestModelQuitCancelsProviderWizardDeviceLoginPoll.

As per coding guidelines: "Code and tests must pass on Linux, macOS, and Windows".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/provider_wizard_oauth_test.go` around lines 224 - 225, Update
TestProviderWizardEscCancelsDeviceLoginPoll at
internal/tui/provider_wizard_oauth_test.go:224-225 and
TestModelQuitCancelsProviderWizardDeviceLoginPoll at
internal/tui/provider_wizard_oauth_test.go:270-271 to call
isolateKimiDeviceIDStorage(t) instead of setting only XDG_CONFIG_HOME, while
retaining the explicit ZERO_OAUTH_TOKENS_PATH setup in both tests.

Source: Coding guidelines

internal/providercatalog/catalog.go (1)

416-433: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

RuntimeHeaders is silently ignored when a descriptor also declares CustomHeaders.

The else if makes the two sources mutually exclusive. Today only kimi-code sets RuntimeHeaders and it has no static CustomHeaders, so behavior is correct. A future descriptor that needs both (for example static attribution plus device identity) would silently drop the runtime headers. Either merge the two maps or state the exclusion in the RuntimeHeaders doc comment on the Descriptor field.

♻️ Optional: merge instead of exclude

-	if descriptor.CustomHeaders != nil {
-		descriptor.CustomHeaders = copyStringMap(descriptor.CustomHeaders)
-	} else if withRuntimeHeaders && descriptor.RuntimeHeaders != nil {
-		descriptor.CustomHeaders = descriptor.RuntimeHeaders()
-	}
+	merged := map[string]string{}
+	if withRuntimeHeaders && descriptor.RuntimeHeaders != nil {
+		merged = descriptor.RuntimeHeaders()
+	}
+	for key, value := range descriptor.CustomHeaders {
+		merged[key] = value
+	}
+	if len(merged) == 0 {
+		merged = nil
+	}
+	descriptor.CustomHeaders = merged

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/providercatalog/catalog.go` around lines 416 - 433, Update
cloneDescriptor so withRuntimeHeaders applies descriptor.RuntimeHeaders even
when CustomHeaders is already present, merging both maps while preserving the
descriptor’s static headers and allowing runtime values to be included. Keep the
existing deep-copy behavior and avoid mutating the original descriptor.

internal/tui/provider_wizard.go (1)

143-149: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Cancel any stored poll context before you overwrite deviceLoginCancel.

applyProviderWizardDeviceCode assigns a new cancel func without releasing a previous one. oauthResultMatches stays true for the same attemptID, so a duplicate providerWizardDeviceCodeMsg for one attempt would drop the earlier cancel func and leak that context until the parent m.ctx ends. One defensive call keeps the invariant local.

♻️ Optional guard

+	m.providerWizard.cancelDeviceLogin()
 	ctx, cancel := context.WithCancel(m.ctx)
 	m.providerWizard.deviceLoginCancel = cancel

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/provider_wizard.go` around lines 143 - 149, In
applyProviderWizardDeviceCode, invoke the existing deviceLoginCancel function
before assigning the newly created cancel function, guarding against nil when no
poll is active. Preserve the current context creation and assignment flow so
duplicate providerWizardDeviceCodeMsg events cannot orphan the previous poll
context.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/cli/auth.go`:
- Around line 81-92: Add regression tests for the auth “kimi” case covering
forwarding to runAuthLogin with the kimi-code preset, --help producing parser
help without starting authorization, and unknown flags or extra positional
arguments being rejected consistently with auth login kimi-code. Reuse existing
CLI auth test helpers and assert both outcomes and relevant output or errors.

In `@internal/oauth/presets.go`:
- Around line 162-189: Update isCanonicalKimiHost to require an HTTPS scheme
before accepting auth.kimi.com or api.kimi.com; continue rejecting malformed
URLs and all other hosts, while preserving case-insensitive scheme and hostname
handling.

In `@internal/tui/model.go`:
- Around line 1077-1080: add a regression test for model.quit() with
setup.deviceLoginCancel active, invoke the direct quit path, and assert that the
device-login poll command returns context.Canceled. Keep the existing Ctrl+C
test unchanged and ensure the new test specifically covers direct exit
cancellation.

---

Duplicate comments:
In `@internal/kimiidentity/kimiidentity.go`:
- Around line 288-302: The reclaimDeadRepairLock flow must remain rooted in the
provided *os.Root instead of reconstructing paths from root.Name(). Replace or
extend lockutil.ReclaimStaleLock so reclamation uses root.Rename, root.ReadFile,
and root.Remove for every lock operation, including the lockHolderAlive
callback, while preserving the existing stale-lock and fail-closed behavior.

In `@internal/kimiidentity/process_alive_windows_test.go`:
- Around line 22-32: Update both liveness tests in
internal/kimiidentity/process_alive_windows_test.go: lines 22-32 should open and
retain a SYNCHRONIZE handle before cmd.Wait(), use it during processAlive(pid),
then close it and remove the no-op cmd.Process reference; lines 65-70 should
open the handle before cmd.Process.Kill(), wait on it, assert processAlive(pid)
while retained, then close it before calling cmd.Wait().

---

Nitpick comments:
In `@internal/providercatalog/catalog.go`:
- Around line 416-433: Update cloneDescriptor so withRuntimeHeaders applies
descriptor.RuntimeHeaders even when CustomHeaders is already present, merging
both maps while preserving the descriptor’s static headers and allowing runtime
values to be included. Keep the existing deep-copy behavior and avoid mutating
the original descriptor.

In `@internal/tui/provider_wizard_oauth_test.go`:
- Around line 224-225: Update TestProviderWizardEscCancelsDeviceLoginPoll at
internal/tui/provider_wizard_oauth_test.go:224-225 and
TestModelQuitCancelsProviderWizardDeviceLoginPoll at
internal/tui/provider_wizard_oauth_test.go:270-271 to call
isolateKimiDeviceIDStorage(t) instead of setting only XDG_CONFIG_HOME, while
retaining the explicit ZERO_OAUTH_TOKENS_PATH setup in both tests.

In `@internal/tui/provider_wizard.go`:
- Around line 143-149: In applyProviderWizardDeviceCode, invoke the existing
deviceLoginCancel function before assigning the newly created cancel function,
guarding against nil when no poll is active. Preserve the current context
creation and assignment flow so duplicate providerWizardDeviceCodeMsg events
cannot orphan the previous poll context.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e85910e7-4876-484c-b83e-40b5347d9356

📥 Commits

Reviewing files that changed from the base of the PR and between 8e26679 and 4c5df13.

📒 Files selected for processing (31)

• docs/oauth-subscriptions.md
• internal/cli/auth.go
• internal/config/resolver.go
• internal/config/resolver_test.go
• internal/kimiidentity/export_test.go
• internal/kimiidentity/kimiidentity.go
• internal/kimiidentity/kimiidentity_test.go
• internal/kimiidentity/process_alive_posix.go
• internal/kimiidentity/process_alive_windows.go
• internal/kimiidentity/process_alive_windows_test.go
• internal/oauth/device.go
• internal/oauth/device_test.go
• internal/oauth/flow.go
• internal/oauth/flow_test.go
• internal/oauth/manager.go
• internal/oauth/oauth.go
• internal/oauth/presets.go
• internal/oauth/presets_test.go
• internal/oauth/providers.go
• internal/oauth/providers_test.go
• internal/providercatalog/catalog.go
• internal/providercatalog/catalog_test.go
• internal/providercatalog/export_test.go
• internal/providercatalog/oauth_test.go
• internal/tui/model.go
• internal/tui/oauth_device.go
• internal/tui/onboarding.go
• internal/tui/onboarding_test.go
• internal/tui/provider_wizard.go
• internal/tui/provider_wizard_discovery.go
• internal/tui/provider_wizard_oauth_test.go

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Add regression tests for zero auth kimi.

Cover alias forwarding, zero auth kimi --help, and invalid flags. These cases protect the promised parity with zero auth login kimi-code.

As per coding guidelines: “Every behavior or security-boundary change requires a regression test, including failure paths.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/cli/auth.go` around lines 81 - 92, Add regression tests for the auth
“kimi” case covering forwarding to runAuthLogin with the kimi-code preset,
--help producing parser help without starting authorization, and unknown flags
or extra positional arguments being rejected consistently with auth login
kimi-code. Reuse existing CLI auth test helpers and assert both outcomes and
relevant output or errors.

Source: Coding guidelines

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# List every providerExtraHeaders call site and its arguments.
rg -nP -C 6 '\bproviderExtraHeaders\s*\(' --type=go
# Confirm the allowlist tests cover scheme variants.
rg -nP -C 3 'isCanonicalKimiHost|http://auth\.kimi\.com' --type=go

Repository: Gitlawb/zero

Length of output: 150

---

🏁 Script executed:

#!/bin/bash
set -u

echo "== locate relevant files =="
fd -a 'presets\.go$|kimi|oauth' . | sed 's#^\./##' | head -200

echo
echo "== search providerExtraHeaders usages =="
rg -n -C 7 'providerExtraHeaders\s*\(' . || true

echo
echo "== search isCanonicalKimiHost usages =="
rg -n -C 4 'isCanonicalKimiHost\s*\(' . || true

echo
echo "== direct git ls-files relevant =="
git ls-files | rg '(^|/)internal/oauth/presets\.go$|oauth|kimi|moonshot|presets' || true

Repository: Gitlawb/zero

Length of output: 8700

---

🏁 Script executed:

#!/bin/bash
set -u

echo "== internal/oauth/presets.go relevant section =="
sed -n '18,200p' internal/oauth/presets.go

echo
echo "== internal/oauth/providers.go relevant section =="
sed -n '60,95p' internal/oauth/providers.go

echo
echo "== internal/oauth/providers_test.go relevant tests =="
sed -n '120,175p' internal/oauth/providers_test.go

echo
echo "== read-only verifier: isCanonicalKimiHost behavior for scheme/host variants =="
python3 - <<'PY'
from urllib.parse import urlparse
host_allowlist = {"auth.kimi.com", "api.kimi.com"}
def proposed(urlStr):
    u = urlparse(urlStr)
    if not u.scheme or not u.netloc:
        return False
    if u.scheme.lower() != "https":
        return False
    return u.hostname and u.hostname.lower() in host_allowlist
def current(urlStr):
    u = urlparse(urlStr)
    if not u.netloc:
        return False
    return bool(u.hostname and u.hostname.lower() in host_allowlist)
cases = [
    "https://auth.kimi.com/oauth",
    "http://auth.kimi.com/oauth",
    "FTP://AUTH.KIMI.COM/oauth",
    "https://evil.kimi.com/oauth",
    "auth.kimi.com/oauth",
    "",
]
for case in cases:
    print(f"{case!r}: current={current(case)} proposed={proposed(case)}")
PY

Repository: Gitlawb/zero

Length of output: 13589

---

Reject non-HTTPS Kimi URLs in isCanonicalKimiHost

providerExtraHeaders passes four overridden endpoints, so the call-site issue does not apply. However, isCanonicalKimiHost only checks the hostname, so http://auth.kimi.com/..., FTP://AUTH.KIMI.COM/..., and other non-HTTPS URLs currently receive X-Msh-* device identity headers. Add !strings.EqualFold(u.Scheme, "https") before returning true.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/oauth/presets.go` around lines 162 - 189, Update isCanonicalKimiHost
to require an HTTPS scheme before accepting auth.kimi.com or api.kimi.com;
continue rejecting malformed URLs and all other hosts, while preserving
case-insensitive scheme and hostname handling.

Source: Coding guidelines

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Add a direct quit-cancellation regression test.

Test model.quit() with an active setup.deviceLoginCancel. Assert that the poll command returns context.Canceled. The current Ctrl+C test does not exercise this direct exit path.

As per coding guidelines: “Every behavior or security-boundary change requires a regression test, including failure paths.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/model.go` around lines 1077 - 1080, add a regression test for
model.quit() with setup.deviceLoginCancel active, invoke the direct quit path,
and assert that the device-login poll command returns context.Canceled. Keep the
existing Ctrl+C test unchanged and ensure the new test specifically covers
direct exit cancellation.

Source: Coding guidelines