mirror of
https://github.com/dtzp555-max/ocp.git
synced 2026-07-26 23:45:08 +00:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4759639d9b | ||
|
|
868a1953f4 |
@@ -30,7 +30,6 @@ Runtime: Node.js (ESM, `.mjs` throughout). No build step. No bundler. `server.mj
|
||||
|
||||
- `server.mjs` — the proxy itself; every request path lives here. Governed by `ALIGNMENT.md`.
|
||||
- `models.json` — single source of truth for model IDs, aliases, and context windows. See ADR 0003.
|
||||
- `models.schema.json` — the schema `models.json` declares in its `$schema`. CI validates the SPOT against it (`test-features.mjs`) using the repo's own `validateJsonSchema`, so a malformed entry fails the build instead of surfacing downstream in OpenClaw.
|
||||
- `setup.mjs` — first-time installer; reads `models.json` to derive bootstrap config.
|
||||
- `scripts/sync-openclaw.mjs` — idempotent OpenClaw registry sync invoked by `ocp update`. See ADR 0004.
|
||||
- `ocp` — user-facing CLI (install, update, start, stop, status, logs, etc.).
|
||||
@@ -53,6 +52,22 @@ Runtime: Node.js (ESM, `.mjs` throughout). No build step. No bundler. `server.mj
|
||||
|
||||
---
|
||||
|
||||
## Testing: reaching faults inside `server.mjs`
|
||||
|
||||
`test-features.mjs` cannot `import` `server.mjs` (it calls `server.listen()` at top level), and that has twice led to the wrong conclusion that a class of bug is untestable. It isn't. Read this before writing "no regression test is possible here".
|
||||
|
||||
**There is a real live-server fixture.** `ltBoot(env, dir, nodeArgs)` (around `test-features.mjs:990`) spawns the actual `server.mjs` as a child with a **fake `claude` binary**, so integration tests cost no quota. `ltPost` / `ltPostStatus` / `ltWait` / `ltFreePort` round it out. It already covers boot gates, cache-epoch invalidation across two boots sharing one SQLite store, and system-prompt capture.
|
||||
|
||||
**`--stack-size` is a fault lever.** `ltBoot`'s third argument passes V8 flags to the child, which puts recursion- and argument-count-limited failures in reach at a much smaller input. `#193` needed a *synchronous throw* deep inside `spawnClaudeProcess`; `buildCliArgs` does `args.push("--allowedTools", ...ALLOWED_TOOLS)`, and under `--stack-size=200` that spread throws at ~24k elements instead of ~124k — which is what brings the trigger under Linux's `MAX_ARG_STRLEN` (131072 bytes for a single env string) so the test runs in CI rather than skipping. **No production fault hook was needed.**
|
||||
|
||||
Three rules that made it hold up, all learned the hard way:
|
||||
|
||||
- **Discover the threshold in a child under the same flags**, never in the test process — the parent's stack is not the one that matters.
|
||||
- **Assert that the fault actually fired**, not just that the outcome looks right. `#193` asserts HTTP 500 *and* that the body carries `call stack size exceeded`; a control mutation (trigger neutered, bug still present) proves the test fails rather than passing vacuously.
|
||||
- **Wait for the thing you are about to assert**, not a proxy for it. Waiting on `listening on` and then asserting a different line is a race (`#199`); waiting for the process to *exit* and then reading its `stderr` is another, because a terminated child's pipes may still hold unread data (`#203` — wait for the stdio to close, not for the exit).
|
||||
|
||||
Allocate ports with `ltFreePort()`. Fixed ports have caused at least one flake here.
|
||||
|
||||
## Release protocol
|
||||
|
||||
OCP follows the machine-readable `release_kit:` overlay in `CLAUDE.md` (Iron Rule 5.5). Before any version bump or tag push, re-read that YAML block and walk every item in `new_feature_doc_expectations` and `bootstrap_quirk_policy`. Tag push triggers `.github/workflows/release.yml`, which creates the GitHub Release automatically — do not create the release manually.
|
||||
|
||||
@@ -177,7 +177,7 @@ Any tool use happens server-side, under the `--allowedTools` set configured on t
|
||||
| `claude-sonnet-4-6` | Previous Sonnet, retained for pinning |
|
||||
| `claude-haiku-4-5-20251001` | Fastest, lightweight (default for `haiku` alias) |
|
||||
|
||||
The canonical list lives in [`models.json`](./models.json) — the single source of truth as of v3.11.0, validated in CI against [`models.schema.json`](./models.schema.json). Both `server.mjs` (the `/v1/models` endpoint) and `setup.mjs` (the OpenClaw registration) derive from it. Adding a new model is now a one-file edit:
|
||||
The canonical list lives in [`models.json`](./models.json) — the single source of truth as of v3.11.0. Both `server.mjs` (the `/v1/models` endpoint) and `setup.mjs` (the OpenClaw registration) derive from it. Adding a new model is now a one-file edit:
|
||||
|
||||
```bash
|
||||
# 1. Edit models.json — add an entry
|
||||
|
||||
@@ -1,65 +0,0 @@
|
||||
{
|
||||
"$schema": "https://json-schema.org/draft/2020-12/schema",
|
||||
"$id": "https://github.com/dtzp555-max/ocp/blob/main/models.schema.json",
|
||||
"title": "OCP models.json",
|
||||
"description": "Schema for models.json, the single source of truth for model metadata (ADR 0003). Enforced in CI by test-features.mjs, which validates models.json against this file using the repo's own validateJsonSchema (lib/structured-output.mjs) — no new dependency. ⚠ ONLY A SUBSET OF DRAFT 2020-12 IS ENFORCED: type, required, const, enum, additionalProperties (boolean and schema forms), items, minItems, maxItems, anyOf/allOf/oneOf, $ref, nullable. Anything else — minimum, maxLength, pattern, uniqueItems, propertyNames, not — is SILENTLY IGNORED by that validator, so adding one here buys nothing and misleads the next reader. Add such a constraint as a test in test-features.mjs instead. NOTE: referential integrity (every aliases/legacyAliases target must exist as a models[].id) is likewise not expressible here and is covered by separate tests.",
|
||||
"type": "object",
|
||||
"required": ["version", "models", "aliases", "legacyAliases"],
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
"$schema": {
|
||||
"type": "string",
|
||||
"description": "Relative path to this file. Editors use it for completion; CI does not read it."
|
||||
},
|
||||
"version": {
|
||||
"const": 1,
|
||||
"description": "Schema version of this document. Bump only alongside a shape change and an ADR."
|
||||
},
|
||||
"models": {
|
||||
"type": "array",
|
||||
"minItems": 1,
|
||||
"description": "Every model OCP advertises on /v1/models, newest first. Order is load-bearing: ocp-connect uses model_ids[0] as a primary fallback.",
|
||||
"items": {
|
||||
"type": "object",
|
||||
"required": ["id", "displayName", "openclawName", "reasoning", "contextWindow", "maxTokens"],
|
||||
"additionalProperties": false,
|
||||
"properties": {
|
||||
"id": {
|
||||
"type": "string",
|
||||
"description": "Canonical CLI model id, passed verbatim to `claude --model`. Must match the id in the compiled CLI registry."
|
||||
},
|
||||
"displayName": {
|
||||
"type": "string",
|
||||
"description": "Human label. Also used as the OpenClaw agent alias by scripts/sync-openclaw.mjs."
|
||||
},
|
||||
"openclawName": {
|
||||
"type": "string",
|
||||
"description": "Label written into OpenClaw's model registry."
|
||||
},
|
||||
"reasoning": {
|
||||
"type": "boolean",
|
||||
"description": "Whether OpenClaw should treat the model as reasoning-capable."
|
||||
},
|
||||
"contextWindow": {
|
||||
"type": "integer",
|
||||
"description": "Advertised context window. GLOBAL side effect: MAX_PROMPT_CHARS derives from max(contextWindow) x 3 across ALL entries (lib/prompt.mjs derivePromptCharBudget), so raising this on ONE model raises the truncation ceiling for EVERY model. See ADR 0009. Also feeds OpenClaw's compaction budget."
|
||||
},
|
||||
"maxTokens": {
|
||||
"type": "integer",
|
||||
"description": "Advertised output cap. OCP does not enforce it; it is propagated to OpenClaw (via setup.mjs / scripts/sync-openclaw.mjs) where it bounds request and compaction budgets."
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
"aliases": {
|
||||
"type": "object",
|
||||
"description": "Short name -> canonical models[].id. Client-addressable, so a repoint changes routing for every request using the alias. Cache keys resolve these before hashing (server.mjs cacheModel).",
|
||||
"additionalProperties": { "type": "string" }
|
||||
},
|
||||
"legacyAliases": {
|
||||
"type": "object",
|
||||
"description": "Retired ids kept resolvable for backward compatibility -> canonical models[].id. Also client-addressable and also resolved in cache keys.",
|
||||
"additionalProperties": { "type": "string" }
|
||||
}
|
||||
}
|
||||
}
|
||||
+12
@@ -2923,6 +2923,16 @@ async function handleChatCompletions(req, res) {
|
||||
const t0s = Date.now();
|
||||
const promptCharsS = messages.reduce((a, m) => a + contentToText(m.content).length, 0);
|
||||
let structuredHash = null;
|
||||
// DO NOT collapse this with `dedupKey` below (#200). The two cacheHash calls take IDENTICAL
|
||||
// arguments and look like obvious duplicate work — they are not interchangeable, because
|
||||
// their GUARDS differ: this one additionally requires CACHE_TTL > 0. CLAUDE_CACHE_TTL
|
||||
// DEFAULTS TO 0, so in the default configuration structuredHash is null while dedupKey must
|
||||
// still be computed — it drives #153's single-flight stampede protection, which is
|
||||
// deliberately independent of whether response caching is on. `dedupKey = structuredHash`
|
||||
// would therefore silently disable stampede protection by default, in exactly the
|
||||
// concurrent-AI-Task case it exists to bound. The duplicate call is the honest price of the
|
||||
// asymmetry. If you do deduplicate it, compute once under the WEAKER guard and derive the
|
||||
// cache lookup under the stronger one — and add a stampede test before you do.
|
||||
if (CACHE_TTL > 0 && !conversationId && !hasCacheControl(messages)) {
|
||||
structuredHash = cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured, configEpoch: CONFIG_EPOCH });
|
||||
try {
|
||||
@@ -2942,6 +2952,8 @@ async function handleChatCompletions(req, res) {
|
||||
// firing several AI Tasks at once) must NOT each pay N× — they share one flight. We dedup every
|
||||
// one-off structured request (not stateful sessions / client-side prompt caching), independent of
|
||||
// whether OCP response caching is enabled; when caching IS on, the same key gates cache read/write.
|
||||
// Note the guard here is deliberately WEAKER than structuredHash's — no CACHE_TTL check. See the
|
||||
// do-not-collapse comment above (#200).
|
||||
const dedupKey = (!conversationId && !hasCacheControl(messages))
|
||||
? cacheHash(cacheModel, messages, { keyId: req._authKeyId, temperature: parsed.temperature, max_tokens: parsed.max_tokens, top_p: parsed.top_p, structured, configEpoch: CONFIG_EPOCH })
|
||||
: null;
|
||||
|
||||
@@ -4305,67 +4305,6 @@ test("models.json: every legacyAliases value resolves to a real models[].id (ref
|
||||
}
|
||||
});
|
||||
|
||||
// ── models.json validates against models.schema.json (#196) ─────────────────
|
||||
// models.json carried `"$schema": "./models.schema.json"` while that file had never been
|
||||
// committed, so the SPOT that ADR 0003 makes canonical had no structural validation at all —
|
||||
// a missing contextWindow or a typo'd openclawName would only surface downstream, in OpenClaw
|
||||
// or in a truncation budget. Validated with the repo's OWN validator (lib/structured-output.mjs,
|
||||
// shipped for #153) rather than a new dependency: zero deps added, and it exercises that
|
||||
// validator on a second real input.
|
||||
//
|
||||
// The schema deliberately does NOT try to express referential integrity (alias -> models[].id);
|
||||
// that is not a JSON Schema concept and is covered by the two tests directly above.
|
||||
import { validateJsonSchema as _spotValidate } from "./lib/structured-output.mjs";
|
||||
|
||||
const _spotSchema = JSON.parse(spotReadFileSync(spotJoin(_spotDir, "models.schema.json"), "utf8"));
|
||||
|
||||
test("models.json: the $schema reference resolves to a committed file", () => {
|
||||
assert.equal(_spotModels.$schema, "./models.schema.json", "models.json must point at the schema");
|
||||
assert.ok(_ltExists(spotJoin(_spotDir, "models.schema.json")),
|
||||
"models.schema.json must exist — a dangling $schema is what #196 was filed for");
|
||||
});
|
||||
|
||||
test("models.json validates against models.schema.json (strict)", () => {
|
||||
const errors = _spotValidate(_spotModels, _spotSchema, "$", true);
|
||||
assert.deepEqual(errors, [], `models.json violates its own schema:\n ${errors.join("\n ")}`);
|
||||
});
|
||||
|
||||
// Three corruptions the SCHEMA structurally cannot catch, asserted directly instead of pretending
|
||||
// the schema covers them: the validator has no uniqueItems, no minLength, and no minimum. Adding
|
||||
// those keywords to the schema would be silently ignored (see its description), so they live here.
|
||||
test("models.json: ids are unique, non-empty, and windows are positive (not schema-expressible)", () => {
|
||||
const ids = _spotModels.models.map(m => m.id);
|
||||
assert.equal(new Set(ids).size, ids.length, `duplicate models[].id: ${ids.filter((v, i) => ids.indexOf(v) !== i)}`);
|
||||
for (const m of _spotModels.models) {
|
||||
for (const f of ["id", "displayName", "openclawName"]) {
|
||||
assert.ok(typeof m[f] === "string" && m[f].trim().length > 0, `${m.id}: ${f} must be a non-empty string`);
|
||||
}
|
||||
assert.ok(m.contextWindow > 0, `${m.id}: contextWindow must be positive`);
|
||||
assert.ok(m.maxTokens > 0, `${m.id}: maxTokens must be positive`);
|
||||
}
|
||||
});
|
||||
|
||||
// Guards the guard: if the schema were vacuous (e.g. `{}` or a typo'd `properties`), the test
|
||||
// above would pass on anything. Each corruption below must be caught.
|
||||
test("models.schema.json actually rejects malformed entries (guard is not vacuous)", () => {
|
||||
const clone = () => JSON.parse(JSON.stringify(_spotModels));
|
||||
const cases = [
|
||||
["missing required field", m => { delete m.models[0].contextWindow; }],
|
||||
["wrong scalar type", m => { m.models[0].reasoning = "yes"; }],
|
||||
["non-integer window", m => { m.models[0].contextWindow = 200000.5; }],
|
||||
["unknown extra field", m => { m.models[0].tokensPerSecond = 42; }],
|
||||
["alias mapped to non-string", m => { m.aliases.opus = { id: "x" }; }],
|
||||
["empty models array", m => { m.models = []; }],
|
||||
["wrong document version", m => { m.version = 2; }],
|
||||
["unknown top-level key", m => { m.providers = {}; }],
|
||||
];
|
||||
for (const [label, corrupt] of cases) {
|
||||
const bad = clone(); corrupt(bad);
|
||||
const errs = _spotValidate(bad, _spotSchema, "$", true);
|
||||
assert.ok(errs.length > 0, `schema failed to reject: ${label}`);
|
||||
}
|
||||
});
|
||||
|
||||
// ── escapeHtml + key-name validator (issue #114) ────────────────────────────
|
||||
// Replicated verbatim from dashboard.html so tests run without a browser.
|
||||
function escapeHtml(s) {
|
||||
|
||||
Reference in New Issue
Block a user