Skip to content

Commit 26d2e72

Browse files
王超claude
authored andcommitted
fix(system-prompt): render template variables at spawn; L1c/L1d test-isolation barriers (data loss)
Three fixes from investigating "model-specific prompt: no matching entry found" breaking third-party model launches: 1. Test-isolation L1c/L1d (data loss, root cause): in NODE_TEST_CONTEXT an explicit CCV_LOG_DIR / CLAUDE_CONFIG_DIR is now only honored when it points inside the OS temp root; anything else forces the private guard dir with a loud warning. A ccv-hosted shell exports CCV_LOG_DIR=<real ~/.claude/cc-viewer> to every child, so a direct `node --test <file>` run there (the everyday loop when developing cc-viewer inside ccv) inherited the real user data dir through the explicit-value fast path the existing guards did not cover, and pty-manager.test.js's IM-worker fixture cleanup rmSync'd the user's real global system_prompt/ -- confirmed live; this is what silently deleted the saved deepseek-v4-pro entry and produced the misleading spawn warning. npm run test was never affected (script pins CCV_LOG_DIR=tmp). The wiped entry was restored from the current preset. 2. ${...} template variables are now actually rendered at spawn (new server/lib/system-prompt-render.js wired into spawnClaude). The editor stores placeholders literal by design, but the variable renderer had zero callers on the live path, so injected prompts reached the model with literal ${model.name}/${os.platform}/... -- the whole "Dynamic Parameter Documentation" feature was inert and the third-party-model presets shipped broken text. Files without placeholders pass through untouched (variable collection is lazy, once per spawn); unknown placeholders stay literal so shell syntax like ${HOME} survives; ${model.name} strips the [1m] suffix; cwd-dependent variables resolve against the launched workspace via createSystemPromptVariables( overrides, { cwd }); render failure falls back to the raw file. 3. The "no matching entry found" spawn warning no longer fires on intentional skips: buildSystemPromptFileArgs tags suppressed:'env' (CCV_DISABLE_AUTO_SYSTEM_PROMPT=1) and suppressed:'manual-flag' (a matched entry suppressed by a user-passed --system-prompt[-file]). Tests: new system-prompt-render suite; L1c/L1d accept/reject cases in logdir-test-guard; suppressed-marker cases in system-prompt-files; three existing cases that asserted the old explicit-env semantics updated to simulate production (read-only echo paths). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent ddd023d commit 26d2e72

12 files changed

Lines changed: 364 additions & 23 deletions

