Skip to content

feat(amsg): client_state 大值透明分块 + fire 级 scratch + /capabilities 特性探测#26

Merged
Tosd0 merged 10 commits into
mainfrom
feat/amsg-universal-gaps
Jul 18, 2026
Merged

feat(amsg): client_state 大值透明分块 + fire 级 scratch + /capabilities 特性探测#26
Tosd0 merged 10 commits into
mainfrom
feat/amsg-universal-gaps

Conversation

@Tosd0

@Tosd0 Tosd0 commented Jul 18, 2026

Copy link
Copy Markdown
Owner

实机验证暴露的三个通用缺口补齐(任务 4 溢出封套经评估本批暂缓,评估见 plan 文末)。

改了什么 / 改后怎样

① client_state 大值透明分块 + 整批局部失败(amsg-server)

之前 之后
单条 value > 200KB 整批 413,宿主要在应用层自己发明分块格式 worker 切片跨行存储,GET / readState() 返回拼好的原值,客户端和 hook 作者无感;总上限默认 5MB(maxStateValueBytes 可调)
批里有坏条目 整批拒绝 只拒它自己,响应 data.rejected 逐条给原因;全成功时响应形状不变(老客户端无感)

分块是服务端实现细节:切片在码点边界(中文 / emoji 不劈开)、缩块覆盖写不残留尾块、块不齐全时该 key 视为不存在(读方走兜底、不吐半截数据)、LWW 语义对分块路径同样成立。≤ 200KB 的值走原单行路径,存储字节级不变。

② fire 级 scratch(amsg-shared + amsg-server)

onBeforeFire 的 fireCtx 和同一次 fire 每轮 onLLMOutput / executeToolCalls 的 sessionCtx 拿到同一个 scratch 对象引用,fire 结束(finish / skip-push / 抛错 / 轮数超限)即丢弃。宿主不用再自己维护 Map<sessionId, state> + 孤儿清理。不落库、不进日志、不跨 fire 共享。

GET /capabilities + getCapabilities()(amsg-server + amsg-client)

worker 部署版本落后时,新功能从「静默失效」变成「探测得到落后」:端点返回 { serverVersion, features }(版本号构建期注入,feature 名中性、随版本追加),客户端 getCapabilities() 打到老 worker(404)返回 null 不抛错,前端可据此亮「worker 需要重新部署」的牌子。

验收

  • 五包 572 测试全绿(shared 68 / server 182 / client 79 / instant 187 / sw 56)
  • 分块往返:全中文大包、emoji 代理对边界、缩块不残留、缺块返兜底、陈旧分块写不覆盖新值
  • 全成功响应形状不变;老单值路径字节级不变(既有测试原样通过)
  • scratch:同一 fire 三个 hook 同引用;跨 fire / 抛错后隔离
  • 老 worker 探测:404 → null 不抛错
  • cloudflare 打包入口零 node 内置;__AMSG_SERVER_VERSION__ 构建期替换无残留
  • 红线 grep:无下游业务标识;hook ctx / scratch 不含凭据
  • changesets ×3(server minor、shared minor、client minor,pre 模式 next tag)

实现计划(含任务 4 评估结论):docs/superpowers/plans/2026-07-18-amsg-universal-gaps.md

https://claude.ai/code/session_012cnJAsyCLcxDH5UV5n3jgT

Tosd0 added 10 commits July 19, 2026 00:07

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces transparent chunking for client_state values exceeding 200KB, partial batch failure handling, a mutable scratch context for hook execution, and a new /capabilities endpoint for feature detection. The reviewer feedback highlights several important improvements: handling non-200/non-404 errors robustly in the client SDK's getCapabilities to avoid false negatives, parallelizing decryption in resolveClientStateEntries to eliminate sequential await bottlenecks, parsing maxStateValueBytes to support string-configured environment variables, and fixing a fragile headers fallback in the capabilities handler.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +487 to +494
if (res.status === 404) return null;

let json;
try {
json = await res.json();
} catch {
return null;
}

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.

medium

If the server returns a non-200 and non-404 HTTP status (such as a temporary 502 Bad Gateway or 503 Service Unavailable HTML page), res.json() will throw an error. Currently, this error is caught and the method silently returns null. This can mislead the client into thinking the worker is outdated (since null indicates the worker predates the endpoint), prompting incorrect UI redeployment hints. We should check the HTTP status and throw an error if it is not 200 or 404.

    if (res.status === 404) return null;

    let json;
    try {
      json = await res.json();
    } catch {
      if (res.status !== 200) {
        throw new Error('Request failed with status ' + res.status);
      }
      return null;
    }

