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 |
@@ -52,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.
|
||||
|
||||
@@ -1,11 +1,5 @@
|
||||
# Changelog
|
||||
|
||||
## Unreleased
|
||||
|
||||
### Changed
|
||||
|
||||
- **`maxTokens` now matches the CLI registry instead of a uniform 16384 (#195).** Every Opus entry and `claude-sonnet-5` go to **64000**, `claude-sonnet-4-6` and `claude-haiku-4-5` to **32000** — the `max_output_tokens.default` each model actually declares in the compiled CLI 2.1.220 registry. OCP never enforced this value; it is propagated to OpenClaw, where it **caps the outbound request**: `maxTokens = Math.min(baseMaxTokens + thinkingBudget, modelMaxTokens)`. OpenClaw's reasoning budgets are medium 8192 / high 16384, and when `maxTokens <= thinkingBudget` it clamps thinking to `maxTokens - 1024` — so a model declared at 16384 running at **high** reasoning spent ~15360 on thinking and left **~1024 tokens for the actual answer**. `ocp-connect`'s independent family table moves to the family floor (opus 64000, sonnet 32000, haiku 32000) since prefixes cannot distinguish versions. Expect longer answers and correspondingly higher per-request quota burn on long generations.
|
||||
|
||||
## v3.25.0 — 2026-07-27
|
||||
|
||||
Minor release. Headline: **Claude Opus 5** joins the model list and the `opus` alias now resolves to it. Alongside it, two `server.mjs` correctness fixes found by review rather than by users — a monotonic in-flight-counter leak, and cache keys that were hashing the alias string instead of the model it resolves to. The three TUI/prompt fixes that landed after v3.24.0 are included here too.
|
||||
|
||||
+7
-7
@@ -8,7 +8,7 @@
|
||||
"openclawName": "Claude Opus 5 (via CLI)",
|
||||
"reasoning": true,
|
||||
"contextWindow": 200000,
|
||||
"maxTokens": 64000
|
||||
"maxTokens": 16384
|
||||
},
|
||||
{
|
||||
"id": "claude-opus-4-8",
|
||||
@@ -16,7 +16,7 @@
|
||||
"openclawName": "Claude Opus 4.8 (via CLI)",
|
||||
"reasoning": true,
|
||||
"contextWindow": 200000,
|
||||
"maxTokens": 64000
|
||||
"maxTokens": 16384
|
||||
},
|
||||
{
|
||||
"id": "claude-opus-4-7",
|
||||
@@ -24,7 +24,7 @@
|
||||
"openclawName": "Claude Opus 4.7 (via CLI)",
|
||||
"reasoning": true,
|
||||
"contextWindow": 200000,
|
||||
"maxTokens": 64000
|
||||
"maxTokens": 16384
|
||||
},
|
||||
{
|
||||
"id": "claude-opus-4-6",
|
||||
@@ -32,7 +32,7 @@
|
||||
"openclawName": "Claude Opus 4.6 (via CLI)",
|
||||
"reasoning": true,
|
||||
"contextWindow": 200000,
|
||||
"maxTokens": 64000
|
||||
"maxTokens": 16384
|
||||
},
|
||||
{
|
||||
"id": "claude-sonnet-5",
|
||||
@@ -40,7 +40,7 @@
|
||||
"openclawName": "Claude Sonnet 5 (via CLI)",
|
||||
"reasoning": true,
|
||||
"contextWindow": 200000,
|
||||
"maxTokens": 64000
|
||||
"maxTokens": 16384
|
||||
},
|
||||
{
|
||||
"id": "claude-sonnet-4-6",
|
||||
@@ -48,7 +48,7 @@
|
||||
"openclawName": "Claude Sonnet 4.6 (via CLI)",
|
||||
"reasoning": true,
|
||||
"contextWindow": 200000,
|
||||
"maxTokens": 32000
|
||||
"maxTokens": 16384
|
||||
},
|
||||
{
|
||||
"id": "claude-haiku-4-5-20251001",
|
||||
@@ -56,7 +56,7 @@
|
||||
"openclawName": "Claude Haiku 4.5 (via CLI)",
|
||||
"reasoning": false,
|
||||
"contextWindow": 200000,
|
||||
"maxTokens": 32000
|
||||
"maxTokens": 8192
|
||||
}
|
||||
],
|
||||
"aliases": {
|
||||
|
||||
+3
-10
@@ -129,17 +129,10 @@ provider = {
|
||||
# re-trip it. Family prefixes classify any versioned ID correctly with no per-model
|
||||
# edit. (ADR 0003: models.json is the SPOT for model existence; /v1/models does not
|
||||
# expose reasoning/maxTokens, so family classification stays here.)
|
||||
# maxTokens values are the FAMILY FLOOR of the CLI registry max_output_tokens.default
|
||||
# (opus family 64000; sonnet 32000 because sonnet-4-6 is 32000 while sonnet-5 is 64000;
|
||||
# haiku 32000). Family prefixes cannot distinguish versions, so the floor is used
|
||||
# deliberately: under-advertising caps a client lower than the model allows, which is safe,
|
||||
# whereas over-advertising would promise capacity a member of the family does not have.
|
||||
# models.json is authoritative per ADR 0003; this table only exists because /v1/models does
|
||||
# not expose maxTokens.
|
||||
model_meta = {
|
||||
"claude-opus": {"name": "Claude Opus (OCP)", "reasoning": True, "maxTokens": 64000},
|
||||
"claude-sonnet": {"name": "Claude Sonnet (OCP)", "reasoning": True, "maxTokens": 32000},
|
||||
"claude-haiku": {"name": "Claude Haiku (OCP)", "reasoning": False, "maxTokens": 32000},
|
||||
"claude-opus": {"name": "Claude Opus (OCP)", "reasoning": True, "maxTokens": 16384},
|
||||
"claude-sonnet": {"name": "Claude Sonnet (OCP)", "reasoning": True, "maxTokens": 16384},
|
||||
"claude-haiku": {"name": "Claude Haiku (OCP)", "reasoning": False, "maxTokens": 8192},
|
||||
}
|
||||
|
||||
def get_model_meta(mid):
|
||||
|
||||
+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;
|
||||
|
||||
@@ -4299,24 +4299,6 @@ test("models.json: every aliases value resolves to a real models[].id (referenti
|
||||
}
|
||||
});
|
||||
|
||||
// maxTokens must leave room for a visible answer once a client's thinking budget is taken out
|
||||
// (#195). OCP never enforces maxTokens itself — it is propagated to OpenClaw (setup.mjs,
|
||||
// scripts/sync-openclaw.mjs) where it CAPS the outbound request:
|
||||
// maxTokens = Math.min(baseMaxTokens + thinkingBudget, modelMaxTokens)
|
||||
// OpenClaw's reasoning budgets are medium 8192 / high 16384, and when maxTokens <= thinkingBudget
|
||||
// it clamps thinking to maxTokens - 1024. So a model declared at 16384 running at `high` spent
|
||||
// ~15360 on thinking and left ~1024 for the actual answer — which is what this repo shipped until
|
||||
// v3.25.0. Asserting the PRINCIPLE rather than pinning per-model numbers: a value test would need
|
||||
// editing every time a model is added, and would not say why.
|
||||
const _spotHighThinkingBudget = 16384; // OpenClaw's `high` reasoning budget
|
||||
test("models.json: every maxTokens exceeds the high thinking budget, leaving room for output (#195)", () => {
|
||||
for (const m of _spotModels.models) {
|
||||
assert.ok(m.maxTokens > _spotHighThinkingBudget,
|
||||
`${m.id} declares maxTokens=${m.maxTokens} <= ${_spotHighThinkingBudget}: at OpenClaw's 'high' reasoning ` +
|
||||
`level the thinking budget would consume all but ~1024 tokens of the response`);
|
||||
}
|
||||
});
|
||||
|
||||
test("models.json: every legacyAliases value resolves to a real models[].id (referential integrity)", () => {
|
||||
for (const [name, target] of Object.entries(_spotModels.legacyAliases || {})) {
|
||||
assert.ok(_spotModelIds.has(target), `legacyAliases.${name} -> '${target}' is a dangling alias (no matching models[].id)`);
|
||||
|
||||
Reference in New Issue
Block a user