findcc.js

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { resolve, join } from 'node:path';
1+
import { resolve, join, sep } from 'node:path';
22
import { fileURLToPath } from 'node:url';
33
import { existsSync, realpathSync, readFileSync } from 'node:fs';
44
import { homedir, tmpdir, arch } from 'node:os';
@@ -22,11 +22,42 @@ const NODE_MODULES = resolve(__dirname, '..');
2222
* Claude Code's config from ~/.claude/ to a custom location.
2323
* @returns {string} absolute path to the Claude config directory
2424
*/
25+
// ████████ Test isolation barrier L1c helper — DO NOT REMOVE (2026-07-12 data loss) ████████
26+
// A ccv-hosted shell exports CCV_LOG_DIR=<real user data dir> (and possibly CLAUDE_CONFIG_DIR)
27+
// into every child process — the claude pty, its Bash tool, any nested shell. A direct
28+
// `node --test <file>` run there inherits those vars and sails through the explicit-value fast
29+
// paths below, handing the test process the REAL user directories; test fixtures/cleanup then
30+
// delete real user data (confirmed 2026-07-12: a pty-manager fixture's finally-rmSync wiped the
31+
// user's global system_prompt/ model prompts). Policy: a test process may only ever target
32+
// disposable temp directories. This helper decides whether an explicit dir qualifies.
33+
function isDisposableTmpPath(p) {
34+
const roots = new Set();
35+
const t = resolve(tmpdir());
36+
roots.add(t);
37+
try { roots.add(realpathSync(t)); } catch { /* keep the unresolved form */ }
38+
if (process.platform !== 'win32') { roots.add('/tmp'); roots.add('/private/tmp'); }
39+
const forms = new Set([resolve(p)]);
40+
try { forms.add(realpathSync(resolve(p))); } catch { /* path may not exist yet */ }
41+
for (const f of forms) {
42+
for (const r of roots) {
43+
if (f === r || f.startsWith(r + sep)) return true;
44+
}
45+
}
46+
return false;
47+
}
48+
2549
export function getClaudeConfigDir() {
2650
const envDir = process.env.CLAUDE_CONFIG_DIR;
2751
if (envDir && typeof envDir === 'string' && envDir.trim()) {
2852
const raw = envDir.trim();
29-
return raw.startsWith('~/') ? join(homedir(), raw.slice(2)) : resolve(raw);
53+
const resolved = raw.startsWith('~/') ? join(homedir(), raw.slice(2)) : resolve(raw);
54+
// ████ L1d: in test context an explicit CLAUDE_CONFIG_DIR must still be a throwaway dir —
55+
// an inherited real config dir would re-open the 2026-06-06 updater CACHE_DIR hole. ████
56+
if (process.env.NODE_TEST_CONTEXT && !isDisposableTmpPath(resolved)) {
57+
console.warn(`[findcc] L1d test-isolation barrier: CLAUDE_CONFIG_DIR="${raw}" is not under the OS temp dir — forcing a private guard config dir (tests may only target disposable temp dirs)`);
58+
return join(tmpdir(), 'cc-viewer-test', `guard-cfg-${process.pid}-${threadId}`);
59+
}
60+
return resolved;
3061
}
3162
// ████████ Test isolation barrier L1b — DO NOT REMOVE (prevents data-loss regressions, 2026-06-06) ████████
3263
// CCV_LOG_DIR=tmp only redirects LOG_DIR; it does not cover this function: updater.js's
@@ -69,7 +100,21 @@ function resolveLogDir() {
69100
return join(tmpdir(), 'cc-viewer-test', `${process.pid}-${threadId}`);
70101
}
71102
const expanded = raw.startsWith('~/') ? join(homedir(), raw.slice(2)) : raw;
72-
return resolve(expanded);
103+
const resolved = resolve(expanded);
104+
// ████████ Test isolation barrier L1c — DO NOT REMOVE (2026-07-12 data loss) ████████
105+
// The NODE_TEST_CONTEXT guard below only covers the no-CCV_LOG_DIR case; this explicit-value
106+
// fast path used to accept ANY inherited dir. Inside a ccv-hosted shell CCV_LOG_DIR points at
107+
// the real ~/.claude/cc-viewer, so a direct `node --test <file>` there resolved LOG_DIR to
108+
// real user data — test cleanup then deleted it (2026-07-12: the user's global system_prompt/
109+
// entries were wiped this way, twice). Tests may only target disposable temp dirs: anything
110+
// outside the OS temp root is forced to the private guard dir. Use CCV_LOG_DIR=tmp or a
111+
// mkdtemp path in tests. Unit test: test/logdir-test-guard.test.js.
112+
if (process.env.NODE_TEST_CONTEXT && !isDisposableTmpPath(resolved)) {
113+
console.warn(`[findcc] L1c test-isolation barrier: CCV_LOG_DIR="${raw}" is not under the OS temp dir — forcing a private guard LOG_DIR (tests may only target disposable temp dirs; use CCV_LOG_DIR=tmp or a mkdtemp path)`);
114+
return join(tmpdir(), 'cc-viewer-test', `guard-${process.pid}-${threadId}`);
115+
}
116+
// ████████████████████████████████████████████████████████████████████████████
117+
return resolved;
73118
}
74119
// Test isolation barrier: in node:test environment (NODE_TEST_CONTEXT is auto-injected by the
75120
// test runner and inherited by spawned child processes via spread env), if CCV_LOG_DIR is not

history.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
## Unreleased
44

