Skip to content

Commit 21fcc55

Browse files
committed
run cmmand supports to exit under all error scenarios & quota detection
Signed-off-by: weizhoublue <weizhou.lan@daocloud.io>
1 parent e2c0797 commit 21fcc55

14 files changed

Lines changed: 738 additions & 60 deletions
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
# Run Retry Bounds Implementation Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+
**Goal:** Bound non-quota retries for non-interactive `opencode run` to five retries and 120 seconds without changing global retry behavior.
6+
7+
**Architecture:** Keep enforcement in `src/cli/cmd/run.ts`, where the CLI consumes `session.status.retry` and can abort a Session. Quota errors retain immediate abort and key rotation; other retry statuses use `attempt` and `next` for CLI-only bounds.
8+
9+
**Tech Stack:** TypeScript, Bun test, Effect, OpenCode SDK v2.
10+
11+
## Global Constraints
12+
13+
- Apply retry bounds only to non-interactive `opencode run`; do not modify `SessionRetry.policy`.
14+
- Known quota errors abort immediately and remain eligible for key rotation.
15+
- Non-quota errors never rotate keys or write `throttle.json`.
16+
- Allow attempts 1 through 5; reject attempt 6 before it waits.
17+
- Abort a non-quota retry when `status.next` would exceed 120 seconds from retry attempt 1.
18+
- Text and JSON retry-bound errors use `OPENCODE_RETRY_LIMIT:`.
19+
- Run tests and `bun typecheck` from `packages/opencode`.
20+
21+
---
22+
23+
### Task 1: Add retry-limit process regressions
24+
25+
**Files:**
26+
- Modify: `packages/opencode/test/cli/run/run-process-limit.test.ts`
27+
28+
**Interfaces:**
29+
- Consumes: `opencode.run(message, { format?, timeoutMs? })` and TestLLMServer response helpers.
30+
- Produces: process assertions for retry count, retry deadline, text output, and JSON output.
31+
32+
- [ ] **Step 1: Write failing retry-count tests**
33+
34+
Queue five retryable 500 responses followed by text and assert exit code `0`. Queue six retryable 500 responses and assert nonzero exit, duration below 30 seconds, and `OPENCODE_RETRY_LIMIT:` in stderr. Set `retry-after-ms: 0` so the suite does not sleep.
35+
36+
```ts
37+
const result = yield* opencode.run("say hi", { timeoutMs: 30_000 })
38+
expect(result.exitCode).not.toBe(0)
39+
expect(result.stderr).toContain("OPENCODE_RETRY_LIMIT:")
40+
expect(result.durationMs).toBeLessThan(30_000)
41+
```
42+
43+
- [ ] **Step 2: Write failing deadline and JSON tests**
44+
45+
Return a retryable response with `retry-after: 121` and assert immediate nonzero exit with `OPENCODE_RETRY_LIMIT:`. Add a JSON-format sixth-failure test that requires a prefixed `error.message` while retaining retry fields.
46+
47+
```ts
48+
const events = opencode.parseJsonEvents(result.stdout)
49+
expect(events.find((event) => event.type === "error")?.error).toMatchObject({
50+
type: "retry",
51+
message: expect.stringMatching(/^OPENCODE_RETRY_LIMIT:/),
52+
attempt: expect.any(Number),
53+
next: expect.any(Number),
54+
})
55+
```
56+
57+
- [ ] **Step 3: Run tests and confirm red**
58+
59+
Run:
60+
61+
```sh
62+
bun test --test-name-pattern 'retry limit|Retry-After|format json' test/cli/run/run-process-limit.test.ts --timeout 60000
63+
```
64+
65+
Expected: new marker and bounded-retry assertions fail because no retry-limit path exists.
66+
67+
### Task 2: Bound CLI retry status handling
68+
69+
**Files:**
70+
- Modify: `packages/opencode/src/cli/cmd/run.ts:35-55`
71+
- Modify: `packages/opencode/src/cli/cmd/run.ts:827-850`
72+
73+
**Interfaces:**
74+
- Consumes: retry status `{ attempt, message, next }`, `SessionRetry.isQuotaOrRateLimitRetryStatus`, and `client.session.abort({ sessionID })`.
75+
- Produces: `OPENCODE_RETRY_LIMIT` text/JSON errors and bounded non-quota retries.
76+
77+
- [ ] **Step 1: Add CLI-only constants and message helper**
78+
79+
Add constants beside `quotaError` and use the existing error-payload pattern for both markers.
80+
81+
```ts
82+
const RUN_RETRY_MAX_ATTEMPTS = 5
83+
const RUN_RETRY_MAX_WAIT = 120_000
84+
85+
function retryLimitError(message: string) {
86+
return `OPENCODE_RETRY_LIMIT: ${message}`
87+
}
88+
```
89+
90+
- [ ] **Step 2: Track a non-quota retry deadline**
91+
92+
Before `loop`, declare `let retryDeadline: number | undefined`. In a non-quota retry branch, reset it when `status.attempt === 1` to `Date.now() + RUN_RETRY_MAX_WAIT`. Abort before waiting when either condition is true:
93+
94+
```ts
95+
status.attempt > RUN_RETRY_MAX_ATTEMPTS || status.next > retryDeadline
96+
```
97+
98+
On the limit, abort the session, produce `retryLimitError(status.message)`, emit the prefixed JSON error in JSON mode, print the same text in default mode, and return from the event loop.
99+
100+
- [ ] **Step 3: Preserve quota ordering**
101+
102+
Keep recognized quota handling before generic retry limiting. Do not set `pendingRotation` for generic retry-limit errors.
103+
104+
```ts
105+
if (status.type === "retry" && SessionRetry.isQuotaOrRateLimitRetryStatus(status)) {
106+
// Existing abort, immediate exit, and key rotation path.
107+
}
108+
```
109+
110+
- [ ] **Step 4: Run focused tests and confirm green**
111+
112+
Run:
113+
114+
```sh
115+
bun test --test-name-pattern 'retry limit|Retry-After|format json' test/cli/run/run-process-limit.test.ts --timeout 60000
116+
```
117+
118+
Expected: all new retry-limit tests pass.
119+
120+
### Task 3: Verify quota and rotation contracts
121+
122+
**Files:**
123+
- Verify: `packages/opencode/test/session/retry.test.ts`
124+
- Verify: `packages/opencode/test/cli/run/run-process-limit.test.ts`
125+
- Verify: `packages/opencode/test/cli/run/run-key-rotation.test.ts`
126+
127+
**Interfaces:**
128+
- Consumes: existing quota classification, process, and key-rotation fixtures.
129+
- Produces: evidence that retry limits do not change quota exits or API-key rotation.
130+
131+
- [ ] **Step 1: Run regression suites**
132+
133+
Run:
134+
135+
```sh
136+
bun test test/session/retry.test.ts --timeout 30000
137+
bun test test/cli/run/run-process-limit.test.ts --timeout 60000
138+
bun test test/cli/run/run-key-rotation.test.ts --timeout 60000
139+
```
140+
141+
Expected: all tests pass, including quota exits, JSON quota markers, rotation, all-key exhaustion, and throttle-disabled rotation.
142+
143+
- [ ] **Step 2: Format and typecheck**
144+
145+
Run:
146+
147+
```sh
148+
bunx prettier --write src/cli/cmd/run.ts test/cli/run/run-process-limit.test.ts
149+
bun typecheck
150+
git diff --check
151+
```
152+
153+
Expected: typecheck succeeds and whitespace check is clean.
154+
155+
- [ ] **Step 3: Build and release verification**
156+
157+
Run:
158+
159+
```sh
160+
bun run script/build.ts --single --skip-install --skip-embed-web-ui
161+
PATH=/tmp/opencode-verify:$PATH timeout --signal=TERM 25s /usr/sbin/opencode-cheap run 'hi'
162+
PATH=/tmp/opencode-verify:$PATH timeout --signal=TERM 25s /usr/sbin/opencode-cheap run --format json 'hi'
163+
```
164+
165+
Expected: both commands return nonzero before timeout; text and JSON expose `OPENCODE_QUOTA_LIMIT:`.
166+
167+
- [ ] **Step 4: Commit implementation**
168+
169+
Run:
170+
171+
```sh
172+
git add packages/opencode/src/cli/cmd/run.ts packages/opencode/test/cli/run/run-process-limit.test.ts
173+
git commit -s -S -m 'fix(cli): bound run retries'
174+
```
175+
176+
Expected: one signed commit containing only retry-limit implementation and tests.
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
# Single-Key Throttle Implementation Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+
**Goal:** Persist and honor quota throttle records when `OPENCODE_API_KEY` contains one key.
6+
7+
**Architecture:** Route every non-empty `OPENCODE_API_KEY` list through `runWithKeyRotation`, including one key. `KeyRotator` remains the shared throttle reader and writer. A quota failure records the key before the controller reports exhaustion; a later CLI invocation sees the same record and exits before contacting the provider.
8+
9+
**Tech Stack:** TypeScript, Bun test, Effect subprocess CLI tests.
10+
11+
## Global Constraints
12+
13+
- Keep `OPENCODE_THROTTLE_ENABLE=false` as a complete read/write opt-out.
14+
- Do not change quota detection, API-key parsing, or retry policy.
15+
- Preserve final quota error and nonzero exit for a throttled single key.
16+
17+
---
18+
19+
### Task 1: Cover single-key shared throttle behavior
20+
21+
**Files:**
22+
23+
- Modify: `packages/opencode/test/cli/run/run-key-rotation.test.ts:194-215`
24+
25+
**Interfaces:**
26+
27+
- Consumes: `opencode.run(prompt, { env, timeoutMs })` and per-test `home` directory.
28+
- Produces: subprocess regression proving persisted record suppresses later invocation.
29+
30+
- [ ] **Step 1: Replace obsolete assertion with failing shared-state test**
31+
32+
Replace existing single-key test with test named `single key writes throttle.json and later invocation skips it after quota limit`. It must run key1 against 429 `RateLimitError`, assert nonzero exit, read `throttle.json`, assert an `OPENCODE_API_KEY` record matching `hashKey("key1")`, then run same command again with same `home` and assert nonzero exit plus `OPENCODE_QUOTA_LIMIT: all configured API keys are exhausted or throttled`.
33+
34+
- [ ] **Step 2: Run test and confirm RED state**
35+
36+
Run: `bun test test/cli/run/run-key-rotation.test.ts --timeout 60000`
37+
38+
Expected: new test fails at reading `throttle.json` with `ENOENT`.
39+
40+
### Task 2: Route one key through shared throttle control
41+
42+
**Files:**
43+
44+
- Modify: `packages/opencode/src/cli/cmd/run/key-rotation.ts:12-17`
45+
- Test: `packages/opencode/test/cli/run/run-key-rotation.test.ts`
46+
47+
**Interfaces:**
48+
49+
- Consumes: `parseKeys(): string[]`, `KeyRotator.selectKey()`, and `KeyRotator.recordThrottle(key)`.
50+
- Produces: quota persistence for any non-empty explicit key list.
51+
52+
- [ ] **Step 1: Change only no-explicit-key fast path**
53+
54+
Replace `if (keys.length <= 1)` with:
55+
56+
```ts
57+
if (keys.length === 0) {
58+
await options.execute(options.createSdk())
59+
return
60+
}
61+
```
62+
63+
Existing quota handling already calls `await rotator.recordThrottle(key)` before final exhaustion, so a one-key quota then writes the shared record.
64+
65+
- [ ] **Step 2: Run regression and confirm GREEN state**
66+
67+
Run: `bun test test/cli/run/run-key-rotation.test.ts --timeout 60000`
68+
69+
Expected: all key-rotation subprocess tests pass.
70+
71+
- [ ] **Step 3: Run focused verification**
72+
73+
Run: `bun test test/provider/key-rotator.test.ts --timeout 30000 && bun test test/provider/throttle-store.test.ts --timeout 30000 && bun typecheck && git diff --check`
74+
75+
Expected: all commands exit 0.
76+
77+
- [ ] **Step 4: Commit implementation**
78+
79+
Run: `git add packages/opencode/src/cli/cmd/run/key-rotation.ts packages/opencode/test/cli/run/run-key-rotation.test.ts && git commit -s -S -m "fix(opencode): persist single key throttle"`
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
# Throttle Local Time Implementation Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+
**Goal:** Store `throttle.json` timestamps as readable local ISO 8601 strings while preserving throttle and expiry behavior.
6+
7+
**Architecture:** `ThrottleStore` formats the two calculated timestamps into local ISO 8601 strings with milliseconds and a numeric UTC offset before persisting. It parses `endTime` when deciding whether a record is active or removable. Test fixtures use the same string representation because numeric timestamp support is deliberately removed.
8+
9+
**Tech Stack:** TypeScript, Bun test runner, Node `fs/promises`, existing `Flock` utility.
10+
11+
## Global Constraints
12+
13+
- Scope is `packages/opencode` throttle persistence and its tests only.
14+
- `startTime` and `endTime` are local ISO 8601 strings with milliseconds and an explicit UTC offset.
15+
- Numeric timestamp compatibility and migration are out of scope.
16+
- Run tests and `bun typecheck` from `packages/opencode`, never from repo root.
17+
- Commit with `git commit -s -S` using a concise conventional one-line message.
18+
19+
---
20+
21+
## File Structure
22+
23+
- Modify `packages/opencode/src/provider/throttle-store.ts`: change the persisted timestamp type, format local values, and parse expiry values.
24+
- Modify `packages/opencode/test/provider/throttle-store.test.ts`: make unit fixtures string-based and verify write format, duration, active state, and expiry cleanup.
25+
- Modify `packages/opencode/test/cli/run/run-key-rotation.test.ts`: replace pre-seeded numeric timestamp fixtures with local ISO 8601 strings.
26+
27+
### Task 1: Persist and consume local ISO timestamps
28+
29+
**Files:**
30+
- Modify: `packages/opencode/test/provider/throttle-store.test.ts`
31+
- Modify: `packages/opencode/test/cli/run/run-key-rotation.test.ts`
32+
- Modify: `packages/opencode/src/provider/throttle-store.ts`
33+
34+
**Interfaces:**
35+
- Consumes: `createThrottleStore({ configDir })` with `isThrottled`, `addThrottle`, and `cleanExpired`.
36+
- Produces: `throttle.json` records whose `startTime` and `endTime` are local ISO 8601 strings, such as `2026-07-11T14:21:02.689+08:00`.
37+
38+
- [x] **Step 1: Write the failing unit and CLI fixture changes**
39+
40+
Add this test helper near `hashKey` in both affected test files, using local date parts and `getTimezoneOffset()` to generate a portable local ISO string:
41+
42+
```typescript
43+
function localTime(time: number) {
44+
const date = new Date(time)
45+
const offset = -date.getTimezoneOffset()
46+
const sign = offset >= 0 ? "+" : "-"
47+
const pad = (value: number) => String(value).padStart(2, "0")
48+
return `${date.getFullYear()}-${pad(date.getMonth() + 1)}-${pad(date.getDate())}T${pad(date.getHours())}:${pad(date.getMinutes())}:${pad(date.getSeconds())}.${String(date.getMilliseconds()).padStart(3, "0")}${sign}${pad(Math.floor(Math.abs(offset) / 60))}:${pad(Math.abs(offset) % 60)}`
49+
}
50+
```
51+
52+
Replace every pre-seeded unit and CLI record timestamp with `localTime(now - 1_000)` or `localTime(now + 60_000)` as appropriate. In the `addThrottle` creation test, replace numeric subtraction with these assertions:
53+
54+
```typescript
55+
expect(data[0].startTime).toMatch(/\.\d{3}[+-]\d{2}:\d{2}$/)
56+
expect(data[0].endTime).toMatch(/\.\d{3}[+-]\d{2}:\d{2}$/)
57+
expect(Date.parse(data[0].endTime) - Date.parse(data[0].startTime)).toBe(120 * 60 * 1000)
58+
```
59+
60+
Apply the same parsed-duration assertion to the upsert test. Add a test that calls `addThrottle(..., -1)` then `isThrottled(...)`, expects `false`, and verifies that the expired record was removed.
61+
62+
- [x] **Step 2: Run the targeted unit test and verify it fails**
63+
64+
Run:
65+
66+
```bash
67+
cd packages/opencode && bun test test/provider/throttle-store.test.ts
68+
```
69+
70+
Expected: FAIL because the production store still writes numbers and compares `now` directly to `record.endTime`.
71+
72+
- [x] **Step 3: Implement the smallest timestamp conversion and parsing change**
73+
74+
Change the record type and add these helpers below it:
75+
76+
```typescript
77+
type ThrottleRecord = {
78+
source: string
79+
key_hint: string
80+
key_hash: string
81+
startTime: string
82+
endTime: string
83+
}
84+
85+
function formatLocalTime(time: number) {
86+
const date = new Date(time)
87+
const offset = -date.getTimezoneOffset()
88+
const sign = offset >= 0 ? "+" : "-"
89+
const pad = (value: number) => String(value).padStart(2, "0")
90+
return `${date.getFullYear()}-${pad(date.getMonth() + 1)}-${pad(date.getDate())}T${pad(date.getHours())}:${pad(date.getMinutes())}:${pad(date.getSeconds())}.${String(date.getMilliseconds()).padStart(3, "0")}${sign}${pad(Math.floor(Math.abs(offset) / 60))}:${pad(Math.abs(offset) % 60)}`
91+
}
92+
93+
function isExpired(record: ThrottleRecord, now: number) {
94+
const endTime = Date.parse(record.endTime)
95+
return Number.isNaN(endTime) || now >= endTime
96+
}
97+
```
98+
99+
In `isThrottled`, replace the numeric comparison with `if (!isExpired(record, now)) return true`. In `addThrottle`, write `startTime: formatLocalTime(now)` and `endTime: formatLocalTime(endTime)`. In `cleanExpired`, replace `now >= r.endTime` with `isExpired(r, now)` in the matching-record predicate so expired and invalid old numeric entries are removed.
100+
101+
- [x] **Step 4: Run focused verification**
102+
103+
Run:
104+
105+
```bash
106+
cd packages/opencode && bun test test/provider/throttle-store.test.ts && bun test test/cli/run/run-key-rotation.test.ts && bun typecheck
107+
```
108+
109+
Expected: all targeted tests pass and `bun typecheck` exits 0.
110+
111+
- [x] **Step 5: Review changed files and commit**
112+
113+
Run:
114+
115+
```bash
116+
git diff --check && git diff -- packages/opencode/src/provider/throttle-store.ts packages/opencode/test/provider/throttle-store.test.ts packages/opencode/test/cli/run/run-key-rotation.test.ts
117+
git add packages/opencode/src/provider/throttle-store.ts packages/opencode/test/provider/throttle-store.test.ts packages/opencode/test/cli/run/run-key-rotation.test.ts
118+
git commit -s -S -m "fix(opencode): store throttle times locally"
119+
```
120+
121+
Expected: no whitespace errors; one signed and signed-off commit containing only the three implementation files.

0 commit comments

Comments
 (0)