Repository navigation
refactor(agents): keep ThinkHarness records in the shared harness store - #2540
Conversation
ThinkHarness now keeps its sessions, operations and started tool calls in agents/harness/store under the cf_think_harness_store_ prefix. On first start it moves rows from its old cf_think_harness_* tables into the store in one transaction and drops the old tables. The store gains createdAt on createSession, for imported sessions, and deleteSettled(session), for a reset.
🦋 Changeset detectedLatest commit: 0ae3d5e The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // Enqueued in the legacy order, so the store's order is the same. | ||
| store.enqueue({ | ||
| session, | ||
| id, | ||
| input: parseJsonColumn(row.input), | ||
| meta: metaJson(meta) | ||
| }); |
There was a problem hiding this comment.
🔴 Rollback operations lost during migration
After a rollback, store.enqueue ignores legacy operations whose ids remain in the shared store. Migration drops those legacy rows, so reused ids return stale results and new work disappears.
Learn more
A rollback can leave both table sets populated. The older version creates its legacy tables again and can reuse an id after a reset, while the shared store retains that id from the earlier deployment. enqueue returns the existing shared record without importing the new legacy record. Migration then drops the legacy tables. This permanently loses that operation, including its input and progress.
Example: Version N stores a completed operation s/op; a rollback resets session s and queues a new s/op in legacy tables. On upgrading to N again, migration keeps the old completed s/op, drops the new queued row, and wait('op') returns the old answer.
Recommended fix: Detect overlapping records before dropping legacy tables. Define a reconciliation policy for collisions, including existing session state and ordering; do not discard a legacy operation unless its record has been correctly preserved.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 0ae3d5e. Legacy tables can only sit beside the store after a rollback, so a legacy row is the newer record of its id and now replaces the store's on import (new HarnessStore.deleteOperation). Covered by "keeps work a rolled-back version queued under an id the store already holds".
| const call = this.#state(session).toolCalls[toolCallId]; | ||
| return call && { session, toolCallId, ...call }; |
There was a problem hiding this comment.
🟡 Reserved tool-call ids skip execution
When a model supplies tool-call id constructor or toString, toolCall reads an inherited object property as a started call. The tool is reported interrupted without running.
Learn more
The session state stores started calls in a plain JavaScript object, and parseSessionState reconstructs it with {}. A lookup for a property inherited from Object.prototype returns a value even when no call was started. runTools uses this value as an earlier call and skips tools whose recovery policy is report.
Example: A model produces a valid server tool call with id toString. No corresponding call has run, but toolCall returns the inherited function and runTools records an interrupted-tool error instead of executing the tool.
Recommended fix: Check Object.hasOwn before looking up a tool-call id, and use a null-prototype dictionary or Map while parsing and updating state so arbitrary ids retain their exact meaning.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 0ae3d5e: lookups use Object.hasOwn over a null-prototype record. Covered by "runs a tool call whose id names an Object.prototype property".
| // Enqueued in the legacy order, so the store's order is the same. | ||
| store.enqueue({ | ||
| session, | ||
| id, | ||
| input: parseJsonColumn(row.input), | ||
| meta: metaJson(meta) | ||
| }); |
There was a problem hiding this comment.
There was a problem hiding this comment.
Intended. An operation's creation and settle times aren't exposed by ThinkHarness, and only relative order is used, which the import keeps. Session creation times are kept, since sessions.list() is ordered by them.
🟡 agents import sizes: 1 entry point grew
Changed exports (1)
How this worksEach runtime export is bundled on its own, minified, and gzipped. Changes smaller than 100 B, or smaller than 1% and 1 KiB, are ignored. Growth over 10% or 5 KiB is marked 🔴. This report is informational and does not fail CI. The workflow artifact contains every measurement. Compared |
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
…ool calls A legacy row beside the shared store can only come from a rollback, so it is the newer record of its id and replaces the store's on import. The store gains deleteOperation(session, id) for that. Started tool calls are looked up with Object.hasOwn in a null-prototype record, so an id like `constructor` is not read as a started call.
When Cloudflare refuses a call with 401 or 403,
executeand the endpoint tools now say why: the connection lacks a scope the endpoint accepts, it already holds one (so it's a role or account problem), or no OAuth scope covers the endpoint (use an API token). If an OAuth connection lacks the scope and retrying is safe, the server answers with a403 insufficient_scopechallenge naminguser:read account:readplus that scope, so clients that support step-up re-authorize and retry.?scopeChallenge=toolkeeps the challenge in the tool result's_meta["mcp/www_authenticate"]for ChatGPT. Combines #267 and #268, ported from the Forge stack onto main.Cloudflare returns the same
10000: Authentication errorfor a missing scope, an endpoint OAuth can't reach, and a missing role. Agents read it as a signed-out connection and keep asking users to reconnect, which changes nothing (#199, #202).x-api-token-group. The scheduled handler now keeps it inspec.jsonand copies it into eachmcp-tools.jsonentry aspermissions. OAuth scopes share those names.executematches the refused path against the cachedmcp-tools.json(4 MB), notspec.json. On mainspec.jsonis ~25 MB, and parsing it in the Worker on a refusal risks the memory limit.executeit also requires that every request the code sent was a GET/HEAD or refused. The sandbox reports the scope, so the host accepts only a catalog scope the connection lacks.npm run checkran. Seed R2 (npm run seed:staging,seed:prod) before deploying. The old code ignores the new field, and without it every 403 says the spec lists no permissions.