5+
- fix(test-isolation, **data loss**): new **L1c/L1d barriers** in `findcc.js` — in a test context (`NODE_TEST_CONTEXT`), an **explicit** `CCV_LOG_DIR` / `CLAUDE_CONFIG_DIR` is now only honored when it points inside the OS temp root; anything else is forced to the private guard dir with a loud warning. Root cause of a real data-loss class the existing L1/L1b guards missed: a ccv-hosted shell exports `CCV_LOG_DIR=<real ~/.claude/cc-viewer>` into every child (the claude pty, its Bash tool, nested shells), so a direct `node --test test/<file>.js` run there — the everyday dev loop when developing cc-viewer inside ccv — inherited the real user data dir straight through the explicit-value fast path (the NODE_TEST_CONTEXT guard only covered the *unset* case), and `test/pty-manager.test.js`'s IM-worker fixture cleanup (`finally { rmSync(join(LOG_DIR,'system_prompt'), {recursive}) }`) then **deleted the user's real global model-prompt directory** (confirmed live: this is what silently ate the saved deepseek-v4-pro entry and produced the misleading "model-specific prompt … no matching entry found" spawn warning; `npm run test` was never affected because the script pins `CCV_LOG_DIR=tmp`). All repo tests already use tmp-based dirs, so nothing legitimate changes; production semantics (no `NODE_TEST_CONTEXT`) are untouched. New L1c/L1d accept/reject cases in `logdir-test-guard`.
6+
7+
- feat(expert/system-prompt): **`${...}` template variables are now actually rendered at spawn** — new `server/lib/system-prompt-render.js`, wired into `pty-manager.spawnClaude`. The Edit System Prompt editor stores presets/entries with placeholders literal (by design), but nothing in the live pipeline ever substituted them: the variable renderer (`renderPreset`/`createSystemPrompt`/`createSystemPromptVariables`) had zero callers on the spawn path, so every model entry and sentinel injected via `--system-prompt-file`/`--append-system-prompt-file` reached the model with literal `${model.name}`, `${os.platform}`, `${memory.index}` … — the entire "Dynamic Parameter Documentation" feature was inert, and the third-party-model presets (deepseek/GLM/Qwen/kimi, whose whole point is replacing the Anthropic-specific system prompt) shipped broken text. Rendering rules: files without placeholders pass through untouched (zero cost — variable collection shells out to git and is now lazy + once per spawn); unknown placeholders stay literal (`missingVariableMode: 'keep'`, so prompt text quoting shell syntax like `${HOME}` survives); `${model.name}` resolves from the spawn's resolved model id with the `[1m]` context-window suffix stripped; cwd-dependent variables (git/cwd/memory) resolve against the launched workspace via a new `createSystemPromptVariables(overrides, { cwd })` param; rendered copies live under `<tmpdir>/cc-viewer-rendered-prompts/<pid>/`; any render failure falls back to injecting the raw file (never breaks the spawn). Tests: new `system-prompt-render` suite (substitution, laziness, unknown-var preservation, fallback, `[1m]` strip, opts.cwd).
8+
9+
- fix(expert/system-prompt): the spawn diagnostic **"modelId resolved but no matching entry found"** no longer fires when injection was *intentionally skipped*`buildSystemPromptFileArgs` now tags `suppressed: 'env'` (kill-switch `CCV_DISABLE_AUTO_SYSTEM_PROMPT=1`) and `suppressed: 'manual-flag'` (a matched model entry suppressed by a user-passed `--system-prompt[-file]`/`--append-system-prompt[-file]`), and `pty-manager` keeps quiet on both; only a genuine no-entry miss warns. Suppressed-marker cases added to `system-prompt-files`.
10+
511
- fix(im, review round): hardening from a six-role review of the drawer Start button. **P1**: the `starting` boolean + platform-guarded `finally` reset leaked permanently when the user switched platforms mid-poll — a loading Start button then appeared on every platform and never stopped spinning until a page reload; `starting` is now a platform-scoped `startingPlatform` (button shows/loads only for the platform being started, functional-update reset clears only its own round). **P2 server**: `'start'` now rejects with 400 `missing <fields>` when required cred/secret fields are unset (new shared `missingCreds`, same gate the `/test` route uses — previously it would persist `enabled:true` for a credential-less platform whose worker no-ops forever, and reconcile would respawn that zombie on every server restart), and flipping `enabled` via `/process` now hits the same empty-allowlist server-side audit warning as the config route (extracted shared `warnIfEmptyAllowlist` — the headless bind-first-conversation warning could previously be bypassed). **P2 client**: the start-success criterion is tightened from `process.state==='ready'` to ready **and** bridge connected (`connection.connected`/`connectionState==='connected'`) in both the drawer and `ImPlatformSettings.start` — `ready` only means the worker's HTTP identity service is up, so stale creds produced a green "Connected" toast contradicting the "Running, connecting…" badge; the drawer's failure toast now appends the server `detail`; the outer POST catch reports via `reportSwallowed` instead of discarding the error; and the drawer mirrors the settings panel's `busyRef` (start-poll pauses the 5s background poll whose failure branch could flash the badge back to "Disconnected" mid-boot) + `mountedRef` (no setState after a real unmount) guards. Tests: idempotent start (already-enabled → prefs file not rewritten, mtime-pinned), creds gate (400, nothing persisted or spawned), and audit-warning cases added to `im-routes-gap`.
612

