Compare commits

...
Author SHA1 Message Date
taodeng a48910191b docs(readme): cache-staleness caveat on CLAUDE_SYSTEM_PROMPT row (reviewer advisory) 2026-07-17 19:18:25 +10:00
taodengandClaude <claude-opus-4-8> <noreply@anthropic.com> c377219c1c fix(server): wire CLAUDE_SYSTEM_PROMPT into the composed system prompt (was dead since APPEND_SYSTEM_PROMPT retirement)
The var was read (SYSTEM_PROMPT, server.mjs), documented in the file header as
"appended to all requests", and echoed on /health.systemPrompt — but nothing on
the request path consumed it: extractSystemPrompt() composed only the wrapper +
client system messages. The wiring was lost when APPEND_SYSTEM_PROMPT was
retired, leaving the header comment and the buildCliArgs comment describing
behavior that did not exist (caught by the PR #170 independent reviewer).

Wire, not delete, because:
- the /health `systemPrompt` field is part of the grandfathered B.2 contract
  (ADR 0006, frozen at v3.16.4) — removal would need an ADR; wiring keeps the
  shape and makes the field honest;
- fleet check: no deployment sets the var (Mac prod plist, Oracle systemd unit,
  PI231 /etc/ocp/ocp.env all clean), so wiring changes behavior for NOBODY today;
- with the var unset, appendOperatorPrompt returns its input string unchanged —
  the default path is byte-for-byte identical.

Mechanics: new pure lib/prompt.mjs `appendOperatorPrompt(base, operatorAppend)`
(trimmed; whitespace-only treated as unset so a stray space in a service unit
cannot inject "\n\n " into every request), applied as the LAST segment in both
extractSystemPrompt branches — an operator-wide directive reads as the final
instruction, after client system messages. TUI-mode is untouched (panes keep the
interactive CLI's own system prompt); documented in the README row.

ALIGNMENT.md Rule 2: no cli.js citation applies. No endpoint, header, or wire
field is added or altered; the change affects only the CONTENT OCP passes to the
already-established `--system-prompt` flag (file header § verified v2.1.104),
i.e. OCP-owned prompt composition. /health shape unchanged.

Tests: +3, doubly mutation-proven (unconditional-base revert → 2 failures;
trim removal → 2 failures). Suite 341 passed / 0 failed.

Co-Authored-By: Claude <claude-opus-4-8> <noreply@anthropic.com>
2026-07-17 19:11:58 +10:00
dtzp555-maxGitHubtaodengClaude <claude-opus-4-8> <noreply@anthropic.com>
f14f4ec754 fix(upgrade): doctor fetches tags before deciding latest + post-flight asserts served version (#173) (#174)
Two fixes for the two halves of issue #173, both from live incidents during the
2026-07-17 fleet update:

1. scripts/doctor.mjs — `git show origin/main:package.json` reads the LOCALLY
   CACHED remote ref; without a fetch first, any machine that hadn't pulled since
   the last release saw latest == current and reported "Already at latest" (live
   repro: Oracle VM at 3.21.1 with v3.22.1 released). The doctor now runs
   `git fetch --tags --quiet` (15s timeout) before comparing, gated on
   !opts.skipNetwork; on failure (offline/auth) it falls through to the cached
   ref — the pre-existing behavior. All existing doctor tests pass mockLatest +
   skipNetwork, so no test touches the network.

2. scripts/upgrade.mjs — post-flight accepted any healthy /health (auth.ok only),
   so a stale process holding the port passed post-flight while still serving the
   OLD version (live repro: a Jul-7 nohup-fallback orphan held :3456; upgrade
   "succeeded", /health kept serving 3.21.1). New exported predicate
   postFlightOk(body, target): auth.ok AND /health.version === target (leading-v
   tolerant; empty target degrades to the auth-only check, never blocks). The
   failure message now reports the last-seen version and points at the
   stale-process diagnosis (`ss -ltnp` / `lsof -i`).

Tests: +4, mutation-proven — reverting the predicate to auth-only fails the
"orphan case" test (337/1). Full suite 338 passed / 0 failed.

No server.mjs change — scripts layer only; no cli.js operation involved, so no
citation applies.

Closes #173

Co-authored-by: dtzp555 <dtzp555@gmail.com>
Co-authored-by: Claude <claude-opus-4-8> <noreply@anthropic.com>
2026-07-17 18:52:45 +10:00
6 changed files with 108 additions and 7 deletions
+1
View File
@@ -228,6 +228,7 @@ The canonical list lives in [`models.json`](./models.json) — the single source
| `CLAUDE_ALLOWED_TOOLS` | `Bash,Read,...,Agent` | Comma-separated tools to pre-approve |
| `CLAUDE_SKIP_PERMISSIONS` | `false` | Bypass all permission checks |
| `CLAUDE_MCP_CONFIG` | *(unset)* | Path to an MCP server config JSON, passed to the spawned `claude` as `--mcp-config` (both the `-p` path and TUI `OCP_TUI_FULL_TOOLS` panes) |
| `CLAUDE_SYSTEM_PROMPT` | *(unset)* | Operator-wide system-prompt text appended (last) to every request's composed system prompt on the default `-p` path. TUI-mode panes are unaffected (they keep the interactive CLI's own system prompt). Echoed truncated on `/health.systemPrompt`. Note: the response cache key does not include server config — after changing this value, flush the cache (`ocp clear`) or let TTL expire. |
| `CLAUDE_NO_CONTEXT` | `false` | Suppress CLAUDE.md and auto-memory injection (pure API mode) |
| `PROXY_API_KEY` | *(unset)* | Bearer token for shared-mode authentication |
| `PROXY_ANONYMOUS_KEY` | *(unset)* | Well-known anonymous key (multi mode) — this exact string bypasses `validateKey()` and grants public access. Exposed via `/health.anonymousKey` only to localhost, or to all callers when `PROXY_ADVERTISE_ANON_KEY=1`. Full setup + security notes: [docs/lan-mode.md § Anonymous Access](docs/lan-mode.md#anonymous-access-optional). |
+17
View File
@@ -0,0 +1,17 @@
// lib/prompt.mjs — pure operator-append step for the system prompt.
//
// Extracted so the rule is unit-testable (the suite never imports server.mjs — it
// boots a listener). server.mjs composes wrapper + client system messages exactly as
// before, then passes the result through this. With CLAUDE_SYSTEM_PROMPT unset the
// return is the INPUT STRING UNCHANGED — the default path stays byte-for-byte
// identical, which is the repo's bar for touching a request-shaping function.
//
// The operator prompt goes LAST deliberately: a server-wide directive ("answer in
// Chinese") should read as the final instruction, not something a client system
// message overrides by coming later. Whitespace-only values are treated as unset —
// a stray space in a service unit's Environment= line must not inject "\n\n " into
// every request.
export function appendOperatorPrompt(base, operatorAppend) {
const op = typeof operatorAppend === "string" ? operatorAppend.trim() : "";
return op ? `${base}\n\n${op}` : base;
}
+10
View File
@@ -59,6 +59,16 @@ export async function runDoctor(opts = {}) {
// of recommending a downgrade against a stale hardcoded value.
let latestVersion = opts.mockLatest;
if (!latestVersion) {
// Issue #173: `git show origin/main:...` reads the LOCALLY CACHED remote ref. Without a
// fetch first, a machine that hasn't pulled since the last release sees latest == current
// and reports noop — new releases were invisible everywhere except the machine that cut
// the tag (live repro: Oracle VM, 2026-07-17). Fetch before comparing; on failure
// (offline, auth, timeout) fall through to the cached ref — the pre-existing behavior.
if (!opts.skipNetwork) {
try {
execSync(`git -C ${ocpDir} fetch --tags --quiet`, { stdio: ["pipe", "pipe", "pipe"], timeout: 15000 });
} catch { /* offline → compare against cached origin/main, as before */ }
}
try {
const out = execSync(`git -C ${ocpDir} show origin/main:package.json 2>/dev/null`, { stdio: ["pipe", "pipe", "pipe"] }).toString();
const remotePkg = JSON.parse(out);
+22 -2
View File
@@ -17,6 +17,20 @@ import { existsSync, copyFileSync } from "node:fs";
import { writeSnapshot, listSnapshots, readSnapshot, gcSnapshots } from "./lib/snapshot.mjs";
import { DEFAULT_PORT } from "../lib/constants.mjs";
// Post-flight acceptance predicate (issue #173). A health probe passes ONLY when the server
// is authed AND actually serving the TARGET version. auth.ok alone is not enough: a stale
// process holding the port answers auth.ok=true while still running the OLD code — exactly
// what a nohup-fallback orphan did on 2026-07-17 (upgrade "succeeded", /health kept serving
// 3.21.1). Comparing /health.version to the checkout target catches orphan-holds-port,
// restart-didn't-take, and wrong-unit-restarted alike. `target` tolerates a leading "v"
// (doctor reports "v3.22.1"; /health reports "3.22.1"); an empty/unknown target degrades to
// the old auth-only check rather than blocking an otherwise-good upgrade.
export function postFlightOk(body, target) {
if (body?.auth?.ok !== true) return false;
const want = String(target || "").replace(/^v/, "");
return !want || body?.version === want;
}
export async function runUpgrade(opts = {}) {
const dryRun = !!opts.dryRun;
const yes = !!opts.yes;
@@ -137,16 +151,22 @@ async function runFullUpgrade({ doctor, opts }) {
if (!opts.mockExec) {
const port = process.env.CLAUDE_PROXY_PORT || String(DEFAULT_PORT);
let ok = false;
let lastSeen = null;
for (let i = 0; i < 10; i++) {
try {
const out = execSync(`curl -sf --max-time 2 http://127.0.0.1:${port}/health`).toString();
const body = JSON.parse(out);
if (body.auth?.ok === true) { ok = true; break; }
lastSeen = body.version;
if (postFlightOk(body, doctor.latest_version)) { ok = true; break; }
} catch { /* retry */ }
await new Promise(r => setTimeout(r, 1000));
}
if (!ok) {
phases.push({ name: "post-flight", status: "fail", message: "health did not return auth.ok=true within 10s" });
phases.push({
name: "post-flight", status: "fail",
message: `health did not return auth.ok=true AND version=${doctor.latest_version} within 10s`
+ (lastSeen ? ` (last saw version=${lastSeen} — a stale process may still hold the port; check \`ss -ltnp\` / \`lsof -i\`)` : ""),
});
throw new Error("post-flight failed");
}
execSync(`curl -sf --max-time 3 http://127.0.0.1:${port}/v1/models > /dev/null`);
+7 -4
View File
@@ -49,6 +49,7 @@ import { TuiSemaphore, SemaphoreAbortError, recordTuiEntrypoint, buildTuiHealthB
import { TuiPanePool, resolvePoolSize, POOL_MAX_SIZE } from "./lib/tui/pool.mjs";
import { TuiDeltaAssembler, DEFAULT_HOLDBACK_CHARS, resolveStreamHoldback } from "./lib/tui/stream.mjs";
import { createSerialMutex, createTtlCache, isTokenExpiring, orderLabelsLastGoodFirst } from "./lib/spawn-auth.mjs";
import { appendOperatorPrompt } from "./lib/prompt.mjs";
const __dirname = dirname(fileURLToPath(import.meta.url));
const _pkg = JSON.parse(readFileSync(join(__dirname, "package.json"), "utf8"));
@@ -195,17 +196,19 @@ function resolveClaude() {
const OCP_SYSTEM_PROMPT_WRAPPER = `You are accessed via the OCP HTTP proxy. You do NOT have access to any local filesystem, working directory, shell, git status, or machine environment. Do not infer or invent such information from any context you observe. Respond only based on the conversation provided.`;
// Build the full system-prompt string: OCP_SYSTEM_PROMPT_WRAPPER prepended,
// then any system-role messages from the request appended (separated by blank line).
// ADR 0009 Amendment 1 analogue § "OLP system prompt wrapper".
// then any system-role messages from the request appended (separated by blank line),
// then the operator-wide CLAUDE_SYSTEM_PROMPT appended LAST (lib/prompt.mjs — a
// no-op returning the same string when the var is unset, so the default path is
// byte-for-byte unchanged). ADR 0009 Amendment 1 analogue § "OLP system prompt wrapper".
function extractSystemPrompt(messages) {
const systemMessages = (messages ?? []).filter(m => m.role === "system");
if (systemMessages.length === 0) {
return OCP_SYSTEM_PROMPT_WRAPPER;
return appendOperatorPrompt(OCP_SYSTEM_PROMPT_WRAPPER, SYSTEM_PROMPT);
}
const clientContent = systemMessages.map(m =>
contentToText(m.content)
).join("\n\n");
return `${OCP_SYSTEM_PROMPT_WRAPPER}\n\n${clientContent}`;
return appendOperatorPrompt(`${OCP_SYSTEM_PROMPT_WRAPPER}\n\n${clientContent}`, SYSTEM_PROMPT);
}
// ── NDJSON line buffer parser (Phase 6c port) ─────────────────────────────
+51 -1
View File
@@ -821,11 +821,61 @@ test("doctor falls back to currentVersion when origin/main unreachable (no stale
assert.equal(result.next_action.kind, "noop");
});
// ── System-prompt operator append (CLAUDE_SYSTEM_PROMPT wiring) ─────────────
// The var was documented + echoed on /health but never reached a request (dead
// since APPEND_SYSTEM_PROMPT was retired — caught in PR #170 review). The wiring
// contract lives in lib/prompt.mjs. Mutation-proof: make appendOperatorPrompt
// return `base` unconditionally and the first test fails; make it stop trimming
// and the whitespace test fails.
import { appendOperatorPrompt } from "./lib/prompt.mjs";
console.log("\nSystem-prompt operator append:");
test("appendOperatorPrompt: appends the operator prompt LAST, blank-line separated", () => {
assert.equal(appendOperatorPrompt("WRAPPER\n\nclient", "Answer in Chinese."), "WRAPPER\n\nclient\n\nAnswer in Chinese.");
});
test("appendOperatorPrompt: unset/empty/whitespace-only → base returned BYTE-IDENTICAL", () => {
const base = "WRAPPER\n\nclient sys";
assert.equal(appendOperatorPrompt(base, undefined), base);
assert.equal(appendOperatorPrompt(base, ""), base);
assert.equal(appendOperatorPrompt(base, " \n "), base, "a stray space in a service unit must not inject anything");
assert.equal(appendOperatorPrompt(base, null), base);
});
test("appendOperatorPrompt: operator value is trimmed before appending", () => {
assert.equal(appendOperatorPrompt("W", " hi "), "W\n\nhi");
});
// ── Upgrade Tests ──
import { runUpgrade } from "./scripts/upgrade.mjs";
import { runUpgrade, postFlightOk } from "./scripts/upgrade.mjs";
console.log("\nUpgrade:");
// ── postFlightOk (issue #173) — the acceptance predicate for phase 6 ─────────
// Mutation-proof: revert the version comparison to auth-only and the "stale process
// still holds the port" test below goes green-to-red (that case is the 2026-07-17
// Oracle incident: orphan answered auth.ok=true while serving the OLD version).
test("postFlightOk: rejects a healthy-looking probe that serves the WRONG version (orphan case)", () => {
assert.equal(postFlightOk({ auth: { ok: true }, version: "3.21.1" }, "v3.22.1"), false);
});
test("postFlightOk: accepts auth.ok + exact target version, tolerating the leading v", () => {
assert.equal(postFlightOk({ auth: { ok: true }, version: "3.22.1" }, "v3.22.1"), true);
assert.equal(postFlightOk({ auth: { ok: true }, version: "3.22.1" }, "3.22.1"), true);
});
test("postFlightOk: auth failure rejects regardless of version", () => {
assert.equal(postFlightOk({ auth: { ok: false }, version: "3.22.1" }, "v3.22.1"), false);
assert.equal(postFlightOk({ version: "3.22.1" }, "v3.22.1"), false);
assert.equal(postFlightOk(null, "v3.22.1"), false);
});
test("postFlightOk: unknown/empty target degrades to the auth-only check (never blocks)", () => {
assert.equal(postFlightOk({ auth: { ok: true }, version: "3.22.1" }, ""), true);
assert.equal(postFlightOk({ auth: { ok: true }, version: "3.22.1" }, undefined), true);
});
test("upgrade --dry-run prints plan, no side effects", async () => {
const result = await runUpgrade({
dryRun: true,