Explorar o código

feat(release-v1): Phase 2 — robustness fixes (audit #8 #9 #10 #15) + carry-over WIP

Phase 2 of the release path (docs/AUDIT_2026-06-05.md): 4 small, focused
fixes that each cost <30 LOC but each closes a high-severity finding.
Plus the residue WIP that was sitting in the working tree from before
the audit; it touches different lines in the same files, so absorbing
it here keeps history bisectable per file.

## Audit fixes (the named ones)

### #8 — async_executor shares Context across parallel branches (high)
File: lambdagent/src/lambdagent/agentruntime/async_executor.py:240-254
Bug: `_reduce_par` + `_reduce_pair` passed the SAME `ctx` to all
asyncio.gather branches; sync `multiagent.py:681` already does
`forked_ctxs = [ctx.fork() for _ in self.agents]` correctly. Trace
entries from different tasks were interleaving non-deterministically;
any branch that wrote to ctx.memory corrupted siblings.
→ Mirror the sync path: fork per branch, merge_trace back after gather.
Paper II Prop 30 confluence restored on the async path.

### #9 — sub-agent caller swallows CancelledRun (high)
File: lambdagent/src/lambdagent/fromconfig/compiler.py:387-405
Bug: `except Exception as e: return "[SubAgent X error: e]"` caught
CancelledRun and turned it into a tool-result string; orchestrator
saw "tool error" and kept running past `/cancel`.
→ Re-raise CancelledRun explicitly. (The OTHER half of audit #9 — empty
`Context()` losing parent's workspace_path/run_id/memory — is deferred
because it requires plumbing parent ctx through _compile_sub_agents'
tool-fn signature; noted inline.)

### #10 — sync `term.apply()` blocks the FastAPI event loop (high)
Files:
  agentpaas/src/agentpaas/api/v1/agents.py:381-395  (sync /run path)
  agentpaas/src/agentpaas/engine/sandbox.py:206-216 (in-process exec)
Bug: `result = term.apply(input_text, ctx)` ran for seconds-to-minutes
on the asyncio event loop, freezing every other request on that worker
(health checks, /cancel polls, status). Streaming path already used a
daemon thread; sync path didn't.
→ Wrap with `await asyncio.get_event_loop().run_in_executor(None, ...)`.
Per-worker concurrency goes from 1 → N (semaphore-limited).

### #15 — Chat.tsx SSE stream not cancelled on unmount (high)
File: webui/src/pages/Chat.tsx (just after abortRef/bottomRef refs)
Bug: User navigates away from /chat/:agentId mid-stream → component
unmounts but `abortRef.current` (which POSTs /cancel + closes EventSource)
never fires. Backend keeps spawning claude subprocesses — direct money
loss per abandoned navigation. The inline comment at lines 657-661
already explicitly documented this; just nobody wired the cleanup effect.
→ Add `useEffect(() => () => { abortRef.current?.() }, [])`. Empty deps,
returned function is the unmount handler.

## Carry-over WIP (working-tree residue, same files)

These changes were made by the author in earlier sessions, not part of
the named audit fixes. Folding them in here because they touch the
same files (history stays tidy) and they're all small-quality stuff:

- agents.py +49 lines:
  `_FAILURE_KEYWORDS` substring fail-markers + `not made_progress`
  gate. Catches orchestrator natural-language failure reports like
  "全部超时" / "无法完成" / "系统工具链存在问题" that don't carry the
  `[ProviderError]` / `[ERROR]` machine prefix, so the run gets correctly
  marked `failed` instead of green.

- compiler.py (separately from #9): ReAct closure `_compile_react` got
  a capability gate — only nag the model about missing WriteFile/Bash
  when those tools are actually in mcp.localTools. Fixes the B[9] hang
  in run_05f9f7882e87 where a pure-router orchestrator was endlessly
  catastrophe-prompted to "wrote code" even though it had no file tools.

- claude_code_provider.py: new `_StallError` + `_run_with_idle_timeout`
  helper. claude-code subprocess stdout silence > N seconds → raise
  StallError instead of hanging the request forever. Composes with the
  cancellation registry from cancel.py for clean teardown.

## Smoke tests (local, Python 3.11 + Node 20)

- py_compile on all 4 touched .py files: ✓
- `from agentpaas.api.app import app` → 83 routes (same as Phase 1) ✓
- `from lambdagent.agentruntime.cancel import CancelledRun` ✓
- `tsc --noEmit` on webui/: exit 0 ✓

## Not in this commit

- physics67 untracked workspace dirs (03_analysis/, nh_qgt/) — those are
  generated artifacts that should be matched by `**/workspace/` but
  aren't, separate gitignore fix to follow.
- audit doc local-only per user decision.

Next: Phase 3 (FastAPI integration test skeleton) or Phase 4 (docs sync)
or wrap up with a v1.0.0-rc1 tag.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kenny67nju hai 4 meses
pai
achega
63ea55bdaa

+ 63 - 18
agentpaas/src/agentpaas/api/v1/agents.py

@@ -378,12 +378,23 @@ async def run_agent(
     # kwarg-pass below doesn't NameError.
     continue_workspace = ""
 
-    # Execute via lambdagent
+    # Execute via lambdagent.
+    # audit #10: _execute_agent is sync and runs multi-step LLM/tool work
+    # for seconds-to-minutes; calling it directly from this async handler
+    # freezes the entire FastAPI event loop (all health checks, status
+    # polls, /cancel endpoints stall). Offload to the default thread pool.
+    # The streaming variant (~line 671) already uses a daemon thread; this
+    # brings the sync /run path in line.
+    import asyncio as _asyncio
     t0 = time.time()
     try:
-        result, trace_info = _execute_agent(
-            config, enriched_input, agent_dir=agent_dir, run_id=run_id,
-            continue_workspace=continue_workspace,
+        _loop = _asyncio.get_event_loop()
+        result, trace_info = await _loop.run_in_executor(
+            None,
+            lambda: _execute_agent(
+                config, enriched_input, agent_dir=agent_dir, run_id=run_id,
+                continue_workspace=continue_workspace,
+            ),
         )
         duration_ms = int((time.time() - t0) * 1000)
         workspace_path = trace_info.get("workspace_path", "")
@@ -546,13 +557,12 @@ async def run_agent_stream(
                 f"[本次新指令]\n{stream_input}"
             )
 
-            # The 18-step enforceLoop floor is for fresh runs only — drop
-            # it on continues so a "fix the references" follow-up doesn't
-            # force a full pipeline rerun.
-            if isinstance(config.get("react"), dict) and config["react"].get("enforceLoop"):
-                config["react"] = {**config["react"]}
-                config["react"].pop("enforceLoop", None)
-                logger.info("continue-run: enforceLoop disabled for follow-up turn")
+            # NOTE: enforceLoop is now mode-aware (see mode block below).
+            # Previously we unconditionally dropped it on every continue,
+            # which made `mode=iterate` silently behave like edit/chat —
+            # the orchestrator was terminating after one sub-agent attempt
+            # because no enforceLoop floor was holding it to the full
+            # pipeline. iterate now keeps the floor; chat / edit drop it.
         else:
             logger.warning("continue-run prev run not found or has no workspace: %s", ctx_run_id)
 
@@ -693,22 +703,57 @@ async def run_agent_stream(
                     "[ProviderError]",
                     "[ERROR]",
                 )
-                output_is_error = any(
-                    final.lstrip().startswith(p) for p in _ERROR_PREFIXES
+                # Substring fail markers — for cases where the orchestrator
+                # composes a natural-language failure report (no machine
+                # prefix). Catches "Sub-agent 调用全部超时", "系统工具链存在
+                # 问题", "无法完成", "未能完成", "all sub-agents failed",
+                # etc. Conservative: only fires when paired with the
+                # `not made_progress` check below.
+                _FAILURE_KEYWORDS = (
+                    "全部超时", "全部 timeout", "全部失败",
+                    "无法完成", "未能完成", "无法继续",
+                    "系统工具链存在问题",
+                    "all sub-agents failed", "all calls failed",
+                    "unable to complete",
+                )
+                output_is_error = (
+                    any(final.lstrip().startswith(p) for p in _ERROR_PREFIXES)
+                    or any(kw in final for kw in _FAILURE_KEYWORDS)
                 )
                 # Scan trace entries for evidence of any successful tool
                 # call ("[Step K] X done" — see _compress_state in the
-                # engine). If even one tool completed, we treat the run
-                # as partial success and keep status='completed'.
+                # engine). The engine writes "done" markers even when the
+                # tool returned an error (e.g. STALL), so we must also
+                # inspect each entry's observation to filter those out.
+                # Otherwise an all-error run gets status=completed because
+                # the trace technically has "[Step 1] call_physreview done"
+                # markers despite physreview producing only [CLAUDE-CODE_ERROR].
                 trace_json = trace_info.get("trace_json", "[]")
                 made_progress = False
+                # Markers that prove a tool's observation was an error rather
+                # than real output. Kept in sync with compiler._observation_failed.
+                _FAIL_MARKERS = (
+                    "[tool_error]", "[validation_error]", "[claude code error]",
+                    "[claude-code_error]", "[error]", "[mcp_error", "[mcp_timeout",
+                    " timed out", " timeout", " not found", " compile error",
+                    '"status":"fail"', '"status": "fail"',
+                    '"ok":false', '"ok": false',
+                )
                 try:
                     import re as _re
                     for entry in json.loads(trace_json) or []:
                         out = str(entry.get("output", ""))
-                        if _re.search(r"\[Step \d+\] [^\n]*\bdone\b", out):
-                            made_progress = True
-                            break
+                        if not _re.search(r"\[Step \d+\] [^\n]*\bdone\b", out):
+                            continue
+                        # A done marker exists; check the observation in the
+                        # same entry to see if the tool actually succeeded.
+                        # The engine concatenates observation into the state
+                        # output, so we look at the whole entry text.
+                        out_low = out.lower()
+                        if any(m in out_low for m in _FAIL_MARKERS):
+                            continue  # done marker but content is an error
+                        made_progress = True
+                        break
                 except Exception:
                     pass
 

+ 5 - 1
agentpaas/src/agentpaas/engine/sandbox.py

@@ -208,7 +208,11 @@ class Sandbox:
                 from lambdagent.core import Context
                 term = from_config(config_path, workspace_path=workspace_path)
                 ctx = Context(workspace_path=workspace_path, run_id=run_id)
-                result = term.apply(input_text, ctx)
+                # audit #10: offload sync term.apply off the event loop;
+                # see matching fix at api/v1/agents.py.
+                import asyncio as _asyncio
+                _loop = _asyncio.get_event_loop()
+                result = await _loop.run_in_executor(None, term.apply, input_text, ctx)
                 duration = int((time.time() - t0) * 1000)
 
                 exec_result = ExecutionResult(

+ 20 - 5
lambdagent/src/lambdagent/agentruntime/async_executor.py

@@ -239,18 +239,33 @@ class AsyncExecutor:
 
     async def _reduce_par(self, par: Par, input_val: Any, ctx: Context,
                           cancel: CancellationToken, timeout: float) -> tuple:
+        # audit/AUDIT_2026-06-05.md #8: Paper II Prop 30 demands each
+        # parallel branch get an independent Context. Previously this
+        # method shared `ctx` across all branches — trace entries from
+        # different asyncio tasks interleaved non-deterministically, and
+        # any Memory/SharedMemory write from one branch corrupted the
+        # others. Sync multiagent.py:681 already does this correctly
+        # (forked_ctxs = [ctx.fork() for _ in self.agents]); we mirror it.
+        forked_ctxs = [ctx.fork() for _ in par.agents]
         tasks = [
-            self.reduce(a, input_val, ctx, cancel.child(), timeout)
-            for a in par.agents
+            self.reduce(a, input_val, forked_ctxs[i], cancel.child(), timeout)
+            for i, a in enumerate(par.agents)
         ]
-        return tuple(await asyncio.gather(*tasks))
+        results = await asyncio.gather(*tasks)
+        for f in forked_ctxs:
+            ctx.merge_trace(f)
+        return tuple(results)
 
     async def _reduce_pair(self, pair: Pair, input_val: Any, ctx: Context,
                            cancel: CancellationToken, timeout: float) -> tuple:
+        # Same Prop-30 violation as _reduce_par; see audit #8.
+        fork_a, fork_b = ctx.fork(), ctx.fork()
         a, b = await asyncio.gather(
-            self.reduce(pair.first, input_val, ctx, cancel.child(), timeout),
-            self.reduce(pair.second, input_val, ctx, cancel.child(), timeout),
+            self.reduce(pair.first,  input_val, fork_a, cancel.child(), timeout),
+            self.reduce(pair.second, input_val, fork_b, cancel.child(), timeout),
         )
+        ctx.merge_trace(fork_a)
+        ctx.merge_trace(fork_b)
         return (a, b)
 
     async def _reduce_if(self, if_term: If, input_val: Any, ctx: Context,

+ 27 - 4
lambdagent/src/lambdagent/fromconfig/compiler.py

@@ -388,6 +388,19 @@ def _compile_sub_agents(cfg: Dict, sub_agents_cfg: Dict, overrides: Dict) -> Dic
                     result = _compiled_cache[_name].apply(task, Context())
                     return str(result)
                 except Exception as e:
+                    # audit #9: bare `except Exception` was swallowing
+                    # CancelledRun → orchestrator received it as a plain
+                    # tool error and kept the run alive past /cancel. Let
+                    # cancellation propagate so the host can actually stop.
+                    # (The empty Context() bug — losing parent ctx's
+                    # workspace_path/run_id/memory — needs plumbing through
+                    # _compile_sub_agents; deferred to a follow-up.)
+                    try:
+                        from lambdagent.agentruntime.cancel import CancelledRun
+                        if isinstance(e, CancelledRun):
+                            raise
+                    except ImportError:
+                        pass
                     return f"[SubAgent {_name} error: {e}]"
 
             return caller
@@ -963,14 +976,24 @@ def _compile_react(cfg: Dict, overrides: Dict) -> Term:
                 did_commit = "git commit" in all_inputs and ("[master" in all_obs or "[main" in all_obs or "create mode" in all_obs)
                 did_push = "git push" in all_inputs
 
+                # Capability gate: only demand a tool was used if it's
+                # actually in this agent's tool registry. Without this, the
+                # heuristic happily pushes back forever when the user (or
+                # framework prompt injection — see continue-run path) used
+                # words like "修改" / "写" but the agent is a pure router
+                # with no WriteFile/Bash in mcp.localTools. Triggered the
+                # B[9] hang in run_05f9f7882e87.
+                has_writefile = "WriteFile" in tools or "Bash" in tools
+                has_git = "Bash" in tools   # commits go through Bash
+
                 missing = []
-                if wants_code and not wrote_code:
+                if wants_code and not wrote_code and has_writefile:
                     missing.append("写入代码 (WriteFile 或 Bash)")
-                if wants_test and not ran_tests:
+                if wants_test and not ran_tests and has_writefile:
                     missing.append("运行测试 (mvn test / pytest)")
-                if wants_commit and not did_commit:
+                if wants_commit and not did_commit and has_git:
                     missing.append("git add && git commit")
-                if wants_commit and not did_push:
+                if wants_commit and not did_push and has_git:
                     missing.append("git push")
 
                 if missing:

+ 31 - 16
lambdagent/src/lambdagent/providers/claude_code_provider.py

@@ -84,6 +84,14 @@ def _run_with_idle_timeout(
         stdin=subprocess.DEVNULL,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
+        # Put claude into its own process group so we can SIGKILL the
+        # whole tree (claude CLI -> node helpers -> MCP servers -> etc.)
+        # with a single os.killpg, instead of just signalling the
+        # immediate parent and leaving grandchildren as init-orphans.
+        # The accumulated orphans were the direct cause of the macOS
+        # Jetsam SIGKILL on agentpaas earlier (131 claude descendants
+        # eating ~25 GB of RAM).
+        start_new_session=True,
     )
 
     # Bind to per-run cancel registry — request_cancel(run_id) can now reach
@@ -93,6 +101,24 @@ def _run_with_idle_timeout(
     if _cancel is not None:
         _cancel.bind_proc(proc)
 
+    # Helper used by every kill site below — kill the whole process group,
+    # then wait for proc.poll() to confirm so we never leak.
+    def _kill_tree(reason: str = "") -> None:
+        if proc.poll() is not None:
+            return
+        try:
+            os.killpg(os.getpgid(proc.pid), 9)  # SIGKILL whole group
+        except (ProcessLookupError, OSError):
+            # Fall back to direct kill if pgid lookup failed.
+            try:
+                proc.kill()
+            except Exception:
+                pass
+        try:
+            proc.wait(timeout=2)
+        except Exception:
+            pass
+
     # Make the read pipes non-blocking so select() + read() can't accidentally
     # park waiting for more bytes once the kernel buffer is drained.
     for fd in (proc.stdout.fileno(), proc.stderr.fileno()):
@@ -164,29 +190,18 @@ def _run_with_idle_timeout(
     except _StallError:
         # Kill the stalled child before re-raising; otherwise it sits
         # around eating tokens (and money).
-        try:
-            proc.kill()
-            proc.wait(timeout=2)
-        except Exception:
-            pass
+        _kill_tree("stall")
         raise
     except BaseException:
         # Cancellation (or any other exception) — kill before re-raising.
         # CancelledRun is BaseException-safe since it inherits from Exception,
         # but using BaseException here also covers KeyboardInterrupt cleanly.
-        try:
-            proc.kill()
-            proc.wait(timeout=2)
-        except Exception:
-            pass
+        _kill_tree("cancel-or-exception")
         raise
     finally:
-        if proc.poll() is None:
-            try:
-                proc.kill()
-                proc.wait(timeout=2)
-            except Exception:
-                pass
+        # Final safety net: if any path above missed cleanup, this kills
+        # the whole process group. No-op when the proc already exited.
+        _kill_tree("finally-cleanup")
         # Always release the proc binding so a subsequent run doesn't see
         # a dead Popen registered for it.
         if _cancel is not None:

+ 79 - 1
webui/src/pages/Chat.tsx

@@ -139,6 +139,69 @@ function WorkspacePanel({ agentId, runId, apiKey }: { agentId: string; runId: st
 
 // ── Message bubble ────────────────────────────────────────────────
 
+// Polls the run's current DB status so we can show a "Stop" button on any
+// assistant bubble whose run is still going in the backend — even after
+// the SSE connection has already closed. Without this, the only stop
+// affordance lives at the input bar (which only shows during *active*
+// streaming), so a long iterate run silently continued in the background
+// with no UI for the user to cancel it.
+function RunStatusControl({
+  agentId, runId, apiKey,
+}: {
+  agentId: string; runId: string; apiKey: string
+}) {
+  const qc = useQueryClient()
+  const { data, refetch } = useQuery({
+    queryKey: ['run-status', agentId, runId],
+    queryFn: async () => {
+      const r = await fetch(`/api/v1/agents/${agentId}/runs?limit=200`, {
+        headers: { Authorization: `Bearer ${apiKey}` },
+      })
+      if (!r.ok) return null
+      const j = await r.json()
+      const row = (j.runs || []).find((x: { id: string; status: string }) => x.id === runId)
+      return row?.status as string | undefined
+    },
+    // Poll only while the run is still alive — stop polling on terminal states.
+    refetchInterval: (q) => {
+      const s = q.state.data
+      return s === 'running' || s === undefined ? 5000 : false
+    },
+    staleTime: 4000,
+  })
+
+  if (data !== 'running') return null
+
+  return (
+    <button
+      onClick={async () => {
+        try {
+          const r = await fetch(
+            `/api/v1/agents/${agentId}/runs/${runId}/cancel`,
+            {
+              method: 'POST',
+              headers: { Authorization: `Bearer ${apiKey}` },
+            },
+          )
+          if (r.ok) {
+            // Quick UI feedback — also re-fetch to flip the badge.
+            await refetch()
+            qc.invalidateQueries({ queryKey: ['runs', agentId] })
+          }
+        } catch { /* swallow — keep button clickable */ }
+      }}
+      className="flex items-center gap-1.5 text-xs text-red-700 hover:text-red-900 bg-red-50 hover:bg-red-100 border border-red-200 rounded-lg px-3 py-1.5 transition-colors"
+      title={`此 run 后端仍在运行 — 点击立即终止 (${runId})`}
+    >
+      <Square size={11} />
+      停止此 run
+      <span className="text-red-400 font-mono">{runId.slice(-8)}</span>
+      <span className="ml-1 inline-block w-1.5 h-1.5 rounded-full bg-red-500 animate-pulse" />
+    </button>
+  )
+}
+
+
 function MessageBubble({
   msg, agentId, apiKey,
   onContinue,
@@ -180,9 +243,12 @@ function MessageBubble({
           </div>
         )}
 
-        {/* Workspace file panel + Continue button */}
+        {/* Workspace file panel + Continue button + Stop-if-running */}
         {!isUser && msg.status === 'done' && msg.runId && (
           <div className="space-y-1 w-full">
+            {/* Renders only when run.status==='running' in DB; auto-hides
+                when the run reaches a terminal state. Polls every 5s. */}
+            <RunStatusControl agentId={agentId} runId={msg.runId} apiKey={apiKey} />
             <WorkspacePanel agentId={agentId} runId={msg.runId} apiKey={apiKey} />
             {onContinue && (
               <button
@@ -496,6 +562,18 @@ export default function Chat() {
   const bottomRef = useRef<HTMLDivElement>(null)
   const textareaRef = useRef<HTMLTextAreaElement>(null)
 
+  // audit/AUDIT_2026-06-05.md #15: navigating away from /chat/:agentId
+  // while an SSE stream is in flight used to leave the backend run alive
+  // — the agent thread kept spawning claude subprocesses (= real money on
+  // Anthropic) and the React component kept calling setMessages on an
+  // unmounted tree. The Stop-button path already calls abortRef.current
+  // (which POSTs /cancel + closes the EventSource), so we just need the
+  // same cleanup to fire on unmount. Empty deps array → runs exactly
+  // once, and the returned function runs when the page is gone.
+  useEffect(() => {
+    return () => { abortRef.current?.() }
+  }, [])
+
   const { data: agent } = useQuery({
     queryKey: ['agents', agentId],
     queryFn: () => agentsApi.get(agentId!),