Skip to content

Commit 0c9df9f

Browse files
ualtinokualtinok
committed
mason: D15 CLI/config hardening batch
- Coerce/merge experimental.pin_key_files (doctor helper + in-memory shim) - TUI plugin writes: tuple/dev-path preservation, atomic tui.json writes - Atomic config writes in CLI setup/doctor/adapters - Unify OpenCode install detection via findOnPath + stock bin - migrate-session --dry-run opens context/opencode DBs read-only - Pi setup merges existing embedding block on write - Add config-docs drift guard test (build-config-docs.test.ts)
1 parent cc9efd2 commit 0c9df9f

18 files changed

Lines changed: 409 additions & 139 deletions

‎packages/cli/src/adapters/opencode.ts‎

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
import { existsSync, readFileSync, writeFileSync } from "node:fs";
2-
import { homedir } from "node:os";
3-
import { dirname, join } from "node:path";
1+
import { existsSync, readFileSync } from "node:fs";
2+
import { dirname } from "node:path";
43
import { parse as parseJsonc, stringify as stringifyJsonc } from "comment-json";
5-
import { findOnPath } from "../lib/find-on-path";
4+
import { writeFileAtomic } from "../lib/atomic-write";
5+
import { isOpenCodeInstalledOnSystem } from "../lib/opencode-install";
66
import {
77
detectConfigPaths,
88
dirSizeBytes,
@@ -25,23 +25,7 @@ export class OpenCodeAdapter implements HarnessAdapter {
2525
readonly pluginPackageName = PLUGIN_NAME;
2626

2727
isInstalled(): boolean {
28-
// Stock OpenCode install location takes priority. Use os.homedir()
29-
// (not process.env.HOME) and OS-specific binary name so this works
30-
// on Windows where HOME is typically undefined and binaries end in
31-
// `.exe`.
32-
const isWindows = process.platform === "win32";
33-
const stockBin = isWindows
34-
? join(homedir(), ".opencode", "bin", "opencode.exe")
35-
: join(homedir(), ".opencode", "bin", "opencode");
36-
if (existsSync(stockBin)) return true;
37-
38-
// PATH walk via Node primitives (issue #75). Previously shelled out
39-
// to `which`/`where`, which fails in environments where those
40-
// binaries aren't on PATH (Alpine/slim containers, bunx sandboxes,
41-
// NixOS) — even when `opencode` itself is reachable. Manually
42-
// walking PATH using process.env.PATH + path.delimiter is portable
43-
// and works regardless of which shell utilities are installed.
44-
return findOnPath("opencode") !== null;
28+
return isOpenCodeInstalledOnSystem();
4529
}
4630

4731
hasPluginEntry(): boolean {
@@ -80,7 +64,7 @@ export class OpenCodeAdapter implements HarnessAdapter {
8064
plugin: [PLUGIN_ENTRY],
8165
};
8266
ensureDir(target);
83-
writeFileSync(target, `${JSON.stringify(initial, null, 4)}\n`);
67+
writeFileAtomic(target, `${JSON.stringify(initial, null, 4)}\n`);
8468
return {
8569
ok: true,
8670
action: "added",
@@ -112,7 +96,7 @@ export class OpenCodeAdapter implements HarnessAdapter {
11296
if (existingIdx === -1 && existingDevIdx === -1) {
11397
plugin.push(PLUGIN_ENTRY);
11498
cfg.plugin = plugin;
115-
writeFileSync(target, `${stringifyJsonc(cfg, null, 4)}\n`);
99+
writeFileAtomic(target, `${stringifyJsonc(cfg, null, 4)}\n`);
116100
return {
117101
ok: true,
118102
action: "added",
@@ -136,7 +120,7 @@ export class OpenCodeAdapter implements HarnessAdapter {
136120
if (typeof current === "string" && current !== PLUGIN_ENTRY) {
137121
plugin[existingIdx] = PLUGIN_ENTRY;
138122
cfg.plugin = plugin;
139-
writeFileSync(target, `${stringifyJsonc(cfg, null, 4)}\n`);
123+
writeFileAtomic(target, `${stringifyJsonc(cfg, null, 4)}\n`);
140124
return {
141125
ok: true,
142126
action: "updated",
@@ -194,7 +178,7 @@ export class OpenCodeAdapter implements HarnessAdapter {
194178
configPath: target,
195179
};
196180
}
197-
writeFileSync(target, `${stringifyJsonc(cfg, null, 4)}\n`);
181+
writeFileAtomic(target, `${stringifyJsonc(cfg, null, 4)}\n`);
198182
return {
199183
ok: true,
200184
action: "updated",

‎packages/cli/src/adapters/pi.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { execFileSync } from "node:child_process";
2-
import { existsSync, readFileSync, writeFileSync } from "node:fs";
2+
import { existsSync, readFileSync } from "node:fs";
3+
import { writeFileAtomic } from "../lib/atomic-write";
34
import { dirname } from "node:path";
45
import { parse as parseJsonc, stringify as stringifyJsonc } from "comment-json";
56
import { getMagicContextLogPath, getPiAgentConfigDir, getPiUserExtensionsPath } from "../lib/paths";
@@ -179,7 +180,7 @@ function writePiSettings(settings: PiSettingsLike): void {
179180
const settingsPath = getPiUserExtensionsPath();
180181
ensureDir(settingsPath);
181182
const text = stringifyJsonc(settings, null, 2);
182-
writeFileSync(settingsPath, `${text}\n`);
183+
writeFileAtomic(settingsPath, `${text}\n`);
183184
}
184185

185186
/**

‎packages/cli/src/commands/doctor-opencode.ts‎

Lines changed: 8 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { execSync, spawnSync } from "node:child_process";
2-
import { existsSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
2+
import { existsSync, readFileSync, rmSync, statSync } from "node:fs";
33
import { createRequire } from "node:module";
44
import { homedir } from "node:os";
55
import { join } from "node:path";
@@ -21,9 +21,11 @@ import { Database } from "@magic-context/core/shared/sqlite";
2121
import { ensureTuiPluginEntry } from "@magic-context/core/shared/tui-config";
2222
import { parse, stringify } from "comment-json";
2323
import { isDevPathPluginEntry, matchesPluginEntry } from "../adapters/opencode";
24+
import { writeFileAtomic } from "../lib/atomic-write";
2425
import { collectDiagnostics } from "../lib/diagnostics-opencode";
2526
import { checkLocalEmbeddingRuntime } from "../lib/embedding-runtime";
2627
import { bundleIssueReport } from "../lib/logs-opencode";
28+
import { migrateExperimentalPinKeyFilesForDoctor } from "../lib/migrate-experimental-doctor";
2729
import { isOpenCodeInstalled } from "../lib/opencode-helpers";
2830
import { detectConfigPaths, getMagicContextLogPath } from "../lib/paths";
2931
import { confirm, intro, log, outro, selectOne, spinner, text } from "../lib/prompts";
@@ -800,45 +802,7 @@ export async function runDoctor(
800802
fixed++;
801803
}
802804

803-
// Migrate experimental.pin_key_files → dreamer.pin_key_files.
804-
// Same story as user_memories — feature graduated to stable under
805-
// dreamer config. Existing `enabled: true` values are preserved so
806-
// we do not silently disable the feature for users who opted in.
807-
if (experimental && "pin_key_files" in experimental) {
808-
const dreamer = (mcConfig.dreamer as Record<string, unknown> | undefined) ?? {};
809-
const oldPKF = experimental.pin_key_files;
810-
const existingPKF = dreamer.pin_key_files;
811-
if (existingPKF === undefined) {
812-
dreamer.pin_key_files = oldPKF;
813-
} else if (
814-
typeof oldPKF === "object" &&
815-
oldPKF !== null &&
816-
typeof existingPKF === "object" &&
817-
existingPKF !== null
818-
) {
819-
// Merge field-by-field so sub-fields like `token_budget`
820-
// and `min_reads` from the old block are not silently lost
821-
// when a user has partially graduated.
822-
const merged = {
823-
...(oldPKF as Record<string, unknown>),
824-
...(existingPKF as Record<string, unknown>),
825-
};
826-
dreamer.pin_key_files = merged;
827-
} else if (typeof oldPKF === "object" && oldPKF !== null) {
828-
// Old block is a proper object but new block is a malformed
829-
// primitive. Coerce and merge so sub-fields like
830-
// `token_budget` / `min_reads` are not dropped.
831-
const coerced: Record<string, unknown> = {
832-
...(oldPKF as Record<string, unknown>),
833-
enabled: Boolean(existingPKF),
834-
};
835-
dreamer.pin_key_files = coerced;
836-
log.warn(
837-
`Coerced malformed dreamer.pin_key_files (${typeof existingPKF}) to object form while merging sub-fields from experimental.pin_key_files`,
838-
);
839-
}
840-
mcConfig.dreamer = dreamer;
841-
delete experimental.pin_key_files;
805+
if (experimental && migrateExperimentalPinKeyFilesForDoctor(mcConfig)) {
842806
mcChanged = true;
843807
log.success(
844808
"Migrated experimental.pin_key_files → dreamer.pin_key_files (preserved user enabled state)",
@@ -910,7 +874,7 @@ export async function runDoctor(
910874
}
911875

912876
if (mcChanged) {
913-
writeFileSync(paths.magicContextConfig, `${stringify(mcConfig, null, 2)}\n`);
877+
writeFileAtomic(paths.magicContextConfig, `${stringify(mcConfig, null, 2)}\n`);
914878
}
915879
} catch {
916880
log.warn("Could not migrate deprecated config keys in magic-context.jsonc");
@@ -982,7 +946,7 @@ export async function runDoctor(
982946
rawPlugins[existingIdx] = PLUGIN_ENTRY_WITH_VERSION;
983947
}
984948
config.plugin = rawPlugins;
985-
writeFileSync(paths.opencodeConfig, `${stringify(config, null, 2)}\n`);
949+
writeFileAtomic(paths.opencodeConfig, `${stringify(config, null, 2)}\n`);
986950
pass(
987951
`Upgraded plugin entry in ${configName}: ${oldEntryStr} → ${PLUGIN_ENTRY_WITH_VERSION}`,
988952
);
@@ -994,7 +958,7 @@ export async function runDoctor(
994958
// tuple/options entry the user already had.
995959
rawPlugins.push(PLUGIN_ENTRY_WITH_VERSION);
996960
config.plugin = rawPlugins;
997-
writeFileSync(paths.opencodeConfig, `${stringify(config, null, 2)}\n`);
961+
writeFileAtomic(paths.opencodeConfig, `${stringify(config, null, 2)}\n`);
998962
pass(`Added plugin to ${configName}`);
999963
fixed++;
1000964
}
@@ -1071,7 +1035,7 @@ export async function runDoctor(
10711035
tuiRawPlugins[tuiIdx] = PLUGIN_ENTRY_WITH_VERSION;
10721036
}
10731037
tuiConfig.plugin = tuiRawPlugins;
1074-
writeFileSync(paths.tuiConfig, `${stringify(tuiConfig, null, 2)}\n`);
1038+
writeFileAtomic(paths.tuiConfig, `${stringify(tuiConfig, null, 2)}\n`);
10751039
pass(`Upgraded TUI plugin: ${tuiEntryStr} → ${PLUGIN_ENTRY_WITH_VERSION}`);
10761040
fixed++;
10771041
} else {

‎packages/cli/src/commands/doctor-pi.ts‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,9 @@
11
import { execFileSync, spawnSync } from "node:child_process";
2-
import {
3-
existsSync,
4-
mkdirSync,
5-
readdirSync,
6-
readFileSync,
7-
rmSync,
8-
statSync,
9-
writeFileSync,
10-
} from "node:fs";
2+
import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync } from "node:fs";
113
import { createRequire } from "node:module";
124
import { homedir } from "node:os";
135
import { dirname, isAbsolute, join } from "node:path";
6+
import { writeFileAtomic } from "../lib/atomic-write";
147
import {
158
dropInheritedEmbeddingKeyOnRedirect,
169
stripUnsafeProjectConfigFields,
@@ -804,7 +797,7 @@ function writeDefaultMagicContextConfig(path: string): void {
804797
"https://raw.githubusercontent.com/cortexkit/magic-context/master/assets/magic-context.schema.json",
805798
...MagicContextConfigSchema.parse({}),
806799
};
807-
writeFileSync(path, `${stringifyJsonc(config, null, 2)}\n`);
800+
writeFileAtomic(path, `${stringifyJsonc(config, null, 2)}\n`);
808801
}
809802

810803
function repair(plan: RepairPlan, prompts: PromptIO): number {

‎packages/cli/src/commands/migrate-session.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,8 +505,9 @@ export async function runMigrateSessionCli(args: string[]): Promise<number> {
505505

506506
const opencodeDbPath = defaultOpenCodeDbPath();
507507
const contextDbPath = defaultContextDbPath();
508-
const opencodeDb = new Database(opencodeDbPath);
509-
const contextDb = new Database(contextDbPath);
508+
const dbReadonly = dryRun ? { readonly: true } : undefined;
509+
const opencodeDb = new Database(opencodeDbPath, dbReadonly);
510+
const contextDb = new Database(contextDbPath, dbReadonly);
510511
// This CLI opens context.db directly (bypassing initializeDatabase), so the
511512
// pragmas the plugin normally sets are absent. foreign_keys=ON is REQUIRED:
512513
// the collision-merge path (rekeyMemoryRowWithCollisionMerge) relies on

‎packages/cli/src/commands/setup-opencode.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
1+
import { existsSync, mkdirSync, readFileSync } from "node:fs";
2+
import { writeFileAtomic } from "../lib/atomic-write";
23
import { dirname } from "node:path";
34
import { detectConflicts } from "@magic-context/core/shared/conflict-detector";
45
import { fixConflicts } from "@magic-context/core/shared/conflict-fixer";
@@ -44,7 +45,7 @@ function addPluginToOpenCodeConfig(configPath: string, format: "json" | "jsonc"
4445
plugin: [PLUGIN_ENTRY],
4546
compaction: { auto: false, prune: false },
4647
};
47-
writeFileSync(configPath, `${stringifyJsonc(config, null, 2)}\n`);
48+
writeFileAtomic(configPath, `${stringifyJsonc(config, null, 2)}\n`);
4849
return;
4950
}
5051

@@ -79,14 +80,14 @@ function addPluginToOpenCodeConfig(configPath: string, format: "json" | "jsonc"
7980
compaction.prune = false;
8081
existing.compaction = compaction;
8182

82-
writeFileSync(configPath, `${stringifyJsonc(existing, null, 2)}\n`);
83+
writeFileAtomic(configPath, `${stringifyJsonc(existing, null, 2)}\n`);
8384
}
8485

8586
function addPluginToTuiConfig(configPath: string, format: "json" | "jsonc" | "none"): void {
8687
ensureDir(dirname(configPath));
8788

8889
if (format === "none") {
89-
writeFileSync(configPath, `${stringifyJsonc({ plugin: [PLUGIN_ENTRY] }, null, 2)}\n`);
90+
writeFileAtomic(configPath, `${stringifyJsonc({ plugin: [PLUGIN_ENTRY] }, null, 2)}\n`);
9091
return;
9192
}
9293

@@ -107,7 +108,7 @@ function addPluginToTuiConfig(configPath: string, format: "json" | "jsonc" | "no
107108
}
108109

109110
existing.plugin = rawPlugins;
110-
writeFileSync(configPath, `${stringifyJsonc(existing, null, 2)}\n`);
111+
writeFileAtomic(configPath, `${stringifyJsonc(existing, null, 2)}\n`);
111112
}
112113

113114
export function findDcpPluginIndexes(plugins: unknown[]): number[] {
@@ -141,7 +142,7 @@ async function resolveDcpConflictBeforeSetup(
141142
const shouldRemove = await confirm("Remove opencode-dcp from your config?", true);
142143
if (shouldRemove) {
143144
ocConfig.plugin = plugins.filter((_plugin, index) => !dcpIndexes.includes(index));
144-
writeFileSync(configPath, `${stringifyJsonc(ocConfig, null, 2)}\n`);
145+
writeFileAtomic(configPath, `${stringifyJsonc(ocConfig, null, 2)}\n`);
145146
log.success("Removed opencode-dcp from plugin list");
146147
} else {
147148
log.warn("Skipped — you may experience context management conflicts");
@@ -208,7 +209,7 @@ function writeMagicContextConfig(
208209
config.cache_ttl = cacheTtl;
209210
}
210211

211-
writeFileSync(configPath, `${stringifyJsonc(config, null, 2)}\n`);
212+
writeFileAtomic(configPath, `${stringifyJsonc(config, null, 2)}\n`);
212213
}
213214
// ─── Main Setup Flow ──────────────────────────────────────
214215

‎packages/cli/src/commands/setup-pi.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
1+
import { existsSync, mkdirSync, readFileSync } from "node:fs";
2+
import { writeFileAtomic } from "../lib/atomic-write";
23
import { dirname } from "node:path";
34

45
import { parse as parseJsonc, stringify as stringifyJsonc } from "comment-json";
@@ -118,7 +119,7 @@ export function writePiSettingsPackage(
118119

119120
if (!hasPackage) packages.push(packageSource);
120121
settings.packages = packages;
121-
writeFileSync(settingsPath, `${stringifyJsonc(settings, null, 2)}\n`);
122+
writeFileAtomic(settingsPath, `${stringifyJsonc(settings, null, 2)}\n`);
122123
return !hasPackage;
123124
}
124125

@@ -165,8 +166,11 @@ export function writeMagicContextConfig(
165166
};
166167
config.sidekick = compactObject(sidekick);
167168

168-
config.embedding = options.embedding;
169-
writeFileSync(configPath, `${stringifyJsonc(config, null, 2)}\n`);
169+
config.embedding = {
170+
...((config.embedding as Record<string, unknown> | undefined) ?? {}),
171+
...options.embedding,
172+
};
173+
writeFileAtomic(configPath, `${stringifyJsonc(config, null, 2)}\n`);
170174
}
171175

172176
async function chooseEmbedding(prompts: PromptIO): Promise<EmbeddingChoice> {

‎packages/cli/src/lib/atomic-write.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { afterEach, describe, expect, it } from "bun:test";
2+
import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, statSync } from "node:fs";
3+
import { tmpdir } from "node:os";
4+
import { join } from "node:path";
5+
import { writeFileAtomic } from "./atomic-write";
6+
7+
const roots: string[] = [];
8+
9+
afterEach(() => {
10+
for (const root of roots.splice(0)) {
11+
rmSync(root, { recursive: true, force: true });
12+
}
13+
});
14+
15+
describe("writeFileAtomic", () => {
16+
it("writes content and leaves no .tmp sibling", () => {
17+
const root = mkdtempSync(join(tmpdir(), "mc-atomic-"));
18+
roots.push(root);
19+
const target = join(root, "config.jsonc");
20+
writeFileAtomic(target, '{"ok":true}\n');
21+
expect(readFileSync(target, "utf-8")).toBe('{"ok":true}\n');
22+
expect(existsSync(`${target}.tmp`)).toBe(false);
23+
});
24+
25+
it("preserves file mode on replace", () => {
26+
const root = mkdtempSync(join(tmpdir(), "mc-atomic-mode-"));
27+
roots.push(root);
28+
const target = join(root, "config.jsonc");
29+
writeFileAtomic(target, "v1\n");
30+
chmodSync(target, 0o600);
31+
writeFileAtomic(target, "v2\n");
32+
expect(readFileSync(target, "utf-8")).toBe("v2\n");
33+
expect(statSync(target).mode & 0o777).toBe(0o600);
34+
});
35+
});

‎packages/cli/src/lib/atomic-write.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { chmodSync, renameSync, statSync, writeFileSync } from "node:fs";
2+
3+
/**
4+
* Write a file atomically: temp sibling + rename. Preserves prior mode when the
5+
* target already exists so user chmod settings survive doctor/setup rewrites.
6+
*/
7+
export function writeFileAtomic(targetPath: string, data: string): void {
8+
const tmpPath = `${targetPath}.tmp`;
9+
writeFileSync(tmpPath, data, { encoding: "utf-8" });
10+
try {
11+
if (statSync(targetPath, { throwIfNoEntry: false })?.isFile()) {
12+
const mode = statSync(targetPath).mode & 0o777;
13+
chmodSync(tmpPath, mode);
14+
}
15+
} catch {
16+
// New file — default umask applies.
17+
}
18+
renameSync(tmpPath, targetPath);
19+
}

0 commit comments

Comments
 (0)