Skip to content

refactor(agents): keep ThinkHarness records in the shared harness store - #2540

Merged
mattzcarey merged 2 commits into
mainfrom
chore/think-harness-shared-store
Oct 9, 2026
Merged

mattzcarey merged 2 commits into
mainfrom
chore/think-harness-shared-store

Conversation

@mattzcarey

@mattzcarey mattzcarey commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

When Cloudflare refuses a call with 401 or 403, execute and 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 a 403 insufficient_scope challenge naming user:read account:read plus that scope, so clients that support step-up re-authorize and retry. ?scopeChallenge=tool keeps 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 error for 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).

  • The accepted permissions come from the upstream spec's x-api-token-group. The scheduled handler now keeps it in spec.json and copies it into each mcp-tools.json entry as permissions. OAuth scopes share those names.
  • execute matches the refused path against the cached mcp-tools.json (4 MB), not spec.json. On main spec.json is ~25 MB, and parsing it in the Worker on a refusal risks the memory limit.
  • The challenge fires only after Cloudflare refuses, never before. For execute it 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.
  • For OAuth connections, 2025-era POSTs are served as JSON instead of SSE, so the response status is still open when a tool asks for a challenge.

npm run check ran. 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.

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-bot

changeset-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0ae3d5e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
agents Patch
@cloudflare/agent-think Patch

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Devin Review

Comment on lines +649 to +655
// Enqueued in the legacy order, so the store's order is the same.
store.enqueue({
session,
id,
input: parseJsonColumn(row.input),
meta: metaJson(meta)
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Comment on lines +463 to +464
const call = this.#state(session).toolCalls[toolCallId];
return call && { session, toolCallId, ...call };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Comment on lines +649 to +655
// Enqueued in the legacy order, so the store's order is the same.
store.enqueue({
session,
id,
input: parseJsonColumn(row.input),
meta: metaJson(meta)
});

@devin-ai-integration devin-ai-integration Bot Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Legacy operation timestamps change on upgrade

Migration re-enqueues old operations, giving them new creation times; settled operations also receive new settlement times. Review whether consumers rely on those historical timestamps.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@agent-think

agent-think Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

🟡 agents import sizes: 1 entry point grew

Entry point Exports Largest gzip change Size now
🟡 agents/harness/think 1 resized +2.2 KiB (+1.2%) 182.1 KiB
Changed exports (1)
Import Gzip change Size now
🟡 agents/harness/think#ThinkHarness +2.2 KiB (+1.2%) 182.1 KiB
How this works

Each 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 66149553 → 0ae3d5e9 · workflow run · reported by agent-think[bot]

@pkg-pr-new

pkg-pr-new Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

agents

npm i https://pkg.pr.new/agents@2540

@cloudflare/ai-chat

npm i https://pkg.pr.new/@cloudflare/ai-chat@2540

@cloudflare/codemode

npm i https://pkg.pr.new/@cloudflare/codemode@2540

hono-agents

npm i https://pkg.pr.new/hono-agents@2540

@cloudflare/shell

npm i https://pkg.pr.new/@cloudflare/shell@2540

@cloudflare/think

npm i https://pkg.pr.new/@cloudflare/think@2540

@cloudflare/voice

npm i https://pkg.pr.new/@cloudflare/voice@2540

@cloudflare/worker-bundler

npm i https://pkg.pr.new/@cloudflare/worker-bundler@2540

commit: 0ae3d5e

…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.
@mattzcarey
mattzcarey merged commit b65956e into main Oct 9, 2026
18 of 19 checks passed
@mattzcarey
mattzcarey deleted the chore/think-harness-shared-store branch October 9, 2026 01:52
@github-actions github-actions Bot mentioned this pull request Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant