-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat(cli): list managed Agent View sessions in qwen sessions ps #10942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
dbe9950
8879852
9b14578
94610b3
3e3dac5
4d84fbc
6b2c711
cd68e10
853d57b
d02752f
558bd25
32295e3
b5cf6d9
eb7642b
79ca2fc
0783984
075b47d
294cbe2
3b772f1
7c3e095
9d891f9
662923e
38e77c9
10d09db
83b98c1
8a95fe3
16c5251
9a0ca98
794fcba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,10 @@ import { | |
| DEFAULT_AGENT_VIEW_ATTACH_LEASE_TTL_MS, | ||
| } from './attach-lease.js'; | ||
| import type { AgentViewAttachLease } from './attach-lease.js'; | ||
| import { | ||
| readPidNamespaceId, | ||
| readProcStartToken, | ||
| } from '@qwen-code/qwen-code-core/utils/process-liveness.js'; | ||
| import { AGENT_VIEW_PROTOCOL_VERSION } from './protocol.js'; | ||
| import type { | ||
| AgentViewActivityFile, | ||
|
|
@@ -80,6 +84,40 @@ import { | |
| getAgentViewActivityInputState, | ||
| } from './presentation.js'; | ||
|
|
||
| /** | ||
| * The pid fields of a worker file, with the identity that makes them | ||
| * trustworthy to a later reader. | ||
| * | ||
| * Recording the numbers alone is not enough. Nothing reaps this store | ||
| * while no supervisor runs — `clearAgentViewWorkerPids` does not get to | ||
| * run after a SIGKILL or a reboot — so the durable file keeps naming pids | ||
| * the OS is free to hand to unrelated processes, and a `~/.qwen` shared | ||
| * between machines or namespaces offers pids that mean nothing here. A | ||
| * reader pairing these tokens with `isSameProcess` gets the same evidence | ||
| * standard the live-session registry already holds its own records to, | ||
| * instead of a bare `kill(pid, 0)`. | ||
| * | ||
| * Reading the token can fail (the process exits between spawn and this | ||
| * call, `/proc` is unreadable, the platform has none); `null` then records | ||
| * "no identity available", which readers degrade to a liveness check | ||
| * rather than treating as a mismatch. | ||
| */ | ||
| function workerPidIdentity(host: { pid: number; workerPid: number }): { | ||
| hostPid: number; | ||
| workerPid: number; | ||
| hostProcStart: string | null; | ||
| workerProcStart: string | null; | ||
| pidNs: number | null; | ||
| } { | ||
| return { | ||
| hostPid: host.pid, | ||
| workerPid: host.workerPid, | ||
| hostProcStart: readProcStartToken(host.pid), | ||
| workerProcStart: readProcStartToken(host.workerPid), | ||
| pidNs: readPidNamespaceId(), | ||
| }; | ||
| } | ||
|
|
||
| function resolveSessionCwd(cwd: string): string { | ||
| try { | ||
| return fs.realpathSync(cwd); | ||
|
|
@@ -470,8 +508,7 @@ class AgentViewSupervisorProcessHandler | |
| result.sessionId, | ||
| { | ||
| schemaVersion: 1, | ||
| hostPid: host.pid, | ||
| workerPid: host.workerPid, | ||
| ...workerPidIdentity(host), | ||
| ...(host.hostId ? { hostId: host.hostId } : {}), | ||
| ...(host.endpoint ? { hostEndpoint: host.endpoint } : {}), | ||
| ...(host.authToken ? { hostAuthToken: host.authToken } : {}), | ||
|
|
@@ -703,8 +740,7 @@ class AgentViewSupervisorProcessHandler | |
| adoption.sessionId, | ||
| { | ||
| schemaVersion: 1, | ||
| hostPid: host.pid, | ||
| workerPid: host.workerPid, | ||
| ...workerPidIdentity(host), | ||
| ...(host.hostId ? { hostId: host.hostId } : {}), | ||
| ...(host.endpoint ? { hostEndpoint: host.endpoint } : {}), | ||
| ...(host.authToken ? { hostAuthToken: host.authToken } : {}), | ||
|
|
@@ -2322,8 +2358,7 @@ class WorkerRegistry { | |
| sessionId, | ||
| { | ||
| schemaVersion: 1, | ||
| hostPid: host.pid, | ||
| workerPid: host.workerPid, | ||
| ...workerPidIdentity(host), | ||
| ...(host.hostId ? { hostId: host.hostId } : {}), | ||
| hostEndpoint: worker.hostEndpoint, | ||
|
yiliang114 marked this conversation as resolved.
yiliang114 marked this conversation as resolved.
Comment on lines
2360
to
2363
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] R7-2: [certifies-falsely] [new-surface] (still standing) The reconnect write site re-stamps the worker file's pid identity with pids an already-running PTY host self-reported over its socket, while Re-ruled at head The three spawn sites are safe (a plain local spawn shares the writer's namespace); only reconnect reaches a host that may live in another namespace, which defeats the cross-namespace guard these fields exist for. A devcontainer bind-mounts The witness recorded on the original thread still stands, and the mis-stamping is permanently stable rather than transient: Omitting the identity spread at this site suffices, because The fix must not write explicit 中文说明[Critical] R7-2:[certifies-falsely]new-surface重连写入点会用「已在运行的 PTY host 通过 socket 自报的 pid」重新盖章 worker 文件的 pid 身份,而 在本轮 head 三个 spawn 写入点是安全的(本地 spawn 的子进程与写入方共享命名空间);只有 reconnect 会连接到可能位于另一命名空间的 host,从而破坏这些字段本要防御的跨命名空间守卫。 具体场景:devcontainer 以 bind-mount 挂载 原线程记录的证据依然成立,且错误盖章是永久稳定的而非瞬时的(见上方 probe:pid 1 与 pid 2 的错误 token 完全相同,因为内核线程共享 starttime)。 在此处省略身份 spread 即可,因为 修复不得用显式 — qwen3.8-max via Qwen Code /review (v0.23.0)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verdict at head I read the chain at this head rather than re-running the previous round's reasoning. Evidence, with the lines I actually opened:
Not fixed this round, on purpose. This PR is at +1628/-75, past the +1500-addition scope fuse this closeout pass runs under, so this round is reply-only: no code, no commit, no push. The remedy below needs the owning maintainer's decision (part of it is a store-schema or identity-model call, not a local edit), so the thread stays open. Proposed fix shape (proposal only, not implemented):
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Re-verified at head
Only this file's import line changed since the previous verdict ( Code-frozen this round: the PR is at +1723/-75, past this closeout pass's 1500-line scope fuse, so no code was changed and nothing was pushed. Left UNRESOLVED for the owning maintainer's decision (no
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] R7-2: [certifies-falsely] [regression] (still standing at 8a95fe3) The reconnect write site spreads Witness: Fix: on the reconnect path, preserve the existing worker file's identity fields (or re-derive them from the host's own reports) instead of re-stamping from the reconnecting process. The spawn paths must keep stamping identity at write time (supervisor-process.ts:503-506 persists pids right after spawn, before any store I/O) — only this site consumes a self-reported handle. A reconnect test asserting the persisted 中文说明[Critical] R7-2(在 8a95fe3 仍未解决):重连写入点把 证据:witness: not run——需要一次跨 PID 命名空间的真实 PTY host 重连;机制已在 head 8a95fe3 逐行重读确认(supervisor-process.ts:2349-2370:重连把 修复:重连路径应保留 worker 文件里已有的身份字段(或根据 host 自己的上报重新推导),而不是用重连进程的信息重新写入。 spawn 路径必须继续在写入时记录身份(supervisor-process.ts:503-506 要求 spawn 后立即持久化 pid、先于任何 store I/O)——只有此点消费自报句柄。需要一个重连测试,断言持久化的 — qwen3.8-max via Qwen Code /review (v0.24.1)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still REAL at "Omit the identity spread" is not a one-line deletion — the spread is also what writes
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] R7-2: [certifies-falsely] [regression] Still standing at 794fcba (:2361): the reconnect write site re-stamps pid identity with self-reported pids (from the worker's own reconnect frame, The author's reply on the original thread stands: "a trust-boundary decision rather than a patch, so it stays unresolved". Neither fix commit touched this site; the other three Witness (this round): all four write sites re-walked at HEAD (chunk-1 round-4 layer receipt): ':511/:743/:2559 stamp pids from just-spawned local handles; the fourth site's self-reported pids are R7-2, already on the ledger'. Fix direction, witness and constraint are in the original thread (refuse to re-stamp on reconnect, or verify the reported pids first; reconnect itself must stay non-fatal per :2374-2376). 中文说明[Critical] R7-2:在 794fcba 仍然存在(:2361):重连写入点用自报的 pid(来自 worker 自己的重连帧 原作者在原始线程的回复仍然有效:"这是信任边界决策而非补丁,因此保持未解决"。两个修复提交都没有触及该写入点;其余三个 **证据(本轮):**四个写入点在 HEAD 全部重走(chunk-1 第 4 轮层收据):':511/:743/:2559 使用刚 spawn 的本地句柄;第四个点的自报 pid 即 R7-2,已在台账'。 修复方向、验收与约束见原始线程(重连时拒绝重新盖章,或先验证所报 pid;按 :2374-2376,重连本身必须保持非致命)。 — qwen3.8-max via Qwen Code /review (v0.24.1)
yiliang114 marked this conversation as resolved.
|
||
| ...(worker.hostAuthToken | ||
|
|
@@ -2521,8 +2556,7 @@ class WorkerRegistry { | |
| sessionId, | ||
| { | ||
| schemaVersion: 1, | ||
| hostPid: host.pid, | ||
| workerPid: host.workerPid, | ||
| ...workerPidIdentity(host), | ||
| ...(host.hostId ? { hostId: host.hostId } : {}), | ||
| ...(host.endpoint ? { hostEndpoint: host.endpoint } : {}), | ||
| ...(host.authToken ? { hostAuthToken: host.authToken } : {}), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Critical] R12-2: [certifies-falsely] [new-surface] The stalled-wiring rule this commit adds can never fire again for its own worked example, and the example's evidence sentence ("still zero external references") is falsified by the very commit that ships it.
The rule's second conjunct is "no external reference" (:162-163), and the block above it now works — the repaired probe (R10-1, fixed in
9a0ca988cc) keyed on the directory basename returns this PR's own consumers. So forpackages/cli/src/agent-viewthe conjunction is permanently false post-merge, the mandated stall report is unreachable, and the only documented outcomes left are silent drops (:137 "drop silently", :193 "Any production consumer → drop") — reinstating the exact behaviour this hunk calls the bug, while the fact situation (11,118 production lines, no entry point, two open conflicting wiring PRs) still holds. The worked example simultaneously ships a measurement ("still zero external references") that is false in the tree that ships it.Failure scenario: a post-merge survey reaches the recency gate on this subsystem, the probe returns
ps.ts/managed-rows.tsas production consumers, the three-conjunct test fails, and the agent drops silently — nothing ever says out loud that a five-figure subsystem with no entry point exists, which :307-310 names as the failure the rule exists to stop.Witness (executed on both arms):
Suggested fix: restate the precondition as the fact the rule reaches for — an entry point, not any reference ("nothing outside it that gives a user a way in") — and add the partial-consumer branch (a read-only listing is a reference and still not an entry point). Date/qualify the example: "still zero external references as of 2026-09-04;
qwen sessions ps(PR 10942) became the first out-of-directory consumer … still not an entry point".The reworded gate must keep the deletion verdict dropped once a production consumer exists (:193) — only the stall report may survive the reference. Acceptance: N/A (instruction prose; no test can pin it) — the witness above is the acceptance check a future round can re-run.
中文说明
[Critical] R12-2:本次提交新增的"接线停滞"规则,对它自己引用的示例目录将永远无法再触发;而示例中的证据句("仍然零外部引用")恰恰被发布它的这个提交证伪。
规则的第二个合取项是"无外部引用"(:162-163),而其上方的探测块现在能正常工作了——修复后的探针(R10-1,由
9a0ca988cc修复)按目录 basename 匹配,会返回本 PR 自己的消费者。因此对packages/cli/src/agent-view而言,合并之后该合取式永久为假,规则强制要求的停滞报告不可达,剩下的唯一文档化结局就是静默丢弃(:137 "drop silently"、:193 "Any production consumer → drop")——恰恰恢复了这一段称之为 bug 的行为,而事实状态(11,118 行生产代码、无入口、两个开放且冲突的接线 PR)依然成立。同时,示例发布了一句在发布它的代码树里为假的测量结论("仍然零外部引用")。**故障场景:**合并后的一次巡检对该子系统走到时效门,探针返回
ps.ts/managed-rows.ts作为生产消费者,三合取判定失败,代理静默丢弃——再也不会大声说出"存在一个五位数行数、没有入口的子系统",而这正是 :307-310 声明该规则要阻止的失败。**证据(在两侧分支上实际执行):**见上方英文 witness 代码块(BASE 0 个真实外部引用;PR 侧 7 个文件、4 个真实引用;入口 flag 扫描为零;7802/7803 均 OPEN 且冲突;生产行数 11,118)。
**建议修复:**把前置条件改述为规则真正想说的事实——入口,而不是任何引用("外面没有任何东西能给用户一个进入方式"),并补充部分消费者分支(只读列表算引用,但仍不是入口)。给示例加上日期/限定:"截至 2026-09-04 仍零外部引用;
qwen sessions ps(PR 10942)成为第一个目录外消费者……仍不是入口"。改述后的判定必须保持:一旦存在生产消费者,删除结论仍为 drop(:193)——只有停滞报告可以越过引用存在。验收:N/A(指令文本,无测试可钉)——上方 witness 即未来轮次可重跑的验收检查。
— qwen3.8-max via Qwen Code /review (v0.24.1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified at head
794fcbad45: REAL on both halves — the conjunct self-disables for its own example, and the example's measurement sentence is false in the tree that ships it.survey.md:162-163).9a0ca988cc, which I confirmed is real and on this branch:git log --oneline -1 9a0ca988cc→ "docs(skills): key the stalled-wiring probe on the directory basename". The probe issurvey.md:150-155:"$RG" -l '/<basename>/' --glob '!<dir>/**' packages./agent-view/occurs atps.ts:41('../../agent-view/supervisor-store.js'),ps.ts:47,managed-rows.ts:43,:47,:52, plus the two test files. So forpackages/cli/src/agent-viewthe second conjunct is permanently false post-merge, the mandated stall report is unreachable, and the only outcomes the text leaves are:137"drop silently" and:193"Any production consumer → drop" — the behaviour this hunk calls the bug.survey.md:302-303: "Measured again on 2026-09-04: 11,004 production lines, still zero external references". I did not re-measure the line count, so your 11,118 versus the doc's 11,004 stays unverified on my side; the zero-references half is what I checked, and it is false here.gh pr list --repo QwenLM/qwen-code --search 'agent-view in:title' --state openreturns feat(cli): Expose agent view commands #7802 "Expose agent view commands" and feat(cli): Add agent view roster UI #7803 "Add agent view roster UI" as of this round, both created 2026-07-27.packages/cli/src/commands/agent-view/does not exist on this head, and the only out-of-directory importers ofagent-view/inpackages/cli/srcandpackages/core/srcareps.ts,managed-rows.tsandui/layouts/DefaultAppLayout.tsx(the in-TUI panel). So the fact situation the rule exists to report is unchanged — which is exactly why the conjunct should read "nothing outside it that gives a user a way in" rather than "no external reference", with the partial-consumer branch you describe.No code this round: it is prose in a file this PR already touches (+53 lines), so it is the second-cheapest of the five, but the round is code-frozen at +1852/-75 pending the maintainer scope ruling, and rewording the gate changes what every future survey reports — that belongs in the ruling rather than in a sweep pass. Not resolving: as written the example ships a measurement the same commit falsifies.