713
- feat(im): the IM conversation-record drawer (对话记录) gains an inline **Start** button next to the status badge — when the worker is confirmed dead (`process.state === 'dead'`, badge shows "Disconnected"), the user can relaunch the bridge without detouring through the settings modal. The button POSTs the existing loopback-only `POST /api/im/:platform/process {action:'start'}` and then polls `/status` until the worker is truly ready (`state === 'ready'`, same 15s criterion as `ImPlatformSettings.start`; success/failure surfaced as toasts, badge transitions live through Starting… → Connected, a platform-switch mid-poll is guarded by ref comparison so no cross-platform state bleed). Server-side, the `'start'` action now **persists `enabled: true`** first (read-merge-write via `loadConfig`+`saveConfig`, creds/allowlist untouched): a worker spawned while the stored config says disabled no-ops its bridge in `im-bridge-core` and would not survive a restart reconcile, so "start" must mean "enable + spawn" — `'stop'`/`'restart'` semantics unchanged (disabling stays on the config route). Remote (LAN) clients never see the button (their trimmed `/status` carries no `process` info, and `/process` is loopback-only anyway). Reuses existing i18n keys (`ui.im.start`/`ui.im.startFailed`/`ui.im.statusConnected`), no new entries. Tests: `im-routes-gap` gains persist-on-start (creds preserved) and stop-does-not-touch-enabled cases.

server/lib/create_system_prompt.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,11 @@ function resolveMemory(home, cwd) {
130130
return { dir, index, enabled: enabled ? 'true' : 'false' }
131131
}
132132

