fix(daemon): mark session supported-commands no-store (#10918)

[harjothkhara]

What this PR does

The daemon endpoint that serves a session's slash-command and Skill metadata now marks its response Cache-Control: no-store, so a client cannot answer a later read from a representation it stored earlier. Both branches of the endpoint are covered — the live-session read and the virtual sub-agent stub — so the directive does not depend on which kind of session id was asked for.

Why it's needed

A file-based Skill can be updated in place while the daemon session stays alive. The endpoint returned dynamic command state with no cache directive at all, which left a client free to store the representation and keep serving it — so autocomplete kept advertising the old description and argument hint while invoking the very same command already ran the updated Skill. Command discovery and command execution disagreed, and nothing in the response told the client its copy might have gone out of date.

It is worth being precise about what the reported 304 was, because if a stale response body were being validated then suppressing the cache would hide a bug rather than fix one. It was not that. There is no server-side cache on this path: the payload is rebuilt from the session's live command registry on every request, and the weak validator is derived from that freshly built body. Confirmed directly: a conditional request whose content is unchanged is answered 304, and one whose content has changed is answered 200 with the new body and a new validator. The validator cannot outlive the content it was computed from, so the endpoint already satisfies the first outcome the issue describes whenever the server-side snapshot is current.

What it could not do was stop a client that had already stored a representation from continuing to use it. The skill registry reloads on a short debounce after its file watcher fires, so a conditional request landing inside that window is answered 304 entirely correctly — and the client then holds that answer until something else prompts it to ask again. That accounts for every reported observation without any stale body: the 304, the later bypassing request returning different content, and execution already running the new Skill. Making the representation non-storable is the fix for that, and it is the second outcome the issue accepts.

One caveat worth recording rather than leaving for a reviewer to find: the directive does not by itself disable conditional handling, so a client that ignores no-store and still sends a validator can still be answered 304. That path is only reachable when the freshly built body is byte-identical to the copy the client holds, so the answer it gets is correct; a conforming cache, having not stored the response, has no validator to send in the first place. This matches how the other live session-state reads in the same router are already marked.

This is separate from the composer Skill-toggle refresh work in #9123 / #9131, which is entirely client-side — it changed only the Web Shell client and the daemon session provider in the web UI, and touched no daemon route and no cache directive. It also addresses only in-app toggles, not an external in-place edit of a Skill directory, and does nothing for other hosts consuming the daemon HTTP API.

Reviewer Test Plan

How to verify

Read a live session's supported-commands endpoint and confirm the response carries Cache-Control: no-store; repeat the read supplying the validator from the first response and confirm the header is present there too. Do the same against a virtual sub-agent session id, which takes the stub branch of the same endpoint, and confirm it is marked identically.

Then reproduce the reported scenario: with a daemon session alive, update a project Skill directory in place with a new description and argument hint, and read the endpoint again. The response should carry the new metadata, and — because it is no longer storable — an embedded client should re-ask rather than continue advertising the previous description while invoking the command runs the updated Skill.

Unit coverage in the daemon server suite: a new test asserts the header on the live-session read and that the payload follows an in-place metadata change rather than being pinned to the earlier representation; the existing virtual-stub test now also asserts the header.

Evidence (Before & After)

N/A — no TUI surface changes. The user-visible effect is in an embedded client's command list, and is reproduced by the steps above.

Tested on

OS	Status
🍏 macOS	✅
🪟 Windows	⚠️
🐧 Linux	⚠️

Environment (optional)

Node 22, unit tests only.

Risk & Scope

• Main risk or tradeoff: the endpoint can no longer be revalidated, so a client that polls it pays for a full body every time instead of an occasional not-modified. The payload is a command list for one session and this is not a hot path, so the cost is small — and it buys back the guarantee that what a client displays matches what invoking the command will do.
• Not validated / out of scope: the sibling read-only session status reads in the same router — context, context usage, stats, tasks, LSP, hooks — have the same shape and likewise carry no cache directive. They are deliberately left alone: a stale number there corrects itself on the next poll, whereas stale command metadata strands discovery against execution. Happy to widen this if maintainers would rather the whole family be consistent.
• Breaking changes / migration notes: none. The response body and status codes are unchanged; only a response header is added.

Linked Issues

Closes #10918

P.S. — you should hire me. 115+ contributions to open source: https://github.com/harjothkhara

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

[qwen-code-ci-bot]

Thanks for the PR — and for a description that actually argues the case instead of restating the diff. That made this easy to check.

Template looks good ✓ — every required section is filled in. One small omission: the template's 中文说明 block is missing. Non-blocking, just noting it.

Problem: observed, not theoretical. #10918 has a concrete reproduction — a named Skill (review), the in-place edit, /workspace/skills and invocation both showing the new metadata, the composer's conditional read returning 304, and a cache-bypassing read returning 200 with the new description. Version and client info are attached. This is a real report, and your read of why the 304 happened (a conditional request landing inside the registry's reload-debounce window, then being held by the client) is more careful than the issue's own framing — you ruled out a stale server-side body rather than assuming one. Good.

Direction: aligned. You picked the exact route the issue names, and Cache-Control: no-store is not a new mechanism here — it is already the house convention for live session-state reads in this same router. /session/:id/stats, the transcript read, and the session-resolve read all set it today. So this closes a genuine inconsistency rather than inventing something.

Size: not applicable — packages/cli/src/serve/** is not a Stage 0 core-protected path, and the change is 7 production lines (6 added, 1 deleted) plus 57 test lines.

Approach: the scope is right, and deliberately narrow. Marking both branches of the route (the live-session read and the virtual sub-agent stub) is the correct call — a directive that only applied to one kind of session id would be a trap. Leaving the sibling reads alone is defensible and you said why. One factual correction: the body lists stats among the siblings that "likewise carry no cache directive", but on current main /session/:id/stats already sets no-store. Trivial on its own — except it turns out to be a symptom of the real problem below.

Risk: this is where I have to stop you. The branch is stale and GitHub reports the PR as CONFLICTING / DIRTY. Your tree predates the workflow trust-boundary work: redactWorkflowsFromSupportedCommands appears zero times in your copy of session.ts, while on main it is live inside the very handler you edited. Concretely, your post-image is:

.json(await runtime.bridge.getSessionSupportedCommandsStatus(sessionId))

and main's is:

const status = await runtime.bridge.getSessionSupportedCommandsStatus(sessionId);
res.status(200).json(
  runtime.trusted ? status : redactWorkflowsFromSupportedCommands(status),
);

Both sides rewrote the same lines, so the conflict lands precisely on a trust boundary. Resolving it by keeping your side — the easy mistake, and the one an --ours/--theirs or an auto-resolver makes silently — drops the redaction and leaks workflowsEnabled, savedWorkflows, and the workflows command to untrusted workspaces. The correct resolution is both changes together: keep the runtime.trusted ternary, add .set('Cache-Control', 'no-store'). Worth calling out explicitly because nothing in the diff would hint that line matters.

No elevated risk signals from the revert-history path check — neither changed file matches the high-risk patterns.

Flagging these for discussion; I am still reading the code and the CI signal, and will follow up below.

中文说明

感谢贡献 —— 也感谢这份真正在论证、而不是复述 diff 的 PR 描述，让核查变得很轻松。

模板完整 ✓，各必填部分都写了。一个小遗漏：模板里的 中文说明 区块缺失。不阻塞合并，仅作提醒。

问题： 是已观测到的问题，不是理论性加固。#10918 有具体复现 —— 明确的 Skill（review）、就地修改、/workspace/skills 与命令执行都已读到新元数据、composer 的条件请求返回 304、绕过缓存的请求返回带新描述的 200，并附上了版本与客户端信息。这是真实报告。而你对 304 成因的判断（条件请求正好落在 skill registry 重载 debounce 窗口内，随后被客户端持有）比 issue 本身的描述更严谨 —— 你排除了"服务端返回了过期 body"这种可能，而不是直接假设它。

方向： 对齐。你改的正是 issue 点名的那条路由，而且 Cache-Control: no-store 在这里不是新机制 —— 它已经是同一个 router 里"会话实时状态读取"的既有约定：/session/:id/stats、transcript 读取、session-resolve 读取现在都设置了它。所以这是补齐一处真实的不一致，而不是另造一套。

规模： 不适用 —— packages/cli/src/serve/** 不属于 Stage 0 核心保护路径，改动为 7 行生产代码（新增 6、删除 1）加 57 行测试。

方案： 范围合适且刻意收窄。同时给路由的两个分支（实时会话读取与 virtual sub-agent stub）加指令是正确的 —— 只对一种 session id 生效反而是个坑。不动其它同类读取也说得通，并且你说明了理由。一处事实更正：正文把 stats 列入"同样没有缓存指令"的兄弟路由，但当前 main 上 /session/:id/stats 已经设置了 no-store。单看是小事 —— 但它恰好是下面这个真问题的征兆。

风险： 这里是必须拦一下的地方。分支已过期，GitHub 判定本 PR 为 CONFLICTING / DIRTY。 你的代码树早于 workflow 信任边界相关改动：redactWorkflowsFromSupportedCommands 在你这份 session.ts 里出现 0 次，而在 main 上它正好位于你所修改的那个 handler 内部。具体来说，你的改后代码是：

.json(await runtime.bridge.getSessionSupportedCommandsStatus(sessionId))

而 main 上是：

const status = await runtime.bridge.getSessionSupportedCommandsStatus(sessionId);
res.status(200).json(
  runtime.trusted ? status : redactWorkflowsFromSupportedCommands(status),
);

双方改动了同样的行，因此冲突恰好落在信任边界上。如果按你这一侧来解决冲突 —— 这是最容易犯的错，--ours/--theirs 或自动解决工具会悄无声息地这么做 —— 就会丢掉这层脱敏，把 workflowsEnabled、savedWorkflows 以及 workflows 命令泄露给不受信任的 workspace。正确的解法是两边都要：保留 runtime.trusted 三元判断，同时加上 .set('Cache-Control', 'no-store')。之所以明确点出来，是因为 diff 本身完全看不出这一行有多重要。

回滚历史的高风险路径检查未发现升级信号 —— 两个改动文件都不匹配高风险模式。

先提出这些；我仍在继续看代码与 CI 信号，结果会在下面跟进。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 1bb2aec978efa312592606fd513705571f4873fd · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

Before reading the diff I wrote down what I would have done from the title and the "Why it's needed" section alone: mark the route's response non-storable, on both branches, using whatever directive the neighbouring live-state reads already use, and pin it with a test that mutates the payload between two reads. Your PR is that, and I did not find a simpler path you missed.

Two alternatives I considered and think you were right to reject:

• no-cache instead of no-store. It permits storage but forces revalidation — and revalidation is exactly the path that produced the confusing 304 inside the registry's reload window. no-store removes the stored copy outright, which is the stronger and simpler guarantee.
• Fixing the client (cache: 'no-store' in the fetch init). That would repair the Web Shell and leave every other host consuming the daemon HTTP API stale — and fix(daemon): supported-commands can stay stale after Skill metadata changes #10918 explicitly calls out those hosts. Server-side is the correct place for this.

Is the directive actually load-bearing for the reported client? I checked rather than assumed. The Web Shell reads this route through DaemonClient.sessionSupportedCommands(), whose default transport is RestSseTransport — a real HTTP GET to this handler, so a browser HTTP cache applies and honours no-store. The ACP twin (_qwen/session/supported_commands in the acp-http dispatch) replies over a JSON-RPC connection where no HTTP cache is in play, so it correctly needs no directive. Marking only the REST route is right, not an omission. I also confirmed the owner-read middleware wrapping this handler never touches Cache-Control and guards on headersSent, so nothing strips or races the header.

Your 304 reasoning holds. Nothing in the serve app overrides Express's etag setting, so the validator stays weak and body-derived — it cannot outlive the content it was computed from. That is what makes the caveat you recorded (a non-conforming client that ignores no-store and still sends a validator can be answered 304, but only when the freshly built body is byte-identical to what it holds) a benign residual path rather than a hidden bug. Worth having said it in the description instead of leaving it to be found.

The new test is a real pin, not a header assertion. It reads once, mutates what the bridge returns to simulate the in-place Skill edit, then re-reads carrying the first response's If-None-Match and asserts 200 — not 304 — plus the new description and argument hint. That fails if the handler ever serves a content-independent or stale representation. Extending the existing virtual-stub test to assert the header is the right way to cover the second branch. I found no existing test anywhere in the serve suite asserting a 304 or an absent cache-control on this route, so nothing regresses.

The four-line comment you added is longer than the change it documents, but the why here genuinely is not obvious from the code — a future reader would reasonably wonder why a command list needs to be non-storable. It earns its place.

Blocking

1. The branch is stale, and the conflict sits on a trust boundary. GitHub reports mergeable: CONFLICTING, mergeStateStatus: DIRTY. Your copy of session.ts contains zero occurrences of redactWorkflowsFromSupportedCommands; on main it is live inside the exact handler you edited, and the route now sits roughly 470 lines further down the file than it does in your tree. Both sides rewrote the same .json(...) call, so the conflict lands precisely on the line that decides whether an untrusted workspace gets workflow metadata:

// main — must survive the rebase
const status = await runtime.bridge.getSessionSupportedCommandsStatus(sessionId);
res
  .status(200)
  .set('Cache-Control', 'no-store')          // ← your change
  .json(
    runtime.trusted
      ? status
      : redactWorkflowsFromSupportedCommands(status),   // ← must not be dropped
  );

Resolving by keeping your side — the easy mistake, and the one --ours/--theirs or an auto-resolver makes silently — drops the redaction and leaks workflowsEnabled, savedWorkflows, and the workflows command to untrusted workspaces. Please rebase onto current main and keep both. The same staleness is why your description lists stats as a sibling with no cache directive: on current main /session/:id/stats already sets no-store, while your reading of the neighbourhood was otherwise accurate.

Your test helpers survive the rebase untouched — sessionSupportedCommandsImpl and ServeSessionSupportedCommandsStatus both still exist in main's serve suite, so only the one handler conflicts.

Test evidence

This is an unattended CI run, so per the gate rules I did not build, run, or execute anything from this PR — no npm, no test runner, no checkout of your branch. The evidence below is the PR's own CI, read through the API.

There is no test evidence yet, and I want to be blunt about that rather than let an all-green-looking table imply otherwise. The head commit carries 16 check-runs, but every one of them is pull_request_target bot orchestration — assign, authorize, label, precheck, triage itself, and the skipped sandbox lanes. There is no pull_request-event workflow run for this head SHA at all, which means the unit suite, lint, typecheck, and build have never executed against your commit, and your new test has not run in CI. The success conclusions in the table are bookkeeping jobs, not a green suite; treat that table as "no signal yet", not "passed".

Check	Conclusion
precheck-pr / precheck (×2)	success
authorize (×2)	success
assign	success
label	success
triage	in_progress
delay-automatic-review	waiting
verify	skipped
tmux-testing	skipped
publish-verify	skipped
publish-tmux	skipped
publish-resolution	skipped
ack-review-request	skipped
resolve-pr	skipped
review-config	skipped

Not verified: that the new test passes, and that the route returns no-store at runtime — no PR CI has run, so neither is observable from here. Re-running CI after the rebase would supply both.

Sandboxed verification would settle this, and it is the one thing that could close the gap before a maintainer commits: @qwen-code /verify — that the response on both branches actually carries no-store against the base build, and that the new test genuinely pins the change rather than passing with the .set(...) removed. Because this is a fork PR without write access, that is a sponsored run: a maintainer's @qwen-code /verify comment approves the head it was written against, and the run carries a pre-execution risk screen plus a full workspace wipe before any of this PR's code executes. Worth reading the resulting report with the same skepticism as the fork's own CI logs — the code under verification is adversarial input, and a crafted PR can shape what a report says even though the sandbox bounds what it can do. @qwen-code /tmux is not available here, since it gates on the author's write access; there is also no TUI surface in this change, so it would not add signal anyway.

中文说明

代码审查

在读 diff 之前，我只根据标题和「Why it's needed」写下了自己会怎么做：给这条路由的响应加上不可存储指令，两个分支都加，沿用相邻实时状态读取已有的指令，再用一个"两次读取之间改动 payload"的测试把它钉住。你的 PR 正是如此，我也没找到更简单而被你漏掉的路径。

我考虑过、并认为你拒绝得对的两个替代方案：

• 用 no-cache 而非 no-store。它允许存储但强制重新校验 —— 而重新校验恰恰就是在 registry 重载窗口内产生那个令人困惑的 304 的路径。no-store 直接不让副本存在，保证更强也更简单。
• 在客户端修（fetch init 里写 cache: 'no-store'）。那样只修好了 Web Shell，其它通过 daemon HTTP API 消费的宿主仍然是过期数据 —— 而 fix(daemon): supported-commands can stay stale after Skill metadata changes #10918 明确点了这些宿主。服务端才是正确的位置。

这条指令对报告问题的客户端是否真的生效？ 我是查证过的，不是假设。Web Shell 通过 DaemonClient.sessionSupportedCommands() 读这条路由，其默认 transport 是 RestSseTransport —— 一次真实的 HTTP GET 打到这个 handler，因此浏览器 HTTP 缓存会介入并遵守 no-store。ACP 那条孪生路径（acp-http dispatch 里的 _qwen/session/supported_commands）是在 JSON-RPC 连接上应答，不涉及 HTTP 缓存，所以它确实不需要该指令。只标记 REST 路由是对的，不是遗漏。我也确认了包裹该 handler 的 owner-read 中间件从不触碰 Cache-Control，并且有 headersSent 保护，所以没有东西会剥掉或与这个 header 竞争。

你对 304 的推断成立。 serve app 没有任何地方覆盖 Express 的 etag 设置，所以校验器仍是 weak 且由 body 派生 —— 它不可能比它所依据的内容活得更久。这正是为什么你记录的那条注意事项（不遵守 no-store、仍带校验器的客户端仍可能收到 304，但只在新生成的 body 与其所持副本逐字节相同时）是一条良性的残余路径，而不是隐藏的 bug。把这点写在描述里、而不是留给 reviewer 去发现，是加分的。

新测试是真正的钉子，不是单纯的 header 断言。 它先读一次，然后改动 bridge 的返回值以模拟 Skill 就地编辑，再带着第一次响应的 If-None-Match 重新读取，并断言是 200 —— 不是 304 —— 外加新的 description 与 argument hint。一旦 handler 返回与内容无关的或过期的表示，它就会失败。把已有的 virtual-stub 测试扩展为同时断言该 header，是覆盖第二个分支的正确做法。我在整个 serve 测试套件里没有找到任何断言该路由返回 304 或断言 cache-control 缺失的既有测试，因此不存在回归。

你新增的四行注释比它所解释的改动还长，但这里的 why 确实无法从代码本身看出来 —— 后来的读者完全有理由疑惑为什么一份命令列表需要不可存储。这条注释是有价值的。

阻塞项

1. 分支已过期，且冲突落在信任边界上。 GitHub 判定 mergeable: CONFLICTING、mergeStateStatus: DIRTY。你这份 session.ts 里 redactWorkflowsFromSupportedCommands 出现 0 次；而在 main 上它正位于你修改的那个 handler 内部，并且该路由在文件中的位置比你的树靠后约 470 行。双方改写了同一个 .json(...) 调用，因此冲突恰好落在决定"不受信任 workspace 是否拿到 workflow 元数据"的那一行：

// main —— rebase 后必须保留
const status = await runtime.bridge.getSessionSupportedCommandsStatus(sessionId);
res
  .status(200)
  .set('Cache-Control', 'no-store')          // ← 你的改动
  .json(
    runtime.trusted
      ? status
      : redactWorkflowsFromSupportedCommands(status),   // ← 不能丢
  );

如果按你这一侧解决冲突 —— 这是最容易犯的错，--ours/--theirs 或自动解决工具会悄无声息地这么做 —— 就会丢掉脱敏逻辑，把 workflowsEnabled、savedWorkflows 和 workflows 命令泄露给不受信任的 workspace。请 rebase 到当前 main 并把两者都保留。同样的过期状态也解释了为什么你的描述把 stats 列为"没有缓存指令"的兄弟路由：当前 main 上 /session/:id/stats 已经设置了 no-store，而你对周边代码的其余判断都是准确的。

你的测试辅助在 rebase 后无需改动 —— sessionSupportedCommandsImpl 和 ServeSessionSupportedCommandsStatus 在 main 的 serve 套件里都仍然存在，因此只有那一个 handler 会冲突。

测试证据

这是一次无人值守的 CI 运行，因此按照门禁规则我没有构建、运行或执行本 PR 的任何代码 —— 没有 npm、没有测试运行器、没有 checkout 你的分支。下面的证据是 PR 自身的 CI，通过 API 读取。

目前没有任何测试证据，我想直说，而不是让一张看起来全绿的表造成误解。 head commit 上有 16 个 check-run，但每一个都是 pull_request_target 的机器人编排任务 —— assign、authorize、label、precheck、triage 自身，以及被跳过的沙箱通道。这个 head SHA 上完全不存在 pull_request 事件的 workflow run，也就是说单元测试套件、lint、typecheck 和 build 从未针对你的 commit 执行过，你新增的测试也没有在 CI 里跑过。表中的 success 结论是流程性任务，不是绿色的测试套件；请把那张表读作"尚无信号"，而不是"已通过"。

未验证：新测试是否通过，以及路由运行时是否真的返回 no-store —— PR CI 未运行，从这里两者都不可观测。rebase 之后重跑 CI 即可同时给出这两项。

沙箱验证可以定论此事，也是维护者投入之前唯一能补上缺口的动作：@qwen-code /verify —— 相对 base 构建确认两个分支的响应都真的带 no-store，并确认新测试真的钉住了改动，而不是把 .set(...) 删掉后依然通过。由于这是无写权限的 fork PR，这属于受赞助运行：由维护者发出 @qwen-code /verify 评论来批准其针对的 head，该运行会在执行本 PR 任何代码之前做执行前风险筛查并完整清空工作区。阅读产出的报告时，建议保持与阅读 fork 自身 CI 日志同样的怀疑 —— 被验证的代码是对抗性输入，精心构造的 PR 可以影响报告"说什么"，尽管沙箱限定了它能"做什么"。这里不适用 @qwen-code /tmux，因为它以作者写权限为前提；本次改动也没有 TUI 界面，所以它本来也不会增加信号。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 1bb2aec978efa312592606fd513705571f4873fd · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 2/5 — the fix itself is right and I expect to approve it once rebased; as submitted it cannot merge, and the conflict lands on a trust boundary.

Stepping back: my hesitation is not about the design. I proposed the same change before reading the diff, and you matched it — one header, both branches, reusing the convention already in the router, plus a test that mutates the payload instead of just asserting a header. There is no new mechanism, no new abstraction, no drive-by refactor, no formatting churn, and nothing in the diff that the stated goal does not need. Leaving the sibling reads alone with a written reason is restraint, not an omission, and if I were maintaining this route in six months I would thank you for the comment explaining why a command list has to be non-storable.

The problem is real, and I did not take your framing on trust. #10918 is an observed report with a named Skill, a client version, and the 304-then-200 sequence; I then checked the mechanism against the code and it holds — the validator is weak and body-derived, so a 304 inside the registry's reload window is a correct answer to a question that should not have been cacheable in the first place. Command discovery disagreeing with command execution is exactly the kind of failure that costs a user an afternoon, so this is worth fixing.

What stops me is that the submitted branch cannot be merged, and the reason it cannot be merged is not cosmetic. GitHub reports the PR CONFLICTING, your tree predates the workflow trust-boundary work, and the one handler you touched is now guarded on main by runtime.trusted ? status : redactWorkflowsFromSupportedCommands(status). Your side rewrote the same lines without that guard. Whoever resolves this has to make a deliberate choice, and the careless resolution silently removes a check that keeps workflowsEnabled, savedWorkflows, and the workflows command away from untrusted workspaces. I am not willing to approve a diff whose merge requires that choice to be made correctly by someone who has no signal in the diff that it matters — which is why I spelled the resolution out in the review comment above rather than just saying "please rebase".

The second gap is evidence. No pull_request-event CI has run against this head, so your new test has never executed anywhere I can observe. I read the test and I believe it is well constructed, but "I believe it would pass" is not the same as "it passed", and I am not going to present the former as the latter. The 16 checks on the commit are all bot orchestration; the green ones are bookkeeping.

So: request changes, on two specific and fixable things — rebase onto current main keeping the trust ternary alongside your .set('Cache-Control', 'no-store'), and let CI run on the new head. Both are yours to do and neither needs a maintainer's judgment call, which is why I am sending this back to you rather than deferring it to someone else. Come back with those and I would expect this to be a quick approve; the substance is already there.

One non-blocking note, offered as a maintainer who reads a lot of these: the P.S. — you should hire me line at the end of the description works against you. Everything above it is strong, careful work, and that is what should be the last thing a reviewer reads.

中文说明

Confidence: 2/5 —— 修复本身是对的，rebase 之后我预计会批准；但按当前提交状态它无法合并，而且冲突正好落在信任边界上。

退一步看：我的保留意见不在设计上。在读 diff 之前我提出的方案与你的一致 —— 一个 header、覆盖两个分支、复用 router 里已有的约定，再加上一个"改动 payload"而非只断言 header 的测试。没有引入新机制、没有新抽象、没有顺手重构、没有格式化噪音，diff 里也没有任何超出既定目标所需的东西。把兄弟路由留在范围外并写明理由，这是克制而不是遗漏；如果六个月后由我来维护这条路由，我会感谢那条解释"为什么命令列表必须不可存储"的注释。

问题是真实的，而且我没有直接采信你的表述。#10918 是一份观测到的报告，有明确的 Skill 名、客户端版本，以及先 304 后 200 的序列；随后我对照代码核查了成因，它成立 —— 校验器是 weak 且由 body 派生的，所以在 registry 重载窗口内返回 304 是对一个本就不该被缓存的请求给出了"正确"回答。命令发现与命令执行相互矛盾，正是那种会耗掉用户一整个下午的故障，所以这值得修。

拦住我的是：当前提交的分支无法合并，而它无法合并的原因并不是无关紧要的。GitHub 判定本 PR 为 CONFLICTING，你的代码树早于 workflow 信任边界相关改动，而你所修改的那个 handler 在 main 上现在由 runtime.trusted ? status : redactWorkflowsFromSupportedCommands(status) 把守。你这一侧改写了同样的行，却没有这道保护。解决冲突的人必须做出一个明确的选择，而草率的解法会悄无声息地移除一项检查 —— 正是它让 workflowsEnabled、savedWorkflows 和 workflows 命令不会暴露给不受信任的 workspace。我不愿意批准一个"合并时需要有人把这道选择做对、而 diff 里完全没有线索提示它重要"的改动 —— 这也是为什么我在上面的审查评论里把解决方案明确写出来，而不是只说一句"请 rebase"。

第二个缺口是证据。这个 head 上没有跑过任何 pull_request 事件的 CI，因此你新增的测试在我可观测的任何地方都从未执行过。我读了那个测试，也相信它写得好，但"我相信它会通过"和"它通过了"不是一回事，我不会把前者当作后者来陈述。该 commit 上的 16 个 check 全是机器人编排任务；绿色的那些只是流程性工作。

所以：请求修改，针对两件具体且可修复的事 —— rebase 到当前 main，在保留信任三元判断的同时加上你的 .set('Cache-Control', 'no-store')；并让 CI 在新的 head 上跑起来。这两件都属于你来做，也都不需要维护者做判断，因此我把它退回给你，而不是转交他人。补上这两点之后，我预计这会是一次很快的批准；实质内容已经具备了。

一条不阻塞的建议，来自一个读过大量 PR 的维护者视角：描述末尾那句 P.S. — you should hire me 对你是减分的。它上面的全部内容都是扎实、细致的工作，而那才应该是 reviewer 读到的最后一句。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 1bb2aec978efa312592606fd513705571f4873fd · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

@harjothkhara requesting changes on two specific, fixable things — the substance of this fix is right and I'd expect a quick approve once they're addressed.

1. The branch is stale and conflicts on a trust boundary. GitHub reports this PR as CONFLICTING / DIRTY. Your tree predates the workflow trust-boundary work: redactWorkflowsFromSupportedCommands appears zero times in your copy of packages/cli/src/serve/routes/session.ts, while on main it guards the exact handler you edited. Both sides rewrote the same .json(...) call, so the conflict lands on the line that decides whether an untrusted workspace receives workflow metadata. Please rebase onto current main and keep both changes:

const status = await runtime.bridge.getSessionSupportedCommandsStatus(sessionId);
res
  .status(200)
  .set('Cache-Control', 'no-store')          // your change
  .json(
    runtime.trusted
      ? status
      : redactWorkflowsFromSupportedCommands(status),   // must not be dropped
  );

Resolving by keeping your side — which --ours/--theirs or an auto-resolver does silently — would drop that redaction and leak workflowsEnabled, savedWorkflows, and the workflows command to untrusted workspaces. Flagging it explicitly because nothing in the diff hints that the line matters.

2. No CI has run against this head. There is no pull_request-event workflow run for 1bb2aec, so the unit suite, lint, typecheck, and build have never executed against your commit and the new test has not run anywhere. The 16 checks on the commit are all pull_request_target bot orchestration. Rebasing should trigger real CI; please confirm it goes green.

Neither item needs a maintainer's judgment call, which is why this is coming back to you rather than being deferred. Full reasoning — including why no-store beats no-cache here, confirmation that the directive is load-bearing for the Web Shell's RestSseTransport path, and why the ACP twin correctly needs no directive — is in my review comment above.

中文说明

@harjothkhara 就两件具体且可修复的事请求修改 —— 这个修复的实质是对的，处理完之后我预计会很快批准。

1. 分支已过期，且冲突落在信任边界上。 GitHub 判定本 PR 为 CONFLICTING / DIRTY。你的代码树早于 workflow 信任边界相关改动：redactWorkflowsFromSupportedCommands 在你这份 packages/cli/src/serve/routes/session.ts 里出现 0 次，而在 main 上它正把守着你所修改的那个 handler。双方改写了同一个 .json(...) 调用，因此冲突落在决定"不受信任 workspace 是否拿到 workflow 元数据"的那一行。请 rebase 到当前 main，并同时保留两处改动：

const status = await runtime.bridge.getSessionSupportedCommandsStatus(sessionId);
res
  .status(200)
  .set('Cache-Control', 'no-store')          // 你的改动
  .json(
    runtime.trusted
      ? status
      : redactWorkflowsFromSupportedCommands(status),   // 不能丢
  );

如果按你这一侧解决冲突 —— --ours/--theirs 或自动解决工具会悄无声息地这么做 —— 就会丢掉这层脱敏，把 workflowsEnabled、savedWorkflows 和 workflows 命令泄露给不受信任的 workspace。之所以明确点出来，是因为 diff 本身完全看不出这一行有多重要。

2. 这个 head 上没有跑过 CI。 1bb2aec 上不存在 pull_request 事件的 workflow run，因此单元测试套件、lint、typecheck 和 build 从未针对你的 commit 执行过，新测试也没有在任何地方运行过。该 commit 上的 16 个 check 全是 pull_request_target 的机器人编排任务。rebase 之后应该会触发真实 CI，请确认它变绿。

这两项都不需要维护者做判断，所以我把它退回给你，而不是转交他人。完整的推理 —— 包括为什么这里 no-store 优于 no-cache、确认该指令对 Web Shell 的 RestSseTransport 路径确实生效、以及为什么 ACP 孪生路径正确地不需要该指令 —— 都在我上面的审查评论里。

— Qwen Code · qwen3.8-max-2026-09-02

_{Reviewed at 1bb2aec978efa312592606fd513705571f4873fd · re-run with @qwen-code /triage}