fix: address usage review findings

This commit is contained in:
justin
2026-05-01 19:33:39 -04:00
parent c433c71928
commit 0b7142fa49
7 changed files with 185 additions and 16 deletions
+4 -2
View File
@@ -4,10 +4,12 @@
"workspaces": { "workspaces": {
"": { "": {
"name": "opencode-kimi-full", "name": "opencode-kimi-full",
"devDependencies": { "dependencies": {
"@opencode-ai/plugin": "^1.4.7",
"@opentui/core": "0.1.99", "@opentui/core": "0.1.99",
"@opentui/solid": "0.1.99", "@opentui/solid": "0.1.99",
},
"devDependencies": {
"@opencode-ai/plugin": "^1.4.7",
"@types/node": "^22.0.0", "@types/node": "^22.0.0",
"typescript": "^5.6.0", "typescript": "^5.6.0",
}, },
+4 -2
View File
@@ -45,10 +45,12 @@
"engines": { "engines": {
"opencode": ">=1.4.6" "opencode": ">=1.4.6"
}, },
"dependencies": {
"@opentui/core": "0.1.99",
"@opentui/solid": "0.1.99"
},
"devDependencies": { "devDependencies": {
"@opencode-ai/plugin": "^1.4.7", "@opencode-ai/plugin": "^1.4.7",
"@opentui/core": "0.1.99",
"@opentui/solid": "0.1.99",
"@types/node": "^22.0.0", "@types/node": "^22.0.0",
"typescript": "^5.6.0" "typescript": "^5.6.0"
} }
+74 -8
View File
@@ -1,3 +1,4 @@
import crypto from "node:crypto"
import fs from "node:fs/promises" import fs from "node:fs/promises"
import path from "node:path" import path from "node:path"
import { PROVIDER_ID, REFRESH_SAFETY_WINDOW_MS } from "./constants.ts" import { PROVIDER_ID, REFRESH_SAFETY_WINDOW_MS } from "./constants.ts"
@@ -13,6 +14,8 @@ import { refreshToken } from "./oauth.ts"
const REFRESH_LOCK_WAIT_MS = 15_000 const REFRESH_LOCK_WAIT_MS = 15_000
const REFRESH_LOCK_POLL_MS = 100 const REFRESH_LOCK_POLL_MS = 100
const REFRESH_LOCK_STALE_MS = 120_000 const REFRESH_LOCK_STALE_MS = 120_000
const REFRESH_LOCK_HEARTBEAT_MS = 30_000
const REFRESH_LOCK_OWNER_FILE = "owner.json"
type RefreshOptions = { type RefreshOptions = {
force?: boolean force?: boolean
@@ -24,6 +27,10 @@ function sleep(ms: number) {
return new Promise((resolve) => setTimeout(resolve, ms)) return new Promise((resolve) => setTimeout(resolve, ms))
} }
function isNodeError(error: unknown, code: string) {
return (error as NodeJS.ErrnoException).code === code
}
function sameAuth(left: OAuthAuth, right: OAuthAuth) { function sameAuth(left: OAuthAuth, right: OAuthAuth) {
return left.access === right.access && left.refresh === right.refresh && left.expires === right.expires return left.access === right.access && left.refresh === right.refresh && left.expires === right.expires
} }
@@ -42,26 +49,77 @@ export function isAuthExpiring(auth: OAuthAuth) {
return auth.expires - Date.now() < REFRESH_SAFETY_WINDOW_MS return auth.expires - Date.now() < REFRESH_SAFETY_WINDOW_MS
} }
function lockOwner(token: string) {
return {
token,
pid: process.pid,
updatedAt: Date.now(),
}
}
async function writeLockOwner(ownerFile: string, token: string) {
const tmpOwnerFile = `${ownerFile}.${process.pid}.${crypto.randomUUID()}.tmp`
try {
await fs.writeFile(tmpOwnerFile, JSON.stringify(lockOwner(token)), "utf8")
await fs.rename(tmpOwnerFile, ownerFile)
} catch (error) {
await fs.rm(tmpOwnerFile, { force: true }).catch(() => undefined)
throw error
}
}
async function ownsLock(ownerFile: string, token: string) {
try {
const data = JSON.parse(await fs.readFile(ownerFile, "utf8")) as { token?: unknown }
return data.token === token
} catch (error) {
if (isNodeError(error, "ENOENT") || error instanceof SyntaxError) return false
throw error
}
}
async function removeStaleLock(lockDir: string, ownerFile: string) {
const stat = await fs.stat(ownerFile).catch(async (error) => {
if (!isNodeError(error, "ENOENT")) throw error
return fs.stat(lockDir).catch((lockError) => {
if (isNodeError(lockError, "ENOENT")) return
throw lockError
})
})
if (!stat) return true
if (Date.now() - stat.mtimeMs <= REFRESH_LOCK_STALE_MS) return false
const staleDir = `${lockDir}.stale.${process.pid}.${Date.now()}.${crypto.randomUUID()}`
try {
await fs.rename(lockDir, staleDir)
} catch (error) {
if (isNodeError(error, "ENOENT")) return true
throw error
}
await fs.rm(staleDir, { recursive: true, force: true })
return true
}
async function withRefreshLock<T>(work: () => Promise<T>) { async function withRefreshLock<T>(work: () => Promise<T>) {
const authFile = await resolveAuthStorePath() const authFile = await resolveAuthStorePath()
const lockDir = `${authFile}.${PROVIDER_ID}.refresh.lock` const lockDir = `${authFile}.${PROVIDER_ID}.refresh.lock`
const ownerFile = path.join(lockDir, REFRESH_LOCK_OWNER_FILE)
const ownerToken = crypto.randomUUID()
await fs.mkdir(path.dirname(lockDir), { recursive: true }) await fs.mkdir(path.dirname(lockDir), { recursive: true })
const deadline = Date.now() + REFRESH_LOCK_WAIT_MS const deadline = Date.now() + REFRESH_LOCK_WAIT_MS
while (true) { while (true) {
try { try {
await fs.mkdir(lockDir) await fs.mkdir(lockDir)
await writeLockOwner(ownerFile, ownerToken).catch(async (error) => {
await fs.rm(lockDir, { recursive: true, force: true }).catch(() => undefined)
throw error
})
break break
} catch (error) { } catch (error) {
const code = (error as NodeJS.ErrnoException).code const code = (error as NodeJS.ErrnoException).code
if (code !== "EEXIST") throw error if (code !== "EEXIST") throw error
try { if (await removeStaleLock(lockDir, ownerFile)) continue
const stat = await fs.stat(lockDir)
if (Date.now() - stat.mtimeMs > REFRESH_LOCK_STALE_MS) {
await fs.rm(lockDir, { recursive: true, force: true })
continue
}
} catch {}
if (Date.now() >= deadline) { if (Date.now() >= deadline) {
throw new Error("kimi oauth: timed out waiting for the auth refresh lock") throw new Error("kimi oauth: timed out waiting for the auth refresh lock")
} }
@@ -69,10 +127,18 @@ async function withRefreshLock<T>(work: () => Promise<T>) {
} }
} }
const heartbeat = setInterval(() => {
writeLockOwner(ownerFile, ownerToken).catch(() => undefined)
}, REFRESH_LOCK_HEARTBEAT_MS)
heartbeat.unref?.()
try { try {
return await work() return await work()
} finally { } finally {
await fs.rm(lockDir, { recursive: true, force: true }).catch(() => undefined) clearInterval(heartbeat)
await ownsLock(ownerFile, ownerToken)
.then((owned) => (owned ? fs.rm(lockDir, { recursive: true, force: true }) : undefined))
.catch(() => undefined)
} }
} }
+19 -2
View File
@@ -111,6 +111,8 @@ function UsageDialog(props: { api: TuiPluginApi; rows?: UsageRow[]; loading?: bo
} }
const tui: TuiPlugin = async (api) => { const tui: TuiPlugin = async (api) => {
let usageRequestId = 0
api.command.register(() => [ api.command.register(() => [
{ {
title: "Kimi usage", title: "Kimi usage",
@@ -121,13 +123,28 @@ const tui: TuiPlugin = async (api) => {
name: "kimi:usage", name: "kimi:usage",
}, },
onSelect: async () => { onSelect: async () => {
api.ui.dialog.replace(() => <UsageDialog api={api} loading />) const requestId = ++usageRequestId
let dismissed = false
let replacing = false
const isCurrent = () => usageRequestId === requestId && !dismissed
const markDismissed = () => {
if (!replacing) dismissed = true
}
api.ui.dialog.replace(() => <UsageDialog api={api} loading />, markDismissed)
try { try {
const auth = await ensureFreshStoredAuth() const auth = await ensureFreshStoredAuth()
const payload = await fetchUsage(auth.access) const payload = await fetchUsage(auth.access)
const rows = parseUsagePayload(payload) const rows = parseUsagePayload(payload)
api.ui.dialog.replace(() => <UsageDialog api={api} rows={rows} />) if (!isCurrent()) return
replacing = true
try {
api.ui.dialog.replace(() => <UsageDialog api={api} rows={rows} />, markDismissed)
} finally {
replacing = false
}
} catch (error) { } catch (error) {
if (!isCurrent()) return
api.ui.dialog.clear() api.ui.dialog.clear()
api.ui.toast({ api.ui.toast({
message: error instanceof Error ? error.message : "Failed to fetch Kimi usage.", message: error instanceof Error ? error.message : "Failed to fetch Kimi usage.",
+2 -2
View File
@@ -78,8 +78,8 @@ function resetHint(data: Record<string, unknown>) {
if (resetAt) return formatResetTime(String(resetAt)) if (resetAt) return formatResetTime(String(resetAt))
const seconds = toInt(data.reset_in ?? data.resetIn ?? data.ttl ?? data.window) const seconds = toInt(data.reset_in ?? data.resetIn ?? data.ttl ?? data.window)
if (!seconds) return if (seconds === undefined) return
return `resets in ${formatDuration(seconds)}` return seconds === 0 ? "reset now" : `resets in ${formatDuration(seconds)}`
} }
function toUsageRow(data: Record<string, unknown>, defaultLabel: string): UsageRow | undefined { function toUsageRow(data: Record<string, unknown>, defaultLabel: string): UsageRow | undefined {
+70
View File
@@ -27,6 +27,10 @@ function authStorePath(base: string) {
return path.join(base, "opencode", "auth.json") return path.join(base, "opencode", "auth.json")
} }
function refreshLockPath(base: string) {
return `${authStorePath(base)}.${PROVIDER_ID}.refresh.lock`
}
async function writeAuthStore(base: string, entry: unknown) { async function writeAuthStore(base: string, entry: unknown) {
await fs.mkdir(path.dirname(authStorePath(base)), { recursive: true }) await fs.mkdir(path.dirname(authStorePath(base)), { recursive: true })
await fs.writeFile(authStorePath(base), JSON.stringify({ [PROVIDER_ID]: entry }), "utf8") await fs.writeFile(authStorePath(base), JSON.stringify({ [PROVIDER_ID]: entry }), "utf8")
@@ -61,3 +65,69 @@ test("ensureFreshStoredAuth refreshes expiring auth and persists it", async () =
expect(stored[PROVIDER_ID].refresh).toBe("refresh-2") expect(stored[PROVIDER_ID].refresh).toBe("refresh-2")
expect(mock.calls).toHaveLength(1) expect(mock.calls).toHaveLength(1)
}) })
test("ensureFreshStoredAuth removes stale refresh locks", async () => {
root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-kimi-auth-refresh-"))
previousXdgDataHome = process.env.XDG_DATA_HOME
process.env.XDG_DATA_HOME = root
await writeAuthStore(root, {
type: "oauth",
access: "stale",
refresh: "refresh-1",
expires: Date.now() - 1_000,
})
const lockDir = refreshLockPath(root)
await fs.mkdir(lockDir)
await fs.writeFile(path.join(lockDir, "owner.json"), JSON.stringify({ token: "dead" }), "utf8")
const stale = new Date(Date.now() - 180_000)
await fs.utimes(path.join(lockDir, "owner.json"), stale, stale)
await fs.utimes(lockDir, stale, stale)
mock = installFetchMock(() => ({
body: {
access_token: "fresh",
refresh_token: "refresh-2",
token_type: "Bearer",
expires_in: 900,
},
}))
const auth = await ensureFreshStoredAuth()
expect(auth.access).toBe("fresh")
expect(
await fs
.access(lockDir)
.then(() => true)
.catch(() => false),
).toBe(false)
expect(mock.calls).toHaveLength(1)
})
test("ensureFreshStoredAuth does not fail when cleanup cannot verify lock ownership", async () => {
root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-kimi-auth-refresh-"))
previousXdgDataHome = process.env.XDG_DATA_HOME
process.env.XDG_DATA_HOME = root
await writeAuthStore(root, {
type: "oauth",
access: "stale",
refresh: "refresh-1",
expires: Date.now() - 1_000,
})
mock = installFetchMock(async () => {
await fs.writeFile(path.join(refreshLockPath(root!), "owner.json"), "{", "utf8")
return {
body: {
access_token: "fresh",
refresh_token: "refresh-2",
token_type: "Bearer",
expires_in: 900,
},
}
})
const auth = await ensureFreshStoredAuth()
expect(auth.access).toBe("fresh")
expect(mock.calls).toHaveLength(1)
})
+12
View File
@@ -39,3 +39,15 @@ test("parseUsagePayload maps summary and rolling limits", () => {
}, },
]) ])
}) })
test("parseUsagePayload preserves immediate reset hints", () => {
const rows = parseUsagePayload({
usage: {
limit: 100,
used: 100,
reset_in: 0,
},
})
expect(rows[0]?.resetHint).toBe("reset now")
})