Repository navigation
MCP 生命周期、ACP 失败诊断与稳定性加固 - #33
Conversation
- start() 清扫 results/ 中无任务行引用的结果文件；state 被隔离、锁超时或读取失败时跳过，清扫前在锁内重读任务行，不误删兄弟实例结果 - read_events 用 errors=replace，坏字节行按格式错误跳过 - worker 嵌套 AGENT_BRIDGE_HOME 从注册表 home 派生 - check_task/get_result 对非本实例任务回落读取 state.json，返回 from_state=true，silent_for_sec 为 null
- flush_state 按调用时的保存代数等待并返回是否落盘，持续保存时不再误报或卡住 - start/stop 落盘失败仍直接报错；wait_task 终态先落盘，并返回 persisted - state.json 中任务 message/result_text 截断保存，全文仍在结果文件与转录 - request_id 绑定持久化，重启后重放返回 reused - 空闲卸载成功后移除 adapter 映射，已卸载会话可被正常淘汰
- session/load 失败回退到 session/new 成功后，写入 warning 与 session_recreated 转录事件 - 只有 worker 自身输出重置 stall 计时；新增 turn_timeout_sec（默认 0 关闭），超时 stop_reason=timed_out - files_changed 只统计 cwd 内路径，越界路径进入 files_outside_cwd；跳过 .agent-bridge 与 Windows junction - 会话淘汰时清理转录缓存与文件；stdout 超长行给出明确错误
- 新增 diagnostics.redact_diagnostic，task.error/warnings、Codex/Antigravity 错误事件落盘前脱敏 - [env] deny 支持用户配置的 glob 名单，显式 set/agent env 优先；无内置禁止名单；dsh 回填同样遵守 - [agents.<name>] permission_policy：allow_once（默认）/ allow_always / deny；allow_once 下不选择 allow_always - 同步 ORCHESTRATION、SETUP、skill 与 agents.toml.example 中 #29–#32 的契约变化
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 3cd3d313-9bc2-4f3a-8cdb-8b75687c1002) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddb49ff1a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| cfg = config or EnvConfig() | ||
| explicit = set(cfg.set) | set(overrides or {}) | ||
| _apply_env_deny(env, origin, cfg, explicit) |
There was a problem hiding this comment.
When OPENROUTER_API_KEY and an Anthropic base URL are present, configuring [env].deny = ["ANTHROPIC_AUTH_TOKEN"] does not actually withhold that key from the Claude worker: this call removes it, but AcpAdapter._env() subsequently calls apply_claude_gateway_env(), which recreates ANTHROPIC_AUTH_TOKEN from OPENROUTER_API_KEY. This bypasses the new trust-boundary control and can authenticate Claude to a gateway the operator intended to block; apply the deny policy after agent-specific environment transformations as well.
Useful? React with 👍 / 👎.
| if task.task_id not in self.tasks: | ||
| # Read via the disk fallback: values are the other instance's | ||
| # last-persisted state, not live data. | ||
| payload["from_state"] = True |
There was a problem hiding this comment.
When a sibling instance owns a task whose persisted status is running, this branch marks the snapshot from_state, but the later status-based calculation still evaluates this process's empty activity map as 0. Consequently check_task reports silent_for_sec: 0, falsely implying fresh worker output even though the value is unknowable from persisted state; disk-only snapshots should explicitly return null as documented.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: 52812ef5-c10c-4621-9dbe-dfa8b6f483cd) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: c183a563-efae-4f29-8612-4d90e5e84641) |
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| _apply_env_deny and enforce_env_deny so a denied key cannot re-enter | ||
| through post-build rewriting. | ||
| """ | ||
| if key in BRIDGE_OWNED_ENV_KEYS or (explicit and key in explicit): |
There was a problem hiding this comment.
🟥 Unpinned Bridge context bypasses deny patterns
With a denied, inherited AGENT_BRIDGE_PARENT_CONTEXT, denied_by_policy preserves its value even when worker context is not pinned. Any subprocess given that environment can start a nested Bridge without the worker restrictions.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for raw in payload.get("requests") or []: | ||
| if not isinstance(raw, dict): | ||
| continue | ||
| rid = raw.get("request_id") | ||
| tid = raw.get("task_id") | ||
| fp = raw.get("fingerprint") | ||
| if rid and fp and tid in self.tasks: | ||
| self._requests[rid] = (fp, tid) |
There was a problem hiding this comment.
严重程度：高
问题： #29 把 request_id 绑定落盘以便重启后复用，但 start() 只在 tid in self.tasks 时装入 _requests。兄弟实例仍活时任务行被 _foreign_live 跳过，绑定也跟着丢掉。dispatch_task 只查本进程内存的 _requests；_merge_owned(..., id_key="request_id") 再 merged.update(mine) 会用本实例的新绑定盖掉盘上同一 request_id 的活兄弟行。test_sibling_instances_do_not_clobber_state 只盖盖了 session/task id，没覆盖客户端提供、可碰撞的 request_id。
触发条件： 两个 Bridge 实例共享 AGENT_BRIDGE_HOME（SETUP 明确支持的拓扑）。实例 A 已经用 UUID request_id=R 派发出 T1。实例 B 在 A 仍活时 start()（MCP 重连时旧进程还在 stop()/flush 是典型窗口），协调器对 B 重试同一 R。
影响： B 返回 reused=false 并再跑一遍 worker（副作用、费用）。磁盘索引从 R→T1 被覆盖成 R→T2，违反 SETUP「落盘不动活兄弟记录」。A 之后的 save() 又会写回 R→T1，索引来回覆盖；重启去重正好落在这个重叠窗口失效。
建议： dispatch_task 在创建前按 state 锁重读 requests：同 request_id +指纹的活兄弟行返回该 task_id（from_state / reused），指纹不同则拒绝。_merge_owned 对 requests 不覆盖 foreign_live 的同键行。补一条与 test_sibling_instances_do_not_clobber_state 对齐的共享 request_id 测试。
Summary
idle_exit_sec默认 0），Closes [bug] AgentBridge会在长线程在一定的空闲时间以后自杀 #14error，新增结构化failure（operation/kind/exit_code/prompt_may_have_been_sent/stderr_summary），Closes [design] ACP 失败缺少阶段、退出码和安全重试信息 #20cancelled；worker 退出码可靠获取；stderr 按块读取不再阻塞wait_task终态先落盘并返回persisted；request_id 绑定跨重启保留。部分处理 [bug] 任务终态落盘前 wait_task 就已返回；Bridge 被强杀后完成的任务被改判 bridge_restarted #27（check_task/get_result不额外落盘）turn_timeout_sec/timed_out、files_outside_cwd、session_recreated警告、跳过 junction[env] deny、permission_policy默认allow_oncefrom_stateTest plan
Note
Medium Risk
Changes core task execution, persistence, worker env, and ACP permission handling—coordinators must interpret new failure/retry semantics and
persisted, though defaults aim to be safer.Overview
This PR hardens Agent Bridge’s MCP lifecycle, task observability, and trust boundaries, with matching doc updates for coordinators.
Lifecycle & timeouts:
server.idle_exit_secnow defaults to 0 (no idle self-exit; EOF drives cleanup). Per-agentturn_timeout_seccaps turn wall-clock (failed/timed_out), whilestall_timeout_seconly considers worker-originated transcript events. Terminalwait_taskflushes state and returnspersisted.ACP failures: Failed turns keep string
errorand add optional structuredfailure(operation,kind,exit_code,prompt_may_have_been_sent, redactedstderr_summary). The ACP adapter classifies errors, redacts diagnostics, improves stderr draining/cancel behavior, and surfacessession/loadfallbacks via warnings plus asession_recreatedtranscript event.Results & files:
files_changedis cwd-scoped; paths outside the task cwd go tofiles_outside_cwd. Workspace scanning skips.agent-bridgeand junctions.check_task/get_resultcan read another instance’s task fromstate.json(from_state: true);wait_task/cancel_taskstill require the owning instance.Config & security:
[env] denyglobs strip keys from worker env (explicitset/agent env wins).permission_policy(allow_oncedefault,allow_always,deny) replaces always-allow for OpenCode/Claude permission prompts.request_iddedup bindings persist in state; state writes are generation-scoped with bounded task rows and orphan-result cleanup.Reviewed by Cursor Bugbot for commit 9d6f77e. Bugbot is set up for automated code reviews on this repo. Configure here.