Comment on lines +94 to +139
export async function resolveClientStateEntries(rows, fetchChunkRows, decryptValue) {
let chunkMap = null;
const loadChunks = async () => {
if (chunkMap === null) {
const chunkRows = await fetchChunkRows();
chunkMap = new Map(chunkRows.map((row) => [row.key, row]));
}
return chunkMap;
};

const entries = [];
for (const row of rows) {
const count = parseChunkedRootCount(row.value);
if (count === null) {
entries.push({
namespace: row.namespace,
key: row.key,
value: await decryptValue(row.value),
updatedAt: row.updated_at,
});
continue;
}

const map = await loadChunks();
const chunkRows = [];
let intact = true;
for (let i = 0; i < count; i++) {
const chunk = map.get(chunkKeyFor(row.key, i));
if (!chunk || chunk.updated_at !== row.updated_at) {
intact = false;
break;
}
chunkRows.push(chunk);
}
if (!intact) continue; // 写到一半断了 → 该 key 视为不存在

const parts = await Promise.all(chunkRows.map((chunk) => decryptValue(chunk.value)));
entries.push({
namespace: row.namespace,
key: row.key,
value: parts.join(''),
updatedAt: row.updated_at,
});
}
return entries;
}

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.

medium

There are two main areas of improvement in resolveClientStateEntries:

  1. Performance (Sequential Await): The current implementation sequentially awaits decryptValue for each non-chunked row inside the for loop. For namespaces with multiple keys, this introduces a sequential bottleneck. We can decrypt all rows in parallel using Promise.all.
  2. Robustness (Defensive Programming): If a custom or buggy database adapter returns null or undefined for rows or chunkRows, the function will crash with a TypeError. We should defensively ensure they are arrays before processing.
export async function resolveClientStateEntries(rows, fetchChunkRows, decryptValue) {
  const safeRows = Array.isArray(rows) ? rows : [];
  let chunkMap = null;
  const loadChunks = async () => {
    if (chunkMap === null) {
      const chunkRows = await fetchChunkRows();
      const safeChunkRows = Array.isArray(chunkRows) ? chunkRows : [];
      chunkMap = new Map(safeChunkRows.map((row) => [row.key, row]));
    }
    return chunkMap;
  };

  const promises = safeRows.map(async (row) => {
    const count = parseChunkedRootCount(row.value);
    if (count === null) {
      const decryptedValue = await decryptValue(row.value);
      return {
        namespace: row.namespace,
        key: row.key,
        value: decryptedValue,
        updatedAt: row.updated_at,
      };
    }

    const map = await loadChunks();
    const chunkRows = [];
    let intact = true;
    for (let i = 0; i < count; i++) {
      const chunk = map.get(chunkKeyFor(row.key, i));
      if (!chunk || chunk.updated_at !== row.updated_at) {
        intact = false;
        break;
      }
      chunkRows.push(chunk);
    }
    if (!intact) return null;

    const parts = await Promise.all(chunkRows.map((chunk) => decryptValue(chunk.value)));
    return {
      namespace: row.namespace,
      key: row.key,
      value: parts.join(''),
      updatedAt: row.updated_at,
    };
  });

  const results = await Promise.all(promises);
  return results.filter((entry) => entry !== null);
}

Comment on lines +142 to +144
const maxValueBytes = Number.isInteger(ctx.maxStateValueBytes) && ctx.maxStateValueBytes > 0
? ctx.maxStateValueBytes
: DEFAULT_MAX_STATE_VALUE_BYTES;

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.

medium

In Cloudflare Workers, environment variables are often injected as strings. If a deployer configures maxStateValueBytes via an environment variable (e.g., env.MAX_STATE_VALUE_BYTES) and passes it directly, Number.isInteger(ctx.maxStateValueBytes) will evaluate to false because it is a string. This will cause the custom limit to be silently ignored and fall back to DEFAULT_MAX_STATE_VALUE_BYTES. We should parse string values to integers to make the configuration robust.

Suggested change
const maxValueBytes = Number.isInteger(ctx.maxStateValueBytes) && ctx.maxStateValueBytes > 0
? ctx.maxStateValueBytes
: DEFAULT_MAX_STATE_VALUE_BYTES;
const rawLimit = ctx.maxStateValueBytes;
const parsedLimit = typeof rawLimit === 'string' ? parseInt(rawLimit, 10) : rawLimit;
const maxValueBytes = Number.isInteger(parsedLimit) && parsedLimit > 0
? parsedLimit
: DEFAULT_MAX_STATE_VALUE_BYTES;

Comment on lines +29 to +31
async function GET(url, headers) {
const effectiveHeaders = headers || url || {};
const tenantResult = await ctx.tenantManager.resolveTenant(effectiveHeaders);

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.

medium

The current assignment const effectiveHeaders = headers || url || {}; is fragile. If headers is undefined (e.g., if GET is called with only the url argument), effectiveHeaders will fall back to url, which is a string. Passing a string URL to resolveTenant (which expects a headers object) will cause runtime errors or fail to resolve the tenant. We should explicitly check the types of headers and url to handle both GET(url, headers) and GET(headers) signatures safely.

  async function GET(url, headers) {
    const effectiveHeaders = (headers && typeof headers === 'object')
      ? headers
      : (url && typeof url === 'object' ? url : {});
    const tenantResult = await ctx.tenantManager.resolveTenant(effectiveHeaders);

@Tosd0
Tosd0 merged commit 783a1ec into main Jul 18, 2026
1 check passed
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