Skip to content

Commit bdb6265

Browse files
committed
code review, lockfiles, and other fixes
1 parent 8536d99 commit bdb6265

File tree

19 files changed

+701
-128
lines changed

19 files changed

+701
-128
lines changed

.github/workflows/config.yml

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
name: config-changes
2+
3+
on:
4+
push:
5+
paths:
6+
- "packages/opencode/src/config/**"
7+
- "packages/opencode/test/config/**"
8+
- "docs/config-hot-reload.md"
9+
pull_request:
10+
paths:
11+
- "packages/opencode/src/config/**"
12+
- "packages/opencode/test/config/**"
13+
- "docs/config-hot-reload.md"
14+
workflow_dispatch:
15+
16+
jobs:
17+
config:
18+
runs-on: ubuntu-latest
19+
steps:
20+
- name: Checkout repository
21+
uses: actions/checkout@v4
22+
23+
- name: Setup Bun
24+
uses: ./.github/actions/setup-bun
25+
26+
- name: Run config validations
27+
env:
28+
CI: true
29+
run: |
30+
bun test packages/opencode/test/config/*
31+
bun run --cwd packages/opencode typecheck

IMPLEMENTATION_SUMMARY.md

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ INFO service=config.invalidation scope=global directory=/tmp/theme-only-oMAT6r
3131

3232
- That reduces invalidation fan-out from 4+ subsystems down to one, eliminating roughly 75% of the work for theme edits. The improvement is enforced by the updated test suite (`bun test packages/opencode/test/config/write.test.ts packages/opencode/test/config/hot-reload.test.ts`).
3333

34+
### Backup Retention Cleanup
35+
36+
- Added `cleanupOldBackups` in `packages/opencode/src/config/backup.ts`, which deletes `.bak-*` files older than a configurable TTL (default 7 days via `OPENCODE_CONFIG_BACKUP_TTL_DAYS`).
37+
- The cleanup routine runs during instance bootstrap and after every successful `Config.update`, ensuring disk usage stays bounded.
38+
- Logged deletion counts make it easy to monitor retention behavior; see `packages/opencode/test/config/backup.test.ts` for TTL enforcement coverage.
39+
3440
## What Was Implemented
3541

3642
### Phase 0: State System Enhancements
@@ -133,10 +139,15 @@ INFO service=config.invalidation scope=global directory=/tmp/theme-only-oMAT6r
133139

134140
- **Type**: Boolean (`"true"` | `"false"`)
135141
- **Default**: `"false"`
136-
- **Purpose**: Debug flag (not yet fully implemented)
142+
- **Purpose**: Debug telemetry for targeted invalidation
137143
- **Behavior**:
138-
- `"true"`: Log complete diff objects for troubleshooting
139-
- `"false"`: Log only diff section names
144+
- `"true"`: Emit `config.invalidate.diff` debug records with the full diff payload plus scope/directory so every config write can be correlated with its invalidation targets.
145+
- `"false"`: Continue logging only section and target names through the existing `config.invalidate.start/complete` info lines.
146+
147+
## Documentation
148+
149+
- Authored `docs/config-hot-reload.md` to describe each feature flag, backup TTL, distributed locking strategy, and the `config.invalidate.diff` toggle so operators know how to interpret logs and adjust retention.
150+
- Updated `README.md` configuration guidance to cover automatic backup cleanup decisions, highlight the hot-reload feature, and link to the new document for deeper guidance.
140151

141152
## Usage
142153

@@ -218,6 +229,11 @@ bun test test/config/hot-reload.test.ts
218229
bun run --cwd packages/opencode typecheck
219230
```
220231

232+
## CI
233+
234+
- Added `.github/workflows/config.yml`, which runs on pushes or pull requests that touch `packages/opencode/src/config/**`, `packages/opencode/test/config/**`, or `docs/config-hot-reload.md`.
235+
- The workflow executes `bun test packages/opencode/test/config/*` and `bun run --cwd packages/opencode typecheck` so config changes receive the extra validation steps required before merging.
236+
221237
## What's Not Yet Implemented (Future Work)
222238

223239
According to the original plan, the following are not yet complete:

bun.lock

Lines changed: 14 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/config-hot-reload.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# Config Hot Reload
2+
3+
This document summarizes the configuration, locking, backup, and observability story around `packages/opencode/src/config`, so you can safely exercise hot reload in development and production.
4+
5+
## Overview
6+
7+
With `OPENCODE_CONFIG_HOT_RELOAD=true` OpenCode switches from a full `Instance.dispose()` restart to a targeted invalidation sweep that touches only the subsystems named in `ConfigInvalidation.apply`. That lets editors, providers, the MCP bus, and language services keep running while only `config`, `provider`, `lsp`, `plugin`, `tool-registry`, and other affected states refresh.
8+
9+
Hot reload is gated by feature flags so you can stage it gradually.
10+
11+
## Feature Flags
12+
13+
- `OPENCODE_CONFIG_HOT_RELOAD` (default `"false"`): Enable targeted invalidation across project/global updates. Set this before launching the server or CLI.
14+
- `OPENCODE_FULL_DISPOSE_ON_CONFIG_UPDATE` (default `"false"`): Force the legacy `Instance.dispose()` behaviour even when hot reload is enabled, useful for troubleshooting.
15+
- `OPENCODE_CONFIG_INVALIDATION_LOG_DIFF` (default `"false"`): Emit `config.invalidate.diff` debug logs that include `scope`, `directory`, and the raw `ConfigDiff` so you can inspect which sections changed before the invalidation tasks run.
16+
- `OPENCODE_CONFIG_BACKUP_TTL_DAYS` (default `7`): How long `.bak-<timestamp>` files should be retained. Shorten if disk space is tight, or extend for longer history.
17+
18+
## Backup Cleanup
19+
20+
Every config update writes a timestamped `.bak-<iso-timestamp>` backup next to the target config file. The cleanup routine in `packages/opencode/src/config/backup.ts` runs during bootstrap and immediately after each successful `Config.update`, deleting backups older than `OPENCODE_CONFIG_BACKUP_TTL_DAYS`. This keeps disk growth bounded without manual maintenance.
21+
22+
## Locking & Concurrency
23+
24+
Writes are protected by `proper-lockfile` in `packages/opencode/src/config/lock.ts`. Before acquiring a lock OpenCode ensures the lock file exists and configures retries (`retries`, `minTimeout`, `maxTimeout`) with stale detection (`stale = 5s`, `update = stale / 2`). On startup `cleanupStaleLocks` scans for residual `.lock` directories, verifies `lockfile.check`, and removes orphaned directories while logging the cleanup. The same lock is reused for project/global config writes, so simultaneous `Config.update` calls serialize safely.
25+
26+
## Updating Config
27+
28+
Apply config changes via `PATCH /config` (defaults to project scope) or `PATCH /config?scope=global` (for global overrides). Example:
29+
30+
```bash
31+
export OPENCODE_CONFIG_HOT_RELOAD=true
32+
33+
curl -X PATCH http://localhost:4096/config \
34+
-H "Content-Type: application/json" \
35+
-d '{"model": "anthropic/claude-3-5-sonnet"}'
36+
```
37+
38+
The API merges the payload with the existing `Info` schema, emits `config.updated` via `Bus`, and publishes a `ConfigDiff` that `ConfigInvalidation.apply` uses to derive invalidation targets.
39+
40+
## Logs & Diagnostics
41+
42+
- `config.invalidate.stateRefreshed` fires whenever a new `Instance` context loads the latest config.
43+
- `config.invalidate.start`/`config.invalidate.complete` include `scope`, `directory`, `sections`, and `targets`.
44+
- Enable `OPENCODE_CONFIG_INVALIDATION_LOG_DIFF=true` to capture `config.invalidate.diff`, which adds the entire diff object to the log entry so you can see every changed section before invalidation begins.
45+
- Any fallback to the JSONC writer still logs warnings via `config.write.fallback`, and backup cleanup emits `config.backup.cleanup` with deletion counts.
46+
47+
## Testing & CI
48+
49+
Config updates now have dedicated regression suites in `packages/opencode/test/config`. The new `.github/workflows/config.yml` workflow runs `bun test packages/opencode/test/config/*` and `bun run --cwd packages/opencode typecheck` whenever config sources, docs, or tests change, ensuring the hot reload story is fully exercised before merging.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
},
5151
"devDependencies": {
5252
"@tsconfig/bun": "catalog:",
53+
"@types/proper-lockfile": "4.1.4",
5354
"husky": "9.1.7",
5455
"prettier": "3.6.2",
5556
"sst": "3.17.23",

packages/opencode/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
"minimatch": "10.0.3",
7676
"open": "10.1.2",
7777
"partial-json": "0.1.7",
78+
"proper-lockfile": "4.1.2",
7879
"remeda": "catalog:",
7980
"solid-js": "catalog:",
8081
"strip-ansi": "7.1.2",

packages/opencode/src/config/backup.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,25 @@
11
import fs from "fs/promises"
2+
import path from "path"
3+
import { Log } from "@/util/log"
4+
5+
const log = Log.create({ service: "config.backup" })
6+
const DAY_MS = 24 * 60 * 60 * 1000
7+
const DEFAULT_TTL_DAYS = 7
8+
9+
const envTtlDays = Number(process.env.OPENCODE_CONFIG_BACKUP_TTL_DAYS)
10+
const CONFIG_BACKUP_TTL_MS =
11+
Number.isFinite(envTtlDays) && envTtlDays > 0 ? envTtlDays * DAY_MS : DEFAULT_TTL_DAYS * DAY_MS
12+
13+
interface CleanupOptions {
14+
ttlMs?: number
15+
}
16+
17+
function resolveTtlMs(override?: number) {
18+
if (typeof override === "number" && Number.isFinite(override) && override > 0) {
19+
return override
20+
}
21+
return CONFIG_BACKUP_TTL_MS
22+
}
223

324
export async function createBackup(filepath: string): Promise<string> {
425
const timestamp = new Date().toISOString().replace(/[:.]/g, "-")
@@ -15,3 +36,47 @@ export async function restoreBackup(backupPath: string, targetPath: string): Pro
1536
await fs.copyFile(backupPath, targetPath)
1637
await fs.unlink(backupPath)
1738
}
39+
40+
export async function cleanupOldBackups(filepath: string, options?: CleanupOptions): Promise<number> {
41+
const ttlMs = resolveTtlMs(options?.ttlMs)
42+
const directory = path.dirname(filepath)
43+
const base = path.basename(filepath)
44+
const entries = await fs.readdir(directory).catch(() => [])
45+
const now = Date.now()
46+
let deleted = 0
47+
48+
for (const entry of entries) {
49+
if (!entry.startsWith(`${base}.bak-`)) {
50+
continue
51+
}
52+
53+
const candidate = path.join(directory, entry)
54+
const stats = await fs.stat(candidate).catch(() => undefined)
55+
if (!stats) {
56+
continue
57+
}
58+
59+
if (now - stats.mtimeMs <= ttlMs) {
60+
continue
61+
}
62+
63+
try {
64+
await fs.unlink(candidate)
65+
deleted += 1
66+
} catch (error) {
67+
log.warn("backup.cleanup.unlinkFailed", {
68+
backupPath: candidate,
69+
error: String(error),
70+
})
71+
}
72+
}
73+
74+
if (deleted > 0) {
75+
log.info("backup.cleanup.completed", {
76+
filepath,
77+
deleted,
78+
})
79+
}
80+
81+
return deleted
82+
}

0 commit comments

Comments
 (0)