133-
export function createSystemPromptVariables(overrides = {}) {
134-
const cwd = stringOrEmpty(() => process.cwd())
133+
export function createSystemPromptVariables(overrides = {}, opts = {}) {
134+
// opts.cwd: resolve cwd-dependent variables (environment.cwd, git.*, memory.dir) against a
135+
// caller-supplied directory instead of process.cwd() — the spawn-time renderer passes the
136+
// workspace being launched, which is not necessarily where the ccv server itself runs.
137+
const cwd = (typeof opts.cwd === 'string' && opts.cwd) ? opts.cwd : stringOrEmpty(() => process.cwd())
135138
const now = new Date()
136139
const timeZone = stringOrEmpty(
137140
() => Intl.DateTimeFormat().resolvedOptions().timeZone,

server/lib/system-prompt-files.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,15 @@ function hasArg(args, ...names) {
5353
* @param {string[]} [existingArgs] 已有的 claude 参数(用于「手动优先」判断)
5454
* @param {Object} [env] 环境变量(默认 process.env)
5555
* @param {{ modelId?: string|null, globalModelDir?: string|null }} [opts]
56-
* @returns {{ args: string[], loaded: string[], model: string|null }}
57-
* args: 待追加参数;loaded: 实际加载的文件(终端提示);model: 命中的条目名(未命中为 null)
56+
* @returns {{ args: string[], loaded: string[], model: string|null, suppressed?: 'env'|'manual-flag' }}
57+
* args: 待追加参数;loaded: 实际加载的文件(终端提示);model: 命中的条目名(未命中为 null);
58+
* suppressed: 注入被有意跳过的原因(env 开关 / 手动同义 flag 抑制了已命中的模型条目)——
59+
* 调用方(pty-manager)据此不再打「no matching entry」误导性告警。
5860
*/
5961
export function buildSystemPromptFileArgs(projectDir, existingArgs = [], env = process.env, opts = {}) {
6062
const out = { args: [], loaded: [], model: null };
6163
if (!projectDir) return out;
62-
if (env?.[DISABLE_AUTO_SYSTEM_PROMPT_ENV] === '1') return out;
64+
if (env?.[DISABLE_AUTO_SYSTEM_PROMPT_ENV] === '1') return { ...out, suppressed: 'env' };
6365

6466
if (opts?.modelId) {
6567
const candidates = [{ dir: join(projectDir, MODEL_PROMPT_DIR), scope: 'workspace' }];
@@ -73,6 +75,8 @@ export function buildSystemPromptFileArgs(projectDir, existingArgs = [], env = p
7375
out.args.push(flagPair[1], match.path);
7476
out.loaded.push(`${match.scope === 'global' ? 'global ' : ''}${MODEL_PROMPT_DIR}/${match.fileName}`);
7577
out.model = match.name;
78+
} else {
79+
out.suppressed = 'manual-flag'; // 条目命中但用户手传了同义 flag:有意跳过,非「无条目」
7680
}
7781
return out; // 命中即返回:默认 sentinel 不再参与(含手动 flag 抑制注入的情况)。
7882
}

server/lib/system-prompt-render.js

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Spawn-time `${...}` template-variable rendering for injected system-prompt files.
2+
//
3+
// The Edit System Prompt editor stores entries (model entries under system_prompt/ and the
4+
// CC_SYSTEM.md / CC_APPEND_SYSTEM.md sentinels) with `${...}` placeholders LITERAL — that is the
5+
// editing surface documented by the "Dynamic Parameter Documentation" popup. The substitution has
6+
// to happen when claude is launched: this module takes the args produced by
7+
// buildSystemPromptFileArgs, and for every injected file whose content contains template
8+
// variables, renders it via create_system_prompt.js and swaps in a rendered temp copy.
9+
//
10+
// Design constraints:
11+
// - Files without `${...}` pass through untouched (zero cost — no variable collection, which
12+
// shells out to git several times, and no temp file).
13+
// - missingVariableMode is 'keep': unknown placeholders stay literal, so user-authored prompt
14+
// text quoting shell syntax like `${HOME}` or `${1:-default}` is never eaten.
15+
// - Any failure (unreadable file, variable collection throwing, temp write failing) falls back
16+
// to the raw path — rendering must never break the claude spawn.
17+
// - Temp copies live under <tmpdir>/cc-viewer-rendered-prompts/<pid>/: per-process so two ccv
18+
// instances never clobber each other; sequential spawns of one instance overwrite the same
19+
// basename, which is safe because claude reads the file once at startup.
20+
import { readFileSync, writeFileSync, mkdirSync } from 'node:fs';
21+
import { join, basename } from 'node:path';
22+
import { tmpdir } from 'node:os';
23+
import { createSystemPrompt, createSystemPromptVariables } from './create_system_prompt.js';
24+
25+
// Detection only (no capture): does the text contain at least one `${...}` placeholder?
26+
const TEMPLATE_VARIABLE_RE = /\$\{[^}]+\}/;
27+
28+
/** Per-process directory holding the rendered temp copies. */
29+
export function renderedPromptDir(pid = process.pid) {
30+
return join(tmpdir(), 'cc-viewer-rendered-prompts', String(pid));
31+
}
32+
33+
/**
34+
* Render template variables in the file args produced by buildSystemPromptFileArgs.
35+
*
36+
* @param {{ args: string[], loaded: string[], model: string|null }} sysPrompt
37+
* @param {{ cwd?: string, modelId?: string|null, variablesFactory?: Function }} [opts]
38+
* cwd: workspace being launched (git/cwd/memory variables resolve against it);
39+
* modelId: resolved model id from the last launch — becomes ${model.name} (the [1m]
40+
* context-window suffix is stripped: it is a claude-code UI notation, not a model name);
41+
* variablesFactory: test seam replacing createSystemPromptVariables.
42+
* @returns {{ args: string[], loaded: string[], model: string|null }} same shape; file paths
43+
* whose content was rendered point at the temp copy instead.
44+
*/
45+
export function renderSystemPromptFileArgs(sysPrompt, opts = {}) {
46+
const args = Array.isArray(sysPrompt?.args) ? sysPrompt.args : [];
47+
if (args.length === 0) return sysPrompt;
48+
49+
let variables = null; // lazy: collected once, and only if some file actually has placeholders
50+
const out = [...args];
51+
for (let i = 0; i + 1 < out.length; i += 2) {
52+
const flag = out[i];
53+
if (flag !== '--system-prompt-file' && flag !== '--append-system-prompt-file') continue;
54+
const path = out[i + 1];
55+
try {
56+
const text = readFileSync(path, 'utf-8');
57+
if (!TEMPLATE_VARIABLE_RE.test(text)) continue;
58+
if (!variables) {
59+
const factory = opts.variablesFactory || createSystemPromptVariables;
60+
const overrides = {};
61+
if (opts.modelId) overrides.model = { name: String(opts.modelId).replace(/\[1m\]$/, '') };
62+
variables = factory(overrides, { cwd: opts.cwd });
63+
}
64+
const rendered = createSystemPrompt(text, { variables, missingVariableMode: 'keep' });
65+
if (rendered === text) continue; // every placeholder unknown → nothing changed, keep raw path
66+
const dir = renderedPromptDir();
67+
mkdirSync(dir, { recursive: true });
68+
const target = join(dir, basename(path));
69+
writeFileSync(target, rendered, 'utf-8');
70+
out[i + 1] = target;
71+
} catch (e) {
72+
console.warn(`[CC Viewer] system-prompt template render failed for ${path} (injecting the raw file):`, e?.message || e);
73+
}
74+
}
75+
return { ...sysPrompt, args: out };
76+
}

0 commit comments

Comments
 (0)