feat(agent-spawn): fluid runtime sub-agent spawning (#199)

[Iftach-Shoham]

Summary

Closes #199

Scoped PR for fluid ad-hoc sub-agent spawning at runtime. Spawned agents use existing workspace text file tools (read_file / read_text). Multimodal image tools, Anthropic platform extras, and unrelated follow-ons are out of scope for this PR.

In scope

• SpawnAgentRuntime with fluid ad-hoc spawning (spawn_agent / get_agent_result)
• Depth limits, spawn_id propagation, SubAgent stream events through AgentLoop
• Skills wiring needed for spawn (inherit_parent_tools, skill tools in parent context)
• Frontend handling of SubAgent SSE events
• Prompt / guardrail updates for the fluid spawning model
• Removal of legacy AGENT.md / agents: SKILL.md infrastructure

Out of scope (deferred)

• Vision / image attach (hybrid Claude + OpenCode multi-provider design — follow-up)
• analyze_image / read_image, pdf_to_images, pptx_to_images
• Anthropic platform TOML / dedicated Anthropic SDK path added on this branch
• Unrelated provider presets bundled earlier for multimodal experiments

Testing

• uv run pytest tests/unit/test_agent_spawn.py tests/unit/test_token_counter.py (50 passed)
• Reviewer: spawn a subagent and confirm it can read a workspace .txt via existing FS tools
• Reviewer: confirm no multimodal / image conversion tools are injected in prepare_node

References

• Closes [Feature] Skills should be able to spawn sub-agents at runtime via use_sub_agents #199 — Skills should be able to spawn sub-agents at runtime
• Incorporates skills/workspace fixes synced from feat: configurable skills root, workspace uploads, and SDK skill discovery #366 / [Bug]: Skills, workspace, and sandbox integration defects (Epic #91) #390 where relevant to spawn

Summary by CodeRabbit

• New Features

  • Added configurable sub-agent spawning with sync/async modes, workspace sharing controls, and future-based result retrieval.
  • Chat UI now streams and groups sub-agent progress/results in-turn.
  • Sandbox execution environment details are now shown in guidance.
  • Increased maximum input length to 20,000 characters.
  • Skills can be enabled independently of shell access.

• Bug Fixes

  • Improved product/date/phone matching and API error handling; clarified empty filesystem search results and safer path normalization.
  • Model context lookup is now capitalization-insensitive.

• Documentation

  • Expanded prompts with robust data-matching, safe spawn/follow-up construction, and reflection prerequisite/error-handling rules.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c6797410-14d4-4999-b207-63f29b1bc54d

📥 Commits

Reviewing files that changed from the base of the PR and between ef4e32a and 25866f2.

📒 Files selected for processing (4)

• .secrets.baseline
• src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/filesystem/tests/test_workspace_fs.py
• tests/unit/test_agent_spawn.py
• tests/unit/test_skill_loader.py

🚧 Files skipped from review as they are similar to previous changes (3)

• src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/filesystem/tests/test_workspace_fs.py
• tests/unit/test_skill_loader.py
• tests/unit/test_agent_spawn.py

---

📝 Walkthrough

Walkthrough

This PR adds configurable synchronous and asynchronous sub-agent spawning, shared future tracking, graph and prompt integration, streamed event forwarding, frontend reasoning rendering, and tests. It also updates filesystem output, prompt guidance, model lookup, server wiring, logging, and repository metadata.

Changes

Agent spawning

Layer / File(s)	Summary
Spawn runtime and tool contracts src/cuga/backend/agent_spawn/*, src/cuga/config.py, src/cuga/settings.toml, tests/unit/test_agent_spawn.py	Adds spawn execution, depth limits, async futures, result polling, callbacks, workspace resolution, configuration, exports, and runtime tests.
Graph, skills, and prompt wiring src/cuga/backend/cuga_graph/nodes/cuga_lite/..., src/cuga/backend/server/workspace_sandbox.py, src/cuga/backend/skills/*, tests/unit/test_skill_loader.py	Shares futures through the graph, registers inherited skill and spawn tools, adds sandbox metadata, and renders sub-agent instructions.
Sub-agent event streaming and rendering src/cuga/backend/cuga_graph/utils/agent_loop.py, src/frontend_workspaces/frontend/src/carbon-chat/*	Merges spawn events with graph streaming and renders accumulated sub-agent activity as nested reasoning steps.

Runtime and prompt adjustments

Layer / File(s)	Summary
Operational prompt behavior src/cuga/backend/cuga_graph/nodes/api/code_agent/prompts/*, src/cuga/backend/cuga_graph/nodes/cuga_lite/reflection/prompts/*, src/cuga/backend/cuga_graph/nodes/task_decomposition_planning/*	Adds defensive matching, prerequisite verification, API-error handling, multi-step completion, and retry guidance.
Filesystem output and command handling src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/filesystem/*	Restricts backslash normalization to recognized workspace paths and returns an empty string for searches with no matches.
Model and server behavior src/cuga/backend/cuga_graph/utils/token_counter.py, src/cuga/backend/server/main.py	Makes model context lookup case-insensitive, decouples skill enablement from shell enablement, and conditionally mounts the A2A router.
Logging and safe task construction src/cuga/backend/cuga_graph/policy/*, src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/common/security.py	Reduces sensitive debug logging and expands safe report and spawn-task construction guidance.
Repository metadata and validation support .gitignore, .secrets.baseline, tests/unit/test_skill_loader.py	Updates ignore rules, the secrets baseline, and skill-loader coverage and descriptions.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

• [Feature] Skills should be able to spawn sub-agents at runtime via use_sub_agents #199 — Adds runtime sub-agent tools and integrates them with skill execution.

Possibly related PRs

• cuga-project/cuga-agent#121 — Modifies the same skills modules and tool-loading paths.
• cuga-project/cuga-agent#125 — Modifies the shared CugaLite prompt construction pipeline.
• cuga-project/cuga-agent#434 — Overlaps with the SecurityValidator syntax-validation guidance.

Suggested labels: complexity: high, readability: fair

Suggested reviewers: sami-marreed, offerakrabi

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.65% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: runtime sub-agent spawning in the agent-spawn feature.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/199-spawn-sub-agents-at-runtime

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/cuga/backend/skills/loader.py (1)

149-185: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Wire the skills cache into discover_skills() or remove it
_discover_skills_cache is never read or populated, so clear_skills_cache() clears dead state and every call still rescans the filesystem. Either hook the cache into discover_skills() or drop both helpers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/skills/loader.py` around lines 149 - 185, The skills cache
is dead state because discover_skills() never reads or populates
_discover_skills_cache, so clear_skills_cache() only clears unused data. Update
discover_skills() to use _discover_skills_cache for repeated calls with the same
arguments (including cuga_folder, global_skills_root, legacy_global_skills_root,
and root), or remove both _discover_skills_cache and clear_skills_cache()
entirely if caching is not intended.

src/cuga/backend/agent_spawn/runtime.py (1)

172-189: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Keep a strong reference to the spawned task asyncio.create_task(...) should be retained until completion; otherwise it can be garbage-collected mid-flight and leave _spawn_futures[future_id] stuck in "running". Store it in a collection on the runtime and drop it in a done callback.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/agent_spawn/runtime.py` around lines 172 - 189, The spawned
asyncio task in execute_async is created without keeping a strong reference,
which can leave _spawn_futures entries stuck in running if the task is
garbage-collected. Update AgentSpawnRuntime to store the task returned by
asyncio.create_task in an instance collection keyed by future_id, and add a done
callback in _execute_and_store or the creation path to remove it once finished.

♻️ Duplicate comments (1)

src/cuga/backend/tools/pptx_to_images.py (1)

263-267: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Same unhandled-exception gap as pdf_to_images.py.

_pdf_to_jpegs only catches ImportError around _via_pymupdf_pdf; other exceptions (e.g. malformed PDF) will propagate instead of returning an "ERROR: ..." string as elsewhere in this module. Apply the same fix used for pdf_to_images.py's _convert_pymupdf.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/tools/pptx_to_images.py` around lines 263 - 267, The
_pdf_to_jpegs helper only handles ImportError around _via_pymupdf_pdf, so other
failures can escape instead of returning an error string. Update _pdf_to_jpegs
in pptx_to_images.py to follow the same pattern as pdf_to_images.py’s
_convert_pymupdf: wrap the PyMuPDF conversion call in a broader exception
handler, log or capture the exception context, and return an "ERROR: ..." string
for any conversion failure while preserving the ImportError fallback behavior.

🧹 Nitpick comments (9)

src/cuga/backend/tools/image_analysis.py (2)

185-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Prefer asyncio.get_running_loop() over get_event_loop().

asyncio.get_event_loop() is deprecated when called from within a running coroutine (as is the case here); get_running_loop() is the idiomatic replacement and avoids future breakage.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/tools/image_analysis.py` around lines 185 - 186, Replace the
use of asyncio.get_event_loop() in image_analysis.py with
asyncio.get_running_loop() inside the coroutine that builds the litellm
completion call; the current loop variable in the run_in_executor path should
come from get_running_loop() so the executor invocation in that function remains
the same while avoiding the deprecated API.

---

116-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log the swallowed exception instead of silently passing.

Static analysis (Ruff S110/BLE001) flags this bare except Exception: pass. Since the impact is minor here (best-effort skip heuristic), logging at debug level would aid troubleshooting without changing behavior.

🪵 Suggested fix

-    except Exception:
-        pass
+    except Exception as exc:
+        logger.debug(f"analyze_image: could not check primary model name: {exc}")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/tools/image_analysis.py` around lines 116 - 127, The bare
exception handler in analyze_image() is swallowing errors silently; update the
try/except around the cuga.config settings lookup to log the caught exception at
debug level instead of passing. Keep the best-effort skip heuristic behavior
unchanged, but use the existing logger in image_analysis.py to record the
exception context so troubleshooting is possible while preserving the fallback
flow.

Source: Linters/SAST tools

src/cuga/backend/cuga_graph/nodes/api/code_agent/prompts/system_accurate.jinja2 (1)

119-119: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Blind retry on non-idempotent SMS call may risk duplicate sends.

The instruction retries an SMS/phone call with an alternate format on any 4xx, assuming the original call had no side effect. This is safe only if 4xx guarantees the message was never sent by the provider — not true for every SMS API surface. Consider adding a caveat to only retry for error types that explicitly confirm non-delivery (e.g. "user does not exist"/"invalid number"), not all 4xx codes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/cuga/backend/cuga_graph/nodes/api/code_agent/prompts/system_accurate.jinja2`
at line 119, The phone-number retry guidance in the prompt is too broad because
it applies to any 4xx and could trigger duplicate SMS/phone sends. Tighten the
instruction in the system_accurate prompt so the retry logic only applies in the
code path handling explicit non-delivery errors such as “user does not exist” or
“invalid number,” and keep the alternate-format retry scoped to the existing
SMS/phone API handling. Preserve the separate retry result variable name, but
make the condition for retry explicit and narrow.

tests/unit/test_analyze_image_tool.py (1)

1-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

LGTM! Solid coverage of URL/path helpers and primary/fallback flow.

One gap worth considering: no test exercises the _skip_primary=True path (when the primary model name matches _KNOWN_NON_VISION_PATTERNS), which skips straight to the fallback model.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_analyze_image_tool.py` around lines 1 - 192, Add a unit test
that covers the analyze_image path where _skip_primary is True because the
configured primary model matches one of the _KNOWN_NON_VISION_PATTERNS, so the
code bypasses the primary LLM and goes directly to the fallback completion path.
Use analyze_image and the existing fallback setup in test_analyze_image_tool.py,
but configure IMAGE_ANALYSIS_MODEL to a known non-vision value and assert that
the primary model’s ainvoke is not called while the fallback response is
returned.

src/cuga/backend/skills/tools.py (1)

60-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid reaching into registry._by_name from outside the class.

Accessing a private attribute couples this module to SkillRegistry's internal representation. Prefer a public accessor (e.g. registry.entries()/registry.all()) if the registry doesn't already expose one.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/skills/tools.py` around lines 60 - 64, Accessing
registry._by_name in the tool-loading path couples tools.py to SkillRegistry
internals; update the callable-tools collection logic to use a public registry
accessor instead. Add or use an existing method on SkillRegistry such as
entries() or all(), then change the loop in the tool-building flow around
_build_callable_tools_from_entry and the return composition so it iterates over
that public API rather than the private _by_name attribute.

src/cuga/backend/tools/pdf_to_images.py (1)

24-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate path-resolution and PDF→JPEG conversion logic vs. pptx_to_images.py.

_resolve, _WORKSPACE_ROOTS, _convert_pymupdf/_via_pymupdf_pdf, and _convert_pdftoppm/_via_pdftoppm are near-identical copies between this file and pptx_to_images.py. Consider extracting a shared helper module to avoid drift between the two implementations.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/tools/pdf_to_images.py` around lines 24 - 124, The path
resolution and PDF-to-image conversion code in pdf_to_images mirrors the logic
in pptx_to_images, so keep the shared behavior in one place to prevent drift.
Extract the duplicated _WORKSPACE_ROOTS, _resolve, and backend conversion flow
used by pdf_to_images, _convert_pymupdf, and _convert_pdftoppm into a common
helper module, then have both pdf_to_images and pptx_to_images call that shared
code. Preserve the existing public behavior and backend selection, but remove
the near-identical copies from each file.

src/cuga/backend/agent_spawn/tools.py (1)

52-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use asyncio.get_running_loop() instead of asyncio.get_event_loop().

Per current Python docs, get_running_loop() is preferred over get_event_loop() inside coroutines/callbacks. get_event_loop() is deprecated when no loop is set and has surprising behavior in future Python versions.

♻️ Proposed fix

-        deadline = asyncio.get_event_loop().time() + timeout
-        while asyncio.get_event_loop().time() < deadline:
+        loop = asyncio.get_running_loop()
+        deadline = loop.time() + timeout
+        while loop.time() < deadline:

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/agent_spawn/tools.py` around lines 52 - 53, Update the loop
timing logic in the timeout handling inside the coroutine to use
asyncio.get_running_loop() instead of asyncio.get_event_loop(). In the code that
sets deadline and checks the loop time in the while condition, keep using the
running loop instance consistently so the behavior is future-proof and matches
current asyncio guidance.

src/frontend_workspaces/frontend/src/carbon-chat/customLoadHistory.ts (1)

139-139: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Silent JSON parse failure defaults to empty object.

If actualData fails to parse, subAgentData stays {}, and the event is silently dropped (no newIteration matches, break at Line 166) with no logging. Acceptable given console.error is used elsewhere for failures, but worth a debug log for troubleshooting malformed SSE payloads.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend_workspaces/frontend/src/carbon-chat/customLoadHistory.ts` at
line 139, The JSON parsing in customLoadHistory is swallowing malformed SSE
payloads and leaving subAgentData as an empty object, which can silently skip
the newIteration handling. Update the try/catch around JSON.parse(actualData) to
log the parse failure with console.error (or the existing failure logging
pattern used in this file) and include enough context to identify the bad
payload, while keeping the current fallback behavior in customLoadHistory and
the newIteration matching logic intact.

src/cuga/config.py (1)

209-214: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider adding type/bound constraints to max_spawn_depth.

Neighboring validators (e.g., sandbox_execution_timeout) enforce is_type_of=int, gt=0, but agent_spawn.max_spawn_depth has no such constraint. A misconfigured negative or non-integer value could bypass recursion-depth protection in the spawn runtime.

🛡️ Proposed fix

-    Validator("agent_spawn.max_spawn_depth", default=2),
+    Validator("agent_spawn.max_spawn_depth", default=2, is_type_of=int, gt=0),

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/config.py` around lines 209 - 214, Add type and lower-bound
validation to the `Validator("agent_spawn.max_spawn_depth", ...)` entry in
`src/cuga/config.py` so it matches the stricter pattern used by nearby
validators like `sandbox_execution_timeout`. Update the
`agent_spawn.max_spawn_depth` validator to require an integer greater than 0,
keeping the default as the current positive value, so invalid negative or
non-integer settings are rejected during config validation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cuga/backend/agent_spawn/tools.py`:
- Around line 38-47: The sync path in spawn_agent currently awaits
rt.execute(task) without any bound, so a hung subagent can block the parent
indefinitely. Update the spawn_agent function to enforce a timeout for the mode
!= "async" branch, using a bounded await or timeout wrapper around
SpawnAgentRuntime.execute while preserving the existing async branch behavior.
Keep the fix localized to spawn_agent and, if needed, align the timeout
source/handling with the existing get_agent_result polling limits so both
execution paths fail predictably.

In `@src/cuga/backend/cuga_graph/utils/agent_loop.py`:
- Around line 731-797: The spawn callback in _feed_graph/_on_spawn_event is
using the process-global _spawn_runtime._event_callback, which lets concurrent
run_stream calls interfere with each other. Move the callback routing in
agent_loop.py to request-local storage, such as a ContextVar or a stream-ID
keyed registry tied to run_stream/self.thread_id, and have _on_spawn_event
resolve the current stream’s queue from that local context before calling
put_nowait. Make sure the cleanup in the finally block only clears the callback
for the current request and does not affect other active streams.

In `@src/cuga/backend/llm/models.py`:
- Around line 682-686: In the Claude proxy branch inside models.py, update the
thinking-request params to match Anthropic’s requirements: when model_name
starts with claude-, set temperature to 1.0 in openai_params alongside the
existing extra_body thinking config, and compute budget_tokens so it never
reaches or exceeds max_tokens while still honoring the native Anthropic minimum
threshold. Adjust the logic around configured_budget and the fallback budget
calculation in this branch so the resulting budget is clamped below max_tokens
for small limits, and keep the change localized to the Claude-specific
openai_params["model_kwargs"] setup.

In `@src/cuga/backend/skills/tools.py`:
- Around line 20-43: The per-tool error handling in
_build_callable_tools_from_entry only catches ImportError, so bad skill
definitions can still abort create_skill_tools for all entries. Broaden the
try/except around the per-entry processing in _build_callable_tools_from_entry
to cover invalid module values, missing attributes, and failures from
StructuredTool.from_function, and log/continue for each offending raw definition
instead of letting the exception escape. Keep the isolation localized to this
helper so one bad SkillEntry.tool_definitions item does not break the rest of
the skill tools.

In `@src/cuga/backend/tools/image_analysis.py`:
- Around line 66-89: The _build_data_url helper currently fetches any HTTP(S)
source and does so synchronously, which can both reach private/internal hosts
and block the event loop when called from analyze_image. Update _build_data_url
to validate the requested host/IP before urllib.request.urlopen (reject
private/link-local/loopback and other disallowed destinations), add a request
timeout, and then refactor analyze_image to invoke the download work off the
event loop via an executor or async-friendly I/O while keeping the existing
_build_data_url and analyze_image flow intact.
- Around line 54-63: The path handling in _resolve_path and the fetch logic in
_build_data_url are too permissive for analyze_image. Update _resolve_path to
only accept files that resolve inside the workspace root and explicitly reject
absolute paths outside it or any traversal via ../ after normalizing the path.
In _build_data_url, if HTTPS fetching remains supported, add a request timeout
and restrict remote hosts with an allowlist before reading image bytes, keeping
the behavior aligned with the workspace-only boundary enforced by analyze_image.

In `@src/cuga/backend/tools/pdf_to_images.py`:
- Around line 63-105: The pdf_to_images flow only handles ImportError around
_convert_pymupdf, so other failures from fitz.open or page.get_pixmap on bad
PDFs escape instead of returning the function’s error-string contract. Update
pdf_to_images to catch the broader conversion exceptions around
_convert_pymupdf, and convert those failures into an "ERROR: ..." response
consistent with the existing pdf_path and backend fallback paths, using the
_convert_pymupdf and pdf_to_images symbols to locate the change.

In `@src/cuga/backend/tools/pptx_to_images.py`:
- Around line 79-197: The LD_PRELOAD shim uses a predictable shared-temp
filename and trusts any existing .so, which can allow arbitrary local code
execution. Update _ensure_shim() and _SHIM_SO to use a unique, process-owned
temporary file (for both the .c source and compiled .so), and verify
ownership/permissions before reusing any existing shim. If possible, avoid the
runtime-compiled shim in _soffice_env() entirely and handle the AF_UNIX
restriction at the sandbox/container level instead.

In `@src/cuga/settings.toml`:
- Around line 96-101: Update the skills-root default documentation so it matches
the actual default in settings.toml. In the settings root configuration comments
near root = "agents" and any related loader docs that describe the default,
change the wording from cuga being the default to agents being the default, and
keep the path examples aligned with the current behavior in the skills-root
loading/config code.

In `@src/frontend_workspaces/frontend/src/carbon-chat/customLoadHistory.ts`:
- Line 84: The keying in customLoadHistory is still merging sub-agent reasoning
by agent_name instead of spawn_id, which can combine concurrent same-name runs
into one step. Update the accumulator and step lookup in customLoadHistory to
use spawn_id as the primary key, with agent_name only as a fallback when
spawn_id is missing, and make the same keying change in the live-stream handling
path so task/result/code blocks stay separated per spawn.

---

Outside diff comments:
In `@src/cuga/backend/agent_spawn/runtime.py`:
- Around line 172-189: The spawned asyncio task in execute_async is created
without keeping a strong reference, which can leave _spawn_futures entries stuck
in running if the task is garbage-collected. Update AgentSpawnRuntime to store
the task returned by asyncio.create_task in an instance collection keyed by
future_id, and add a done callback in _execute_and_store or the creation path to
remove it once finished.

In `@src/cuga/backend/skills/loader.py`:
- Around line 149-185: The skills cache is dead state because discover_skills()
never reads or populates _discover_skills_cache, so clear_skills_cache() only
clears unused data. Update discover_skills() to use _discover_skills_cache for
repeated calls with the same arguments (including cuga_folder,
global_skills_root, legacy_global_skills_root, and root), or remove both
_discover_skills_cache and clear_skills_cache() entirely if caching is not
intended.

---

Duplicate comments:
In `@src/cuga/backend/tools/pptx_to_images.py`:
- Around line 263-267: The _pdf_to_jpegs helper only handles ImportError around
_via_pymupdf_pdf, so other failures can escape instead of returning an error
string. Update _pdf_to_jpegs in pptx_to_images.py to follow the same pattern as
pdf_to_images.py’s _convert_pymupdf: wrap the PyMuPDF conversion call in a
broader exception handler, log or capture the exception context, and return an
"ERROR: ..." string for any conversion failure while preserving the ImportError
fallback behavior.

---

Nitpick comments:
In `@src/cuga/backend/agent_spawn/tools.py`:
- Around line 52-53: Update the loop timing logic in the timeout handling inside
the coroutine to use asyncio.get_running_loop() instead of
asyncio.get_event_loop(). In the code that sets deadline and checks the loop
time in the while condition, keep using the running loop instance consistently
so the behavior is future-proof and matches current asyncio guidance.

In
`@src/cuga/backend/cuga_graph/nodes/api/code_agent/prompts/system_accurate.jinja2`:
- Line 119: The phone-number retry guidance in the prompt is too broad because
it applies to any 4xx and could trigger duplicate SMS/phone sends. Tighten the
instruction in the system_accurate prompt so the retry logic only applies in the
code path handling explicit non-delivery errors such as “user does not exist” or
“invalid number,” and keep the alternate-format retry scoped to the existing
SMS/phone API handling. Preserve the separate retry result variable name, but
make the condition for retry explicit and narrow.

In `@src/cuga/backend/skills/tools.py`:
- Around line 60-64: Accessing registry._by_name in the tool-loading path
couples tools.py to SkillRegistry internals; update the callable-tools
collection logic to use a public registry accessor instead. Add or use an
existing method on SkillRegistry such as entries() or all(), then change the
loop in the tool-building flow around _build_callable_tools_from_entry and the
return composition so it iterates over that public API rather than the private
_by_name attribute.

In `@src/cuga/backend/tools/image_analysis.py`:
- Around line 185-186: Replace the use of asyncio.get_event_loop() in
image_analysis.py with asyncio.get_running_loop() inside the coroutine that
builds the litellm completion call; the current loop variable in the
run_in_executor path should come from get_running_loop() so the executor
invocation in that function remains the same while avoiding the deprecated API.
- Around line 116-127: The bare exception handler in analyze_image() is
swallowing errors silently; update the try/except around the cuga.config
settings lookup to log the caught exception at debug level instead of passing.
Keep the best-effort skip heuristic behavior unchanged, but use the existing
logger in image_analysis.py to record the exception context so troubleshooting
is possible while preserving the fallback flow.

In `@src/cuga/backend/tools/pdf_to_images.py`:
- Around line 24-124: The path resolution and PDF-to-image conversion code in
pdf_to_images mirrors the logic in pptx_to_images, so keep the shared behavior
in one place to prevent drift. Extract the duplicated _WORKSPACE_ROOTS,
_resolve, and backend conversion flow used by pdf_to_images, _convert_pymupdf,
and _convert_pdftoppm into a common helper module, then have both pdf_to_images
and pptx_to_images call that shared code. Preserve the existing public behavior
and backend selection, but remove the near-identical copies from each file.

In `@src/cuga/config.py`:
- Around line 209-214: Add type and lower-bound validation to the
`Validator("agent_spawn.max_spawn_depth", ...)` entry in `src/cuga/config.py` so
it matches the stricter pattern used by nearby validators like
`sandbox_execution_timeout`. Update the `agent_spawn.max_spawn_depth` validator
to require an integer greater than 0, keeping the default as the current
positive value, so invalid negative or non-integer settings are rejected during
config validation.

In `@src/frontend_workspaces/frontend/src/carbon-chat/customLoadHistory.ts`:
- Line 139: The JSON parsing in customLoadHistory is swallowing malformed SSE
payloads and leaving subAgentData as an empty object, which can silently skip
the newIteration handling. Update the try/catch around JSON.parse(actualData) to
log the parse failure with console.error (or the existing failure logging
pattern used in this file) and include enough context to identify the bad
payload, while keeping the current fallback behavior in customLoadHistory and
the newIteration matching logic intact.

In `@tests/unit/test_analyze_image_tool.py`:
- Around line 1-192: Add a unit test that covers the analyze_image path where
_skip_primary is True because the configured primary model matches one of the
_KNOWN_NON_VISION_PATTERNS, so the code bypasses the primary LLM and goes
directly to the fallback completion path. Use analyze_image and the existing
fallback setup in test_analyze_image_tool.py, but configure IMAGE_ANALYSIS_MODEL
to a known non-vision value and assert that the primary model’s ainvoke is not
called while the fallback response is returned.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a03727a5-4288-4a4a-b99f-442375989a66

📥 Commits

Reviewing files that changed from the base of the PR and between 0106d13 and b57e2e1.

⛔ Files ignored due to path filters (8)

• package-lock.json is excluded by !**/package-lock.json
• src/cuga/frontend/dist/background.js is excluded by !**/dist/**
• src/cuga/frontend/dist/main.692684707f9dd85a3759.js is excluded by !**/dist/**
• src/cuga/frontend/dist/main.692684707f9dd85a3759.js.map is excluded by !**/dist/**, !**/*.map
• src/cuga/frontend/dist/tailwind.js is excluded by !**/dist/**
• src/cuga/frontend/dist/vendors.b64eb4e72b714154cad2.js is excluded by !**/dist/**
• src/cuga/frontend/dist/vendors.b64eb4e72b714154cad2.js.map is excluded by !**/dist/**, !**/*.map
• uv.lock is excluded by !**/*.lock

📒 Files selected for processing (40)

• .gitignore
• package.json
• pyproject.toml
• src/cuga/backend/agent_spawn/__init__.py
• src/cuga/backend/agent_spawn/prompt_utils.py
• src/cuga/backend/agent_spawn/runtime.py
• src/cuga/backend/agent_spawn/tools.py
• src/cuga/backend/cuga_graph/nodes/api/code_agent/prompts/system_accurate.jinja2
• src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/graph_adapter.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/cuga_lite_graph.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/prompt_utils.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/prompts/mcp_prompt.jinja2
• src/cuga/backend/cuga_graph/nodes/cuga_lite/reflection/prompts/reflection_system.jinja2
• src/cuga/backend/cuga_graph/nodes/task_decomposition_planning/plan_controller_agent/prompts/system.jinja2
• src/cuga/backend/cuga_graph/utils/agent_loop.py
• src/cuga/backend/cuga_graph/utils/token_counter.py
• src/cuga/backend/llm/models.py
• src/cuga/backend/server/main.py
• src/cuga/backend/server/workspace_sandbox.py
• src/cuga/backend/skills/__init__.py
• src/cuga/backend/skills/loader.py
• src/cuga/backend/skills/registry.py
• src/cuga/backend/skills/tools.py
• src/cuga/backend/tools/__init__.py
• src/cuga/backend/tools/image_analysis.py
• src/cuga/backend/tools/pdf_to_images.py
• src/cuga/backend/tools/pptx_to_images.py
• src/cuga/cli/main.py
• src/cuga/config.py
• src/cuga/configurations/models/settings.anthropic.toml
• src/cuga/configurations/models/settings.rits.gemma4.toml
• src/cuga/configurations/models/settings.rits.gptoss.toml
• src/cuga/settings.toml
• src/frontend_workspaces/frontend/src/carbon-chat/customLoadHistory.ts
• src/frontend_workspaces/frontend/src/carbon-chat/customSendMessage.ts
• tests/fixtures/__init__.py
• tests/unit/test_agent_spawn.py
• tests/unit/test_analyze_image_tool.py
• tests/unit/test_skill_loader.py

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

No timeout on sync-mode spawn_agent.

await rt.execute(task) in sync mode has no timeout, unlike get_agent_result's bounded polling for async mode. If the spawned subagent hangs (tool call blocks, infinite reasoning loop, etc.), the parent agent's tool call — and potentially the whole turn — blocks indefinitely with no way to recover.

🛡️ Proposed fix — bound sync execution with a timeout

+import asyncio
+
     async def spawn_agent(task: str = "", mode: str = "sync") -> str:
         rt = SpawnAgentRuntime.from_parent(
             parent_config,
             spawn_futures_ref=spawn_futures,
             parent_structured_tools=parent_structured_tools,
         )

         if mode == "async":
             return await rt.execute_async(task)
-        return await rt.execute(task)
+        try:
+            return await asyncio.wait_for(rt.execute(task), timeout=SYNC_SPAWN_TIMEOUT)
+        except asyncio.TimeoutError:
+            return f"[SpawnTimeout] Subagent did not complete within {SYNC_SPAWN_TIMEOUT}s"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/agent_spawn/tools.py` around lines 38 - 47, The sync path in
spawn_agent currently awaits rt.execute(task) without any bound, so a hung
subagent can block the parent indefinitely. Update the spawn_agent function to
enforce a timeout for the mode != "async" branch, using a bounded await or
timeout wrapper around SpawnAgentRuntime.execute while preserving the existing
async branch behavior. Keep the fix localized to spawn_agent and, if needed,
align the timeout source/handling with the existing get_agent_result polling
limits so both execution paths fail predictably.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='src/cuga/backend/llm/models.py'

echo '== Nearby lines around 660-700 =='
sed -n '660,700p' "$file"

echo
echo '== Native anthropic branch around 890-920 =='
sed -n '890,920p' "$file"

echo
echo '== Search for temperature assignments and thinking budget handling =='
rg -n 'openai_params\["temperature"\]|thinking_budget_tokens|budget_tokens|thinking' "$file"

Repository: cuga-project/cuga-agent

Length of output: 5053

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='src/cuga/backend/llm/models.py'

echo '== Context around 620-640 =='
sed -n '620,640p' "$file"

echo
echo '== OpenAI/LangChain client instantiation around 700-780 =='
sed -n '700,780p' "$file"

echo
echo '== Search for ReasoningChatOpenAI or ChatOpenAI wrappers =='
rg -n 'ReasoningChatOpenAI|ChatOpenAI\(' src/cuga/backend/llm/models.py src/cuga/backend -g '!**/__pycache__/**'

Repository: cuga-project/cuga-agent

Length of output: 6114

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='src/cuga/backend/llm/models.py'

echo '== Top of file / ReasoningChatOpenAI definition =='
sed -n '1,120p' "$file"

echo
echo '== Search for 1.0 temperature or Claude handling in file =='
rg -n 'temperature\s*=\s*1\.0|temperature.*claude|claude.*temperature|thinking.*budget|max_tokens - 1000|ReasoningChatOpenAI' "$file"

Repository: cuga-project/cuga-agent

Length of output: 5472

---

🌐 Web query:

Anthropic thinking budget_tokens must be less than max_tokens temperature may only be set to 1 when thinking is enabled docs

💡 Result:

When using extended thinking (manual mode via thinking: {type: "enabled", ...}) in the Anthropic Messages API, you must adhere to specific configuration requirements: 1. Budget vs. Max Tokens: The budget_tokens parameter must be set to a value less than max_tokens [1][2][3]. This is because max_tokens serves as a hard limit on the total output, which encompasses both the internal thinking tokens and the final response text [4][5]. Exceptions exist for interleaved thinking with tools, where the token limit can represent the entire context window [1][2]. 2. Temperature Requirements: On models where manual extended thinking is supported (e.g., Claude Opus 4.6, Claude Sonnet 4.6), the temperature parameter must be set to 1.0 when extended thinking is enabled [3][6]. The API will reject requests with non-default temperature values when thinking is active [7]. 3. Model Support and Migration: Manual extended thinking (type: "enabled") is deprecated on Claude Opus 4.6 and Claude Sonnet 4.6 and is not supported on newer models like Claude Fable 5, Claude Mythos 5, and Claude Opus 4.7/4.8, which instead use type: "adaptive" with an effort parameter [4][5][8]. Newer models also strictly reject non-default temperature, top_p, and top_k values for all requests, regardless of whether thinking is active [4][8]. Always consult the latest official Anthropic documentation for the specific model you are using, as requirements and support for "manual" vs. "adaptive" thinking modes evolve [4][8].

Citations:

• 1: https://doc.jarvisuni.com/claude/api/en/build-with-claude/extended-thinking.html
• 2: https://claude.yourdocs.dev/docs/build-with-claude/extended-thinking
• 3: https://tools.yiteai.com/en/books/claude-guide/ch07
• 4: https://platform.claude.com/docs/en/build-with-claude/adaptive-thinking?fcdaa149_sort_date=desc&vno=z7
• 5: https://platform.claude.com/docs/en/build-with-claude/adaptive-thinking?38d7aa68_page=2&5a5b91bc_page=1
• 6: https://platform.claude.com/docs/en/claude_api_primer
• 7: https://likeone.ai/blog/claude-extended-thinking-guide/
• 8: https://platform.claude.com/docs/en/build-with-claude/adaptive-thinking.md

---

Align Claude proxy thinking params with Anthropic’s constraints

• Set temperature to 1.0 in the OpenAI-proxy Claude branch; leaving it unset sends the client default instead of the value Anthropic expects for thinking requests.
• Clamp budget_tokens below max_tokens and mirror the native Anthropic threshold; max(1024, int(max_tokens * 0.6)) can produce an invalid budget for small limits.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/llm/models.py` around lines 682 - 686, In the Claude proxy
branch inside models.py, update the thinking-request params to match Anthropic’s
requirements: when model_name starts with claude-, set temperature to 1.0 in
openai_params alongside the existing extra_body thinking config, and compute
budget_tokens so it never reaches or exceeds max_tokens while still honoring the
native Anthropic minimum threshold. Adjust the logic around configured_budget
and the fallback budget calculation in this branch so the resulting budget is
clamped below max_tokens for small limits, and keep the change localized to the
Claude-specific openai_params["model_kwargs"] setup.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Broaden per-entry exception handling so one bad skill definition can't break all skill tools.

Only ImportError is caught around importlib.import_module. A malformed module field (e.g. empty string) raises ValueError instead, and a non-callable fn will raise inside StructuredTool.from_function — both propagate out of create_skill_tools uncaught, breaking tool creation for every skill, not just the offending one.

🛡️ Proposed fix to isolate per-entry failures

         try:
             mod = importlib.import_module(module_path)
-        except ImportError as exc:
+        except (ImportError, ValueError) as exc:
             logger.warning(f"skill {entry.name!r}: cannot import {module_path!r}: {exc}")
             continue
         fn = getattr(mod, fn_name, None)
-        if fn is None:
+        if not callable(fn):
             logger.warning(f"skill {entry.name!r}: {module_path!r} has no attribute {fn_name!r}")
             continue
-        kwargs: dict = {"name": tool_name, "description": description}
-        if asyncio.iscoroutinefunction(fn):
-            kwargs["coroutine"] = fn
-        else:
-            kwargs["func"] = fn
-        out.append(StructuredTool.from_function(**kwargs))
+        kwargs: dict = {"name": tool_name, "description": description}
+        if asyncio.iscoroutinefunction(fn):
+            kwargs["coroutine"] = fn
+        else:
+            kwargs["func"] = fn
+        try:
+            out.append(StructuredTool.from_function(**kwargs))
+        except Exception as exc:
+            logger.warning(f"skill {entry.name!r}: failed to build tool {tool_name!r}: {exc}")
+            continue

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def _build_callable_tools_from_entry(entry: SkillEntry) -> list[StructuredTool]:
	"""Import and wrap callable tools declared in a SkillEntry's tool_definitions."""
	out: list[StructuredTool] = []
	for raw in entry.tool_definitions:
	module_path = raw.get("module", "")
	fn_name = raw.get("function", "")
	tool_name = raw.get("name", "")
	description = raw.get("description", "")
	try:
	mod = importlib.import_module(module_path)
	except ImportError as exc:
	logger.warning(f"skill {entry.name!r}: cannot import {module_path!r}: {exc}")
	continue
	fn = getattr(mod, fn_name, None)
	if fn is None:
	logger.warning(f"skill {entry.name!r}: {module_path!r} has no attribute {fn_name!r}")
	continue
	kwargs: dict = {"name": tool_name, "description": description}
	if asyncio.iscoroutinefunction(fn):
	kwargs["coroutine"] = fn
	else:
	kwargs["func"] = fn
	out.append(StructuredTool.from_function(**kwargs))
	return out
	def _build_callable_tools_from_entry(entry: SkillEntry) -> list[StructuredTool]:
	"""Import and wrap callable tools declared in a SkillEntry's tool_definitions."""
	out: list[StructuredTool] = []
	for raw in entry.tool_definitions:
	module_path = raw.get("module", "")
	fn_name = raw.get("function", "")
	tool_name = raw.get("name", "")
	description = raw.get("description", "")
	try:
	mod = importlib.import_module(module_path)
	except (ImportError, ValueError) as exc:
	logger.warning(f"skill {entry.name!r}: cannot import {module_path!r}: {exc}")
	continue
	fn = getattr(mod, fn_name, None)
	if not callable(fn):
	logger.warning(f"skill {entry.name!r}: {module_path!r} has no attribute {fn_name!r}")
	continue
	kwargs: dict = {"name": tool_name, "description": description}
	if asyncio.iscoroutinefunction(fn):
	kwargs["coroutine"] = fn
	else:
	kwargs["func"] = fn
	try:
	out.append(StructuredTool.from_function(**kwargs))
	except Exception as exc:
	logger.warning(f"skill {entry.name!r}: failed to build tool {tool_name!r}: {exc}")
	continue
	return out

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/skills/tools.py` around lines 20 - 43, The per-tool error
handling in _build_callable_tools_from_entry only catches ImportError, so bad
skill definitions can still abort create_skill_tools for all entries. Broaden
the try/except around the per-entry processing in
_build_callable_tools_from_entry to cover invalid module values, missing
attributes, and failures from StructuredTool.from_function, and log/continue for
each offending raw definition instead of letting the exception escape. Keep the
isolation localized to this helper so one bad SkillEntry.tool_definitions item
does not break the rest of the skill tools.

[coderabbitai]

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== File map ==\n'
git ls-files | rg '^(src/cuga/backend/tools/image_analysis\.py|src/cuga/backend|.*workspace_sandbox.*|.*sandbox.*)$' || true

printf '\n== image_analysis outline ==\n'
ast-grep outline src/cuga/backend/tools/image_analysis.py --view expanded || true

printf '\n== relevant sandbox files ==\n'
fd -a 'workspace_sandbox.py|sandbox' src . 2>/dev/null || true

printf '\n== search for _resolve_path / urlopen / get_event_loop / workspace sandbox ==\n'
rg -n "_resolve_path|urlopen|get_event_loop|workspace_sandbox|sandbox" src/cuga/backend -g '!**/__pycache__/**' || true

Repository: cuga-project/cuga-agent

Length of output: 50380

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== image_analysis.py (selected lines) ==\n'
sed -n '1,240p' src/cuga/backend/tools/image_analysis.py | cat -n

printf '\n== sandbox-related files (if any) ==\n'
fd -a 'workspace_sandbox.py' . 2>/dev/null || true
fd -a 'sandbox*.py' src . 2>/dev/null || true

Repository: cuga-project/cuga-agent

Length of output: 10285

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== image_analysis.py (selected lines) ==\n'
sed -n '1,260p' src/cuga/backend/tools/image_analysis.py | cat -n

printf '\n== search for sandbox enforcement ==\n'
rg -n "chroot|jail|workspace.*sandbox|sandbox.*workspace|thread-scoped|workspace isolation|Path\(source\)|_resolve_path" src -g '!**/__pycache__/**' || true

Repository: cuga-project/cuga-agent

Length of output: 50384

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== tool registration / usage ==\n'
rg -n "create_analyze_image_tool|analyze_image\(" src tests -g '!**/__pycache__/**' | head -n 80

printf '\n== workspace_sandbox containment helpers ==\n'
sed -n '200,340p' src/cuga/backend/server/workspace_sandbox.py | cat -n

printf '\n== opensandbox path normalization ==\n'
sed -n '1,120p' src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/opensandbox/opensandbox_executor.py | cat -n

Repository: cuga-project/cuga-agent

Length of output: 14321

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== image_analysis import/registration sites ==\n'
rg -n "image_analysis|create_analyze_image_tool|analyze_image" src/cuga/backend -g '!**/__pycache__/**' | sed -n '1,120p'

printf '\n== workspace boundary resolver in main.py ==\n'
sed -n '130,170p' src/cuga/backend/server/main.py | cat -n

Repository: cuga-project/cuga-agent

Length of output: 4813

---

Restrict analyze_image to workspace inputs src/cuga/backend/tools/image_analysis.py:54-89

• _resolve_path accepts any existing absolute path and strips /workspace/ without rejecting .. traversal, so this can read files outside the intended workspace boundary.
• _build_data_url also fetches arbitrary HTTPS URLs inline; if remote images are meant to stay supported, add a timeout and an allowlist.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/tools/image_analysis.py` around lines 54 - 63, The path
handling in _resolve_path and the fetch logic in _build_data_url are too
permissive for analyze_image. Update _resolve_path to only accept files that
resolve inside the workspace root and explicitly reject absolute paths outside
it or any traversal via ../ after normalizing the path. In _build_data_url, if
HTTPS fetching remains supported, add a request timeout and restrict remote
hosts with an allowlist before reading image bytes, keeping the behavior aligned
with the workspace-only boundary enforced by analyze_image.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git ls-files src/cuga/backend/tools/image_analysis.py
wc -l src/cuga/backend/tools/image_analysis.py
cat -n src/cuga/backend/tools/image_analysis.py | sed -n '1,240p'

Repository: cuga-project/cuga-agent

Length of output: 9537

---

Block arbitrary remote image fetches

• _build_data_url will fetch any http(s):// target, so this can reach private/internal hosts. Add host/IP filtering and a timeout.
• It is called directly from async def analyze_image, so the download blocks the event loop; move it to the executor or async I/O.

🧰 Tools

🪛 ast-grep (0.44.1)

[warning] 76-76: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(req)
Note: [CWE-918] Server-Side Request Forgery (SSRF).

(urlopen-unsanitized-data)

🪛 Ruff (0.15.20)

[error] 73-76: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.

(S310)

---

[error] 77-77: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.

(S310)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/tools/image_analysis.py` around lines 66 - 89, The
_build_data_url helper currently fetches any HTTP(S) source and does so
synchronously, which can both reach private/internal hosts and block the event
loop when called from analyze_image. Update _build_data_url to validate the
requested host/IP before urllib.request.urlopen (reject
private/link-local/loopback and other disallowed destinations), add a request
timeout, and then refactor analyze_image to invoke the download work off the
event loop via an executor or async-friendly I/O while keeping the existing
_build_data_url and analyze_image flow intact.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Unhandled exceptions from _convert_pymupdf bypass the documented error-string contract.

Only ImportError is caught; a malformed/corrupt PDF makes fitz.open()/get_pixmap() raise other exceptions (e.g. RuntimeError) that propagate out of pdf_to_images, instead of returning the documented "ERROR: ..." string like every other failure path in this function.

🐛 Proposed fix

     try:
         return _convert_pymupdf(pdf_path, str(prefix_path), dpi, first_page, last_page)
-    except ImportError:
-        pass
+    except ImportError:
+        pass
+    except Exception as exc:
+        return f"ERROR: PyMuPDF conversion failed: {exc}"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	pdf_path = _resolve(pdf)
	if not pdf_path.exists():
	return f"ERROR: PDF not found: {pdf!r}"
	# Ensure the output directory exists.
	prefix_path = Path(prefix)
	if prefix_path.parent != Path("."):
	prefix_path.parent.mkdir(parents=True, exist_ok=True)
	try:
	return _convert_pymupdf(pdf_path, str(prefix_path), dpi, first_page, last_page)
	except ImportError:
	pass
	if shutil.which("pdftoppm"):
	return _convert_pdftoppm(pdf_path, str(prefix_path), dpi, first_page, last_page)
	return (
	"ERROR: No PDF-to-image backend available.\n"
	"Install one of:\n"
	" uv pip install pymupdf # pure-Python (recommended)\n"
	" brew install poppler # provides pdftoppm"
	)
	def _convert_pymupdf(pdf: Path, prefix: str, dpi: int, first: int | None, last: int | None) -> str:
	import fitz # noqa: PLC0415 (PyMuPDF)
	doc = fitz.open(str(pdf))
	total = len(doc)
	start = (first - 1) if first is not None else 0
	stop = last if last is not None else total
	mat = fitz.Matrix(dpi / 72, dpi / 72)
	generated: list[str] = []
	for i in range(start, min(stop, total)):
	page = doc[i]
	pix = page.get_pixmap(matrix=mat)
	out = Path(f"{prefix}-{i + 1:02d}.jpg")
	pix.save(str(out))
	generated.append(str(out))
	return "\n".join(generated) if generated else "WARNING: no pages converted"
	pdf_path = _resolve(pdf)
	if not pdf_path.exists():
	return f"ERROR: PDF not found: {pdf!r}"
	# Ensure the output directory exists.
	prefix_path = Path(prefix)
	if prefix_path.parent != Path("."):
	prefix_path.parent.mkdir(parents=True, exist_ok=True)
	try:
	return _convert_pymupdf(pdf_path, str(prefix_path), dpi, first_page, last_page)
	except ImportError:
	pass
	except Exception as exc:
	return f"ERROR: PyMuPDF conversion failed: {exc}"
	if shutil.which("pdftoppm"):
	return _convert_pdftoppm(pdf_path, str(prefix_path), dpi, first_page, last_page)
	return (
	"ERROR: No PDF-to-image backend available.\n"
	"Install one of:\n"
	" uv pip install pymupdf # pure-Python (recommended)\n"
	" brew install poppler # provides pdftoppm"
	)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/tools/pdf_to_images.py` around lines 63 - 105, The
pdf_to_images flow only handles ImportError around _convert_pymupdf, so other
failures from fitz.open or page.get_pixmap on bad PDFs escape instead of
returning the function’s error-string contract. Update pdf_to_images to catch
the broader conversion exceptions around _convert_pymupdf, and convert those
failures into an "ERROR: ..." response consistent with the existing pdf_path and
backend fallback paths, using the _convert_pymupdf and pdf_to_images symbols to
locate the change.

[coderabbitai]

🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Predictable /tmp path used for an LD_PRELOAD shim, trusted without verification — local code-execution risk.

_SHIM_SO is a fixed, non-random path in the shared temp directory. _ensure_shim() blindly reuses it if it already exists (if _SHIM_SO.exists(): return _SHIM_SO), then it's injected into the LibreOffice subprocess via LD_PRELOAD. On any multi-tenant host, another local user/process could pre-create lo_socket_shim.so at that exact path with arbitrary native code, which would then be silently loaded and executed inside the soffice process the next time this tool runs. The intermediate .c source file is similarly written to a predictable path before gcc compiles it, adding a TOCTOU window. This is a classic insecure-temporary-file pattern (CWE-377) compounded by untrusted-search-path use of LD_PRELOAD (CWE-426).

Recommend using tempfile.mkstemp() (unique name, 0600 perms, owned by current process) instead of a fixed filename, verifying file ownership/mode before trusting an existing .so, or — better — avoiding the runtime-compiled shim altogether by addressing the AF_UNIX restriction at the sandbox/container level (e.g. allowing AF_UNIX for the soffice process instead of intercepting syscalls).

🧰 Tools

🪛 ast-grep (0.44.1)

[error] 178-181: Command coming from incoming request
Context: subprocess.run(
[gcc, "-shared", "-fPIC", "-o", str(_SHIM_SO), str(src), "-ldl"],
capture_output=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

🪛 Ruff (0.15.20)

[error] 179-179: subprocess call: check for execution of untrusted input

(S603)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/tools/pptx_to_images.py` around lines 79 - 197, The
LD_PRELOAD shim uses a predictable shared-temp filename and trusts any existing
.so, which can allow arbitrary local code execution. Update _ensure_shim() and
_SHIM_SO to use a unique, process-owned temporary file (for both the .c source
and compiled .so), and verify ownership/permissions before reusing any existing
shim. If possible, avoid the runtime-compiled shim in _soffice_env() entirely
and handle the AF_UNIX restriction at the sandbox/container level instead.

[coderabbitai]

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check whether other places in the codebase assume "cuga" is still the default skills root
rg -n 'get_skill_root|SKILLS__ROOT|skills\.root' -C3 src/cuga

Repository: cuga-project/cuga-agent

Length of output: 5431

---

Update the skills-root default docs

root = "agents" makes .agents/skills/ the default here, but the surrounding comments and loader docs still describe cuga as the default.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/settings.toml` around lines 96 - 101, Update the skills-root default
documentation so it matches the actual default in settings.toml. In the settings
root configuration comments near root = "agents" and any related loader docs
that describe the default, change the wording from cuga being the default to
agents being the default, and keep the path examples aligned with the current
behavior in the skills-root loading/config code.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)

tests/unit/test_agent_spawn.py (1)

333-356: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Mock set_session_attribute in test_execute_emits_spawn_agent_and_result_events for test isolation.

This test creates SpawnAgentRuntime([]) without parent_config, so _parent_thread_id() likely returns None/empty. The real set_session_attribute is then called with that value — other tests (lines 191, 434) properly monkeypatch it. Add the mock to avoid side effects and keep the test focused on event emission.

🛡️ Suggested fix

     rt = SpawnAgentRuntime([])
 
     events: list = []
     runtime.set_event_callback(lambda name, data: events.append((name, data)))
 
+    monkeypatch.setattr("cuga.backend.agent_spawn.runtime.set_session_attribute", lambda sid: None)
+
     async def _fake_run_stream(agent, task, thread_id, cfg, spawn_id=""):
         return "mocked-answer"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_agent_spawn.py` around lines 333 - 356, Update
test_execute_emits_spawn_agent_and_result_events to monkeypatch
runtime.set_session_attribute with a no-op mock before invoking rt.execute. Keep
the existing event assertions and cleanup unchanged so the test remains isolated
from session state side effects.

src/cuga/backend/skills/tools.py (1)

18-32: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Do not instruct callers to print output that the tool already emits.

The implementation prints the skill body, while the description asks the agent to print it again. Typical print(await load_skill(...)) usage duplicates potentially large instructions in the execution context.

Proposed fix

         description=(
             "Fetch the full instructions for a named skill. "
-            "Call this first, print the output, then follow the instructions."
+            "Call this first; its instructions are emitted automatically. "
+            "Do not print the returned value again."
         ),

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/skills/tools.py` around lines 18 - 32, Update the load_tool
description associated with load_skill_impl so callers are no longer instructed
to print the returned output. Keep the existing print(instructions) behavior in
load_skill_impl and describe only that the tool fetches and emits the full skill
instructions, avoiding duplicate output guidance.

src/cuga/backend/agent_spawn/runtime.py (1)

177-194: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Emit a terminal result event when subagent execution fails.

If agent construction or streaming raises, SpawnAgent is emitted without a matching SpawnAgentResult, leaving event consumers with an indefinitely running subagent.

Proposed fix

         try:
             agent = self._build_agent(tools)
             answer = await self._run_stream(agent, task, thread_id, invoke_cfg, spawn_id=spawn_id)
+        except Exception as exc:
+            _emit(
+                "SpawnAgentResult",
+                {
+                    "agent_name": "SubCuga",
+                    "thread_id": thread_id,
+                    "workspace_thread_id": workspace_thread_id,
+                    "status": "error",
+                    "error": str(exc)[:500],
+                    "spawn_id": spawn_id,
+                },
+            )
+            raise
         finally:
             _spawn_depth.reset(token)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/agent_spawn/runtime.py` around lines 177 - 194, Update the
execution flow around _build_agent and _run_stream to always emit a terminal
SpawnAgentResult when either operation raises. Preserve the existing success
event, and add a failure result with an appropriate failure status and available
error details before re-raising or otherwise preserving the existing exception
behavior.

src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py (1)

433-435: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Deduplicate the inherited spawn tools

When find_tools is off, tools_for_prompt aliases tools_for_execution, so extending it with _runtime_bundle.prompt_tools and then appending the same bundle again adds those tools twice to the spawned agent’s inherited set. Build the subagent tool list from a de-duplicated copy, or skip the second append.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py` around
lines 433 - 435, Update the construction of
_parent_structured_tools_for_subagent to prevent duplicate inherited tools when
tools_for_prompt aliases tools_for_execution. Deduplicate the combined
tools_for_execution, _skill_callable_tools, and _runtime_bundle.prompt_tools
inputs, or ensure the runtime bundle is appended only once while preserving all
required inherited tools.

src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/common/security.py (1)

102-148: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add direct tests for the remaining syntax hints
src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/tests/test_code_executor.py already covers the unterminated f-string path; add focused cases for the spawn_agent hint and the unexpected indent hint so those branches don’t regress.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/common/security.py`
around lines 102 - 148, Add focused tests in test_code_executor.py for
SecurityValidator.format_syntax_error or the relevant syntax-validation path:
verify an unterminated string error includes the spawn_agent guidance, and
verify an unexpected indent error includes its top-level indentation hint. Keep
assertions targeted to the corresponding hint text so both branches are
protected.

Source: Coding guidelines

src/cuga/backend/llm/models.py (1)

675-683: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Normalize provider-prefixed Claude model names before reasoning checks
_is_reasoning_model and the Claude-thinking branch only match raw names like claude-..., but _get_model_name can return provider-prefixed IDs such as anthropic/claude-3.5-sonnet (the OpenRouter default). Those names skip the reasoning path, so temperature/top_p are applied and Claude thinking never turns on. Strip the provider prefix here, matching lookup_model_context_size.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/llm/models.py` around lines 675 - 683, The reasoning-model
detection around _is_reasoning_model and the Claude-thinking branch must
normalize provider-prefixed model IDs before matching. Strip the provider prefix
from names such as anthropic/claude-3.5-sonnet, using the same normalization
approach as lookup_model_context_size, then apply the existing reasoning and
Claude checks to the normalized name so temperature/top_p overrides remain
disabled and thinking is enabled.

♻️ Duplicate comments (1)

src/cuga/backend/cuga_graph/utils/agent_loop.py (1)

740-741: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Global spawn callback still needs request-local isolation (previously flagged).

_spawn_runtime.set_event_callback remains process-global. Concurrent run_stream() calls still overwrite each other's spawn handler — one request's spawn events route to another request's queue, and the first stream to exit clears the callback for all others. This was flagged in a prior review and the code pattern is unchanged.

Also applies to: 802-803

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/cuga_graph/utils/agent_loop.py` around lines 740 - 741,
Replace the process-global callback usage around
_spawn_runtime.set_event_callback in run_stream with request-local spawn event
routing so concurrent streams cannot overwrite or clear one another’s handlers.
Ensure each stream’s events reach only its own queue, and cleanup removes only
that stream’s registration; apply the same change to the corresponding logic
near the second referenced block.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cuga/backend/agent_spawn/runtime.py`:
- Around line 120-128: Update _build_invoke_config to preserve the parent
request’s skills_enabled and skills_folder values when constructing the subagent
invoke configuration. Merge these fields from _parent_config into cfg, retaining
the existing workspace_thread_id handling and allowing request-scoped parent
settings to reach prepare_node.py.
- Around line 202-204: Store the asyncio task returned by create_task in the
existing future-tracking structure or a dedicated task collection, and remove it
only after _execute_and_store completes. Update the spawn flow around
_spawn_futures so the background task remains strongly referenced while
preserving the existing future state updates.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/common/security.py`:
- Around line 112-117: Remove the trailing period after the spawn-agent
example’s json.dumps expression in the hint string within the security prompt,
keeping the example syntactically valid Python while preserving the surrounding
guidance.

---

Outside diff comments:
In `@src/cuga/backend/agent_spawn/runtime.py`:
- Around line 177-194: Update the execution flow around _build_agent and
_run_stream to always emit a terminal SpawnAgentResult when either operation
raises. Preserve the existing success event, and add a failure result with an
appropriate failure status and available error details before re-raising or
otherwise preserving the existing exception behavior.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py`:
- Around line 433-435: Update the construction of
_parent_structured_tools_for_subagent to prevent duplicate inherited tools when
tools_for_prompt aliases tools_for_execution. Deduplicate the combined
tools_for_execution, _skill_callable_tools, and _runtime_bundle.prompt_tools
inputs, or ensure the runtime bundle is appended only once while preserving all
required inherited tools.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/common/security.py`:
- Around line 102-148: Add focused tests in test_code_executor.py for
SecurityValidator.format_syntax_error or the relevant syntax-validation path:
verify an unterminated string error includes the spawn_agent guidance, and
verify an unexpected indent error includes its top-level indentation hint. Keep
assertions targeted to the corresponding hint text so both branches are
protected.

In `@src/cuga/backend/llm/models.py`:
- Around line 675-683: The reasoning-model detection around _is_reasoning_model
and the Claude-thinking branch must normalize provider-prefixed model IDs before
matching. Strip the provider prefix from names such as
anthropic/claude-3.5-sonnet, using the same normalization approach as
lookup_model_context_size, then apply the existing reasoning and Claude checks
to the normalized name so temperature/top_p overrides remain disabled and
thinking is enabled.

In `@src/cuga/backend/skills/tools.py`:
- Around line 18-32: Update the load_tool description associated with
load_skill_impl so callers are no longer instructed to print the returned
output. Keep the existing print(instructions) behavior in load_skill_impl and
describe only that the tool fetches and emits the full skill instructions,
avoiding duplicate output guidance.

In `@tests/unit/test_agent_spawn.py`:
- Around line 333-356: Update test_execute_emits_spawn_agent_and_result_events
to monkeypatch runtime.set_session_attribute with a no-op mock before invoking
rt.execute. Keep the existing event assertions and cleanup unchanged so the test
remains isolated from session state side effects.

---

Duplicate comments:
In `@src/cuga/backend/cuga_graph/utils/agent_loop.py`:
- Around line 740-741: Replace the process-global callback usage around
_spawn_runtime.set_event_callback in run_stream with request-local spawn event
routing so concurrent streams cannot overwrite or clear one another’s handlers.
Ensure each stream’s events reach only its own queue, and cleanup removes only
that stream’s registration; apply the same change to the corresponding logic
near the second referenced block.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 468c5132-28a2-4fd9-a68d-06b2e9c245b2

📥 Commits

Reviewing files that changed from the base of the PR and between b57e2e1 and 65f9618.

📒 Files selected for processing (25)

• .gitignore
• .secrets.baseline
• src/cuga/backend/agent_spawn/prompt_utils.py
• src/cuga/backend/agent_spawn/runtime.py
• src/cuga/backend/agent_spawn/tools.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/graph_adapter.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/common/security.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/filesystem/paths.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/filesystem/tests/test_workspace_fs.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/filesystem/workspace_fs.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/prompts/mcp_prompt.jinja2
• src/cuga/backend/cuga_graph/nodes/cuga_lite/reflection/prompts/reflection_system.jinja2
• src/cuga/backend/cuga_graph/policy/agent.py
• src/cuga/backend/cuga_graph/policy/enactment.py
• src/cuga/backend/cuga_graph/utils/agent_loop.py
• src/cuga/backend/cuga_graph/utils/token_counter.py
• src/cuga/backend/llm/models.py
• src/cuga/backend/server/main.py
• src/cuga/backend/skills/tools.py
• src/cuga/config.py
• src/cuga/settings.toml
• src/frontend_workspaces/frontend/src/carbon-chat/customSendMessage.ts
• tests/unit/test_agent_spawn.py
• tests/unit/test_skill_loader.py

🚧 Files skipped from review as they are similar to previous changes (8)

• src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/graph_adapter.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/prompts/mcp_prompt.jinja2
• src/cuga/backend/agent_spawn/tools.py
• src/cuga/backend/agent_spawn/prompt_utils.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/reflection/prompts/reflection_system.jinja2
• src/cuga/config.py
• src/cuga/backend/server/main.py
• src/frontend_workspaces/frontend/src/carbon-chat/customSendMessage.ts

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Preserve request-scoped skill configuration for subagents.

This rebuild drops parent skills_enabled and skills_folder, although prepare_node.py Lines 314-323 consumes those values. With global skills disabled, a parent that enables skills per request spawns a child without them.

Proposed fix

         cfg = get_langfuse_invoke_config()
+        configurable = dict(cfg.get("configurable") or {})
+        parent_configurable = self._parent_config.get("configurable") or {}
+        for key in ("skills_enabled", "skills_folder"):
+            if key in parent_configurable:
+                configurable[key] = parent_configurable[key]
         if workspace_thread_id:
-            configurable = dict(cfg.get("configurable") or {})
             configurable["workspace_thread_id"] = workspace_thread_id
-            cfg = {**cfg, "configurable": configurable}
+        cfg = {**cfg, "configurable": configurable}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def _build_invoke_config(self, workspace_thread_id: str = "") -> dict:
	if self._parent_config:
	sync_langfuse_callbacks_from_config(self._parent_config)
	cfg = get_langfuse_invoke_config()
	if workspace_thread_id:
	configurable = dict(cfg.get("configurable") or {})
	configurable["workspace_thread_id"] = workspace_thread_id
	cfg = {**cfg, "configurable": configurable}
	return cfg
	def _build_invoke_config(self, workspace_thread_id: str = "") -> dict:
	if self._parent_config:
	sync_langfuse_callbacks_from_config(self._parent_config)
	cfg = get_langfuse_invoke_config()
	configurable = dict(cfg.get("configurable") or {})
	parent_configurable = self._parent_config.get("configurable") or {}
	for key in ("skills_enabled", "skills_folder"):
	if key in parent_configurable:
	configurable[key] = parent_configurable[key]
	if workspace_thread_id:
	configurable["workspace_thread_id"] = workspace_thread_id
	cfg = {**cfg, "configurable": configurable}
	return cfg

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/agent_spawn/runtime.py` around lines 120 - 128, Update
_build_invoke_config to preserve the parent request’s skills_enabled and
skills_folder values when constructing the subagent invoke configuration. Merge
these fields from _parent_config into cfg, retaining the existing
workspace_thread_id handling and allowing request-scoped parent settings to
reach prepare_node.py.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep the spawn-agent example syntactically valid.

The new example ends with })., so copying the hint produces invalid Python. Remove the trailing period from the expression.

Proposed fix

-            'task = ".... Inputs JSON: " + json.dumps({"a": a_out, "b": b_out}).'
+            'task = ".... Inputs JSON: " + json.dumps({"a": a_out, "b": b_out})'

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"\nHint: Do not embed large markdown, JSON, or prior tool/subagent outputs inside "
	"f-strings or triple-quoted strings. Build reports with '\\n'.join([...]), "
	"json.dumps() for dict sections, or await write_file('./output/report.md', content). "
	"For spawn_agent follow-ups: "
	'task = ".... Inputs JSON: " + json.dumps({"a": a_out, "b": b_out}).'
	)
	"\nHint: Do not embed large markdown, JSON, or prior tool/subagent outputs inside "
	"f-strings or triple-quoted strings. Build reports with '\\n'.join([...]), "
	"json.dumps() for dict sections, or await write_file('./output/report.md', content). "
	"For spawn_agent follow-ups: "
	'task = ".... Inputs JSON: " + json.dumps({"a": a_out, "b": b_out})'

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/executors/common/security.py`
around lines 112 - 117, Remove the trailing period after the spawn-agent
example’s json.dumps expression in the hint string within the security prompt,
keeping the example syntactically valid Python while preserving the surrounding
guidance.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py (1)

55-724: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

prepare_tools_and_apps keeps growing into a single ~670-line function.

This PR adds another agent-spawn injection block (Lines 420-459) on top of an already large function that also handles policy enactment, tool/app resolution, todos, tool-guides, skills discovery, and knowledge awareness. As per coding guidelines, files under cuga_graph/nodes/**/*.py should stay lightweight and single-responsibility, extracting cohesive sub-components (e.g., skills setup, agent-spawn tool wiring, knowledge injection) into dedicated helper modules/sub-packages once a module "begins to approach... the limit," rather than continuing to accrete logic into one node function.

Consider extracting the skills-discovery block (Lines 310-344) and the agent-spawn injection block (Lines 420-459) into helpers/ functions callable from this node, keeping prepare_tools_and_apps as an orchestrator.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py` around
lines 55 - 724, Refactor prepare_tools_and_apps so it remains an orchestration
node by extracting the skills discovery/tool wiring block and the agent-spawn
injection block into cohesive helpers under helpers/. Preserve their existing
settings, registry, prompt-section, tool-context, spawn-depth, and parent-tool
behavior; have the helpers return the data needed to update tools_for_prompt,
adapter._tools_context, and prompt metadata, then invoke them from
prepare_tools_and_apps.

Source: Coding guidelines

🧹 Nitpick comments (2)

src/cuga/backend/agent_spawn/runtime.py (1)

59-65: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider logging swallowed event-callback exceptions.

The blind except Exception: pass is intentional (never crash the agent on event emission), but silently discarding the exception makes failures in the UI/event-forwarding pipeline undebuggable. A debug-level log preserves the "never crash" guarantee while keeping the failure visible.

♻️ Proposed tweak

     if cb:
         try:
             cb(event_name, data)
-        except Exception:
-            pass  # never let event emission crash the agent
+        except Exception:
+            logger.debug("agent_spawn: event callback raised", exc_info=True)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/agent_spawn/runtime.py` around lines 59 - 65, Update the
exception handler in _emit to log swallowed event-callback exceptions at debug
level while retaining the existing suppression behavior so event emission never
crashes the agent.

Source: Linters/SAST tools

src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py (1)

324-326: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid reaching into runtime.py's private _spawn_depth ContextVar.

Importing _spawn_depth (leading underscore = internal) directly couples this module to agent_spawn.runtime's private state. A small public accessor keeps the contract stable if the internal representation changes.

♻️ Proposed refactor

-        from cuga.backend.agent_spawn.runtime import _spawn_depth as _agent_spawn_depth
-
-        _spawn_depth_now = _agent_spawn_depth.get()
+        from cuga.backend.agent_spawn.runtime import current_spawn_depth
+
+        _spawn_depth_now = current_spawn_depth()

Add to runtime.py:

def current_spawn_depth() -> int:
    """Return the spawn depth for the current context."""
    return _spawn_depth.get()

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py` around
lines 324 - 326, Replace the direct private `_spawn_depth` import in the
preparation flow with the public `current_spawn_depth()` accessor from
`agent_spawn.runtime`. Add that accessor in `runtime.py`, returning the current
value from `_spawn_depth`, and use its result where `_spawn_depth_now` is
assigned.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cuga/backend/agent_spawn/runtime.py`:
- Around line 82-96: Update the stale cleanup logic in _track_task’s _done
callback so it removes _tasks_by_thread[key] only when the mapping still
references the exact bucket captured by that callback and that bucket is empty.
Preserve newly created buckets for reused thread IDs, including the cancellation
path in clear_runtime_caches.

---

Outside diff comments:
In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py`:
- Around line 55-724: Refactor prepare_tools_and_apps so it remains an
orchestration node by extracting the skills discovery/tool wiring block and the
agent-spawn injection block into cohesive helpers under helpers/. Preserve their
existing settings, registry, prompt-section, tool-context, spawn-depth, and
parent-tool behavior; have the helpers return the data needed to update
tools_for_prompt, adapter._tools_context, and prompt metadata, then invoke them
from prepare_tools_and_apps.

---

Nitpick comments:
In `@src/cuga/backend/agent_spawn/runtime.py`:
- Around line 59-65: Update the exception handler in _emit to log swallowed
event-callback exceptions at debug level while retaining the existing
suppression behavior so event emission never crashes the agent.

In `@src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py`:
- Around line 324-326: Replace the direct private `_spawn_depth` import in the
preparation flow with the public `current_spawn_depth()` accessor from
`agent_spawn.runtime`. Add that accessor in `runtime.py`, returning the current
value from `_spawn_depth`, and use its result where `_spawn_depth_now` is
assigned.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b567cfc2-03f2-46bf-b541-ae2b6324bea0

📥 Commits

Reviewing files that changed from the base of the PR and between 65f9618 and ef4e32a.

📒 Files selected for processing (12)

• .secrets.baseline
• src/cuga/backend/agent_spawn/__init__.py
• src/cuga/backend/agent_spawn/runtime.py
• src/cuga/backend/agent_spawn/tools.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/adapter/prepare_node.py
• src/cuga/backend/cuga_graph/nodes/cuga_lite/prompts/mcp_prompt.jinja2
• src/cuga/backend/cuga_graph/utils/agent_loop.py
• src/cuga/config.py
• src/cuga/settings.toml
• src/frontend_workspaces/frontend/src/carbon-chat/customLoadHistory.ts
• src/frontend_workspaces/frontend/src/carbon-chat/customSendMessage.ts
• tests/unit/test_agent_spawn.py

💤 Files with no reviewable changes (1)

• src/cuga/config.py

🚧 Files skipped from review as they are similar to previous changes (6)

• src/frontend_workspaces/frontend/src/carbon-chat/customLoadHistory.ts
• .secrets.baseline
• src/cuga/backend/cuga_graph/utils/agent_loop.py
• src/cuga/settings.toml
• src/cuga/backend/agent_spawn/tools.py
• src/frontend_workspaces/frontend/src/carbon-chat/customSendMessage.ts

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Stale done-callback can evict a freshly-tracked task bucket for a reused thread_id.

_track_task's _done callback closes over the bucket object captured at tracking time and does _tasks_by_thread.pop(key, None) once that specific bucket empties — not the bucket currently stored under key. If clear_runtime_caches(thread_id) cancels tasks and pops _tasks_by_thread[key] (Line 95) while those tasks are still finishing cancellation, and a new spawn is tracked for the same thread_id in that window (_track_task creates a fresh bucket via setdefault), the old tasks' eventual _done callbacks will see their (now orphaned) bucket become empty and unconditionally pop _tasks_by_thread[key] — which is now the new bucket, silently dropping tracking of live tasks from pending_spawn_tasks/wait_pending_spawns. This can cause late SubAgent stream events to be missed because AgentLoop.run_stream stops waiting for spawns it no longer sees as pending.

🐛 Proposed fix: only pop if the dict still holds this exact bucket

     def _done(t: asyncio.Task) -> None:
         bucket.discard(t)
-        if not bucket:
+        if not bucket and _tasks_by_thread.get(key) is bucket:
             _tasks_by_thread.pop(key, None)

Also applies to: 118-127

🧰 Tools

🪛 Ruff (0.15.20)

[warning] 85-85: Loop control variable tid not used within loop body

Rename unused tid to _tid

(B007)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cuga/backend/agent_spawn/runtime.py` around lines 82 - 96, Update the
stale cleanup logic in _track_task’s _done callback so it removes
_tasks_by_thread[key] only when the mapping still references the exact bucket
captured by that callback and that bucket is empty. Preserve newly created
buckets for reused thread IDs, including the cancellation path in
clear_runtime_caches.