massgen/orchestrator_collaborators/orchestrator_timeout_calculator.py-37-40 (1)

37-40: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle unknown agent_id before indexing agent_states.

This path currently raises KeyError for a stale or already-cleaned-up agent, which turns a timeout check into a round-breaking exception. get_agent_timeout_state() already uses a safe lookup; this helper should do the same.

Suggested fix

         orchestrator = self._orchestrator
         timeout_config = orchestrator.config.timeout_config
-        round_start = orchestrator.agent_states[agent_id].round_start_time
+        state = orchestrator.agent_states.get(agent_id)
+        if state is None:
+            return False
+        round_start = state.round_start_time

🤖 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 `@massgen/orchestrator_collaborators/orchestrator_timeout_calculator.py` around
lines 37 - 40, The code directly indexes orchestrator.agent_states[agent_id]
which can raise KeyError for stale/cleaned agents; change the lookup to a safe
retrieval (mirroring get_agent_timeout_state()) by using
orchestrator.agent_states.get(agent_id) (or the existing safe helper) and bail
out or return a default timeout when missing before accessing round_start, so
the timeout check does not raise an exception.

massgen/orchestrator_collaborators/bootstrap_criteria_engine.py-137-147 (1)

137-147: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Don't cache the discriminator signature before the run outcome is known.

seen.add(content_signature) happens before run_bootstrap_discriminator_step() returns, but that callee returns 0 for both legitimate “nothing new to add” runs and outright failures/import errors/spawn failures. A transient failure therefore marks the answer set as completed and suppresses every future retry for the same content.

Please make the callee return an explicit completion status and only record the signature after a real completed discriminator pass.

🤖 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 `@massgen/orchestrator_collaborators/bootstrap_criteria_engine.py` around lines
137 - 147, The code currently adds content_signature to
orch._bootstrap_discriminator_completed_signatures before knowing the outcome of
run_bootstrap_discriminator_step(), causing transient failures to be treated as
completed; change the flow so run_bootstrap_discriminator_step() returns an
explicit completion status (e.g., boolean or enum) and only call
seen.add(content_signature) when that status indicates a successful/real
completed discriminator pass; update the check here (using content_signature,
orch._bootstrap_discriminator_completed_signatures, and
run_bootstrap_discriminator_step) to add the signature on success and leave it
unadded on failure, and adjust any other callers of
run_bootstrap_discriminator_step to handle the new explicit return value.

massgen/orchestrator_collaborators/round_evaluator_runner.py-644-654 (1)

644-654: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Emit the display completion status after salvage/degraded handling, not before.

This notification is finalized off the raw success bit, but the method can still salvage a failed spawn from recovered artifacts or convert a timeout into the degraded-success path. In those cases the UI is told failed here and never corrected, even though orchestration continues with a usable evaluator outcome.

Move the completion notification to the final exit paths so it reflects the derived status (completed, degraded, or terminal failure) exactly once.

🤖 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 `@massgen/orchestrator_collaborators/round_evaluator_runner.py` around lines
644 - 654, The current code calls
orch.coordination_ui.display.notify_runtime_subagent_completed early using the
raw success flag (variables: display, orch.coordination_ui,
notify_runtime_subagent_completed, parent_agent_id, upcoming_round,
tool_call_id, success), which can misreport status if later salvage/degraded
logic converts failures into usable outcomes; move this notification out of the
early path and invoke it exactly once at the final exit points after all
salvage/degraded handling finishes, passing the derived status string
("completed", "degraded", or "failed") so the UI reflects the final outcome.

massgen/orchestrator_collaborators/pre_collab_helpers.py-38-40 (1)

38-40: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle non-dict backend configs before calling .items().

This helper now assumes agent.backend.config is always a mapping, but other extracted code in this PR still treats backend config as potentially object-shaped. On those agents, .items() raises and breaks every pre-collab flow that calls _build_parent_agent_configs().

Suggested fix

+from collections.abc import Mapping
 import tempfile
 from typing import TYPE_CHECKING, Any
@@
         for agent_id, agent in self._orchestrator.agents.items():
             agent_cfg: dict[str, Any] = {"id": agent_id}
-            if hasattr(agent, "backend") and hasattr(agent.backend, "config"):
-                backend_cfg = {k: v for k, v in agent.backend.config.items() if k not in ("mcp_servers", "_config_path")}
+            cfg = getattr(getattr(agent, "backend", None), "config", None)
+            if isinstance(cfg, Mapping):
+                backend_cfg = {
+                    k: v
+                    for k, v in cfg.items()
+                    if k not in ("mcp_servers", "_config_path")
+                }
+                agent_cfg["backend"] = backend_cfg
+            elif cfg is not None:
+                backend_cfg = {
+                    k: v
+                    for k, v in vars(cfg).items()
+                    if k not in ("mcp_servers", "_config_path")
+                }
                 agent_cfg["backend"] = backend_cfg
             configs.append(agent_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 `@massgen/orchestrator_collaborators/pre_collab_helpers.py` around lines 38 -
40, The code in _build_parent_agent_configs assumes agent.backend.config is a
mapping and calls .items(), which breaks for object-shaped configs; update the
branch that builds backend_cfg to first test the type of agent.backend.config
(e.g., isinstance(agent.backend.config, Mapping) from collections.abc) and only
call .items() for mappings, otherwise fallback to a safe conversion such as
vars(agent.backend.config) or {k: getattr(agent.backend.config, k) for k in
dir(agent.backend.config) if not k.startswith("_")} to produce a dict; ensure
this logic is applied where agent_cfg["backend"] is set so non-dict
backend.configs no longer raise.

massgen/orchestrator_collaborators/active_coordination_cleanup.py-49-60 (1)

49-60: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Snapshot _active_tasks before awaiting cancelled tasks.

This loop iterates the live _active_tasks dict and then awaits inside the iteration. Cancellation callbacks can mutate _active_tasks while control is yielded, which can raise RuntimeError: dictionary changed size during iteration and abort the cleanup path.

Suggested fix

-            for agent_id, task in orch._active_tasks.items():
+            for agent_id, task in list(orch._active_tasks.items()):
                 if not task.done():
                     if not orch.is_orchestrator_timeout:
                         orch.coordination_tracker.track_agent_action(

🤖 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 `@massgen/orchestrator_collaborators/active_coordination_cleanup.py` around
lines 49 - 60, The loop mutates orch._active_tasks while awaiting cancelled
tasks, causing a RuntimeError; fix by snapshotting the items first (e.g., items
= list(orch._active_tasks.items()) or collect tasks to cancel in a separate
list) and then iterate that snapshot to call
orch.coordination_tracker.track_agent_action (using ActionType.CANCELLED when
appropriate), task.cancel(), and await the task from the snapshot so the
original orch._active_tasks dict cannot change during iteration.

massgen/orchestrator_collaborators/checkpoint_coordinator.py-190-295 (1)

190-295: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset checkpoint state in a finally path.

_checkpoint_active, _checkpoint_task, and _checkpoint_participants are set before the subprocess work starts, but any exception from spawn(), _copy_subprocess_logs(), event emission, or cleanup() will exit this method without clearing that state. That can leave the orchestrator stuck in checkpoint mode for the rest of the session.

The early "no workspace available" return also happens after _checkpoint_participants is populated, so stale participant state can leak even without an exception.

Suggested direction

-        orch._checkpoint_active = True
-        orch._checkpoint_number += 1
-        orch._checkpoint_task = signal.get("task", "")
+        orch._checkpoint_active = True
+        orch._checkpoint_number += 1
+        orch._checkpoint_task = signal.get("task", "")
+        orch._checkpoint_participants = {}
+
+        try:
+            ...
+            if not parent_workspace:
+                logger.error("[Checkpoint] No parent workspace for subprocess")
+                return "Checkpoint failed: no workspace available"
+
+            result = await manager.spawn(
+                signal=signal,
+                on_event=_relay_event,
+            )
+            ...
+            return consensus
+        finally:
+            orch._checkpoint_active = False
+            orch._checkpoint_participants = {}

🤖 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 `@massgen/orchestrator_collaborators/checkpoint_coordinator.py` around lines
190 - 295, Wrap the subprocess work and all subsequent post-processing (the call
to CheckpointSubprocessManager(...), await manager.spawn(...),
manager._copy_subprocess_logs(), event emissions, and manager.cleanup()) in a
try/finally so orch._checkpoint_active, orch._checkpoint_task and
orch._checkpoint_participants are always reset in the finally block; also ensure
the early "no workspace available" return path clears those same fields before
returning. Use the existing symbols (CheckpointSubprocessManager, manager.spawn,
manager._copy_subprocess_logs, manager.cleanup, get_event_emitter) to locate
where to add the try/finally and the cleanup assignments.

massgen/orchestrator_collaborators/subagent_tool_injector.py-97-111 (1)

97-111: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make list-backed MCP injection idempotent.

When mcp_servers is a dict you replace by key, but in the list branch you always append. If this injector runs more than once for the same agent, the same subagent server gets registered repeatedly and the backend will see duplicate tool definitions. Filter the list by self.subagent_server_name(agent_id) before appending the new config.

🤖 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 `@massgen/orchestrator_collaborators/subagent_tool_injector.py` around lines 97
- 111, The list-backed MCP injection is not idempotent: in the else branch you
always append subagent_mcp_config to mcp_servers causing duplicates; modify the
logic that handles list mcp_servers (variable mcp_servers, method
subagent_server_name(agent_id), and value subagent_mcp_config) to first filter
out any existing entries whose server name equals
self.subagent_server_name(agent_id) (or whose "name" field matches that value)
and then append the new subagent_mcp_config only once, finally writing back to
agent.backend.config["mcp_servers"] so repeated runs replace the previous entry
instead of duplicating it.

massgen/orchestrator_collaborators/subagent_tool_injector.py-542-568 (1)

542-568: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the round-2 rewrite path in sync with the initial config writer.

rewrite_subagent_mcp_config_files() is not reproducing the payload from create_subagent_mcp_config(): it drops the backend-model fallback and never rewrites *_context_paths.json. After a workspace clear, later subagent launches can therefore run with a different config than round 1 even though the docstring says this is the counterpart path. Please share the serialization logic or rewrite the same files/fields here.

🤖 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 `@massgen/orchestrator_collaborators/subagent_tool_injector.py` around lines
542 - 568, rewrite_subagent_mcp_config_files() currently builds agent_configs
but omits the backend-model fallback and never emits the *_context_paths.json
(so round-2 payload diverges from create_subagent_mcp_config()); update
rewrite_subagent_mcp_config_files() to mirror create_subagent_mcp_config()
exactly by: when serializing each agent (agent_cfg in the loop over orch.agents)
include the backend model fallback field(s) you add/expect from backend.config
(e.g., preserve any "model" or fallback keys removed by the filter), serialize
runtime_agent_config.context_paths (or the same attribute
create_subagent_mcp_config reads) and write it to mcp_temp_dir /
f"{_token}_context_paths.json", and ensure the same three files are produced
(*_agent_configs.json, *_coordination_config.json, *_orchestrator_config.json
plus *_context_paths.json) using the same to_dict/json serialization logic used
in create_subagent_mcp_config() so the round‑2 payload matches round‑1.

massgen/orchestrator_collaborators/previous_log_restorer.py-39-42 (1)

39-42: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use Google-style docstrings on the new methods.

These new APIs still use summary-only docstrings, so the extracted collaborator is missing the required Args/Returns structure where callers now need it most.

As per coding guidelines, "**/*.py: For new or changed functions, include Google-style docstrings"

Also applies to: 42-52, 215-222, 281-282

🤖 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 `@massgen/orchestrator_collaborators/previous_log_restorer.py` around lines 39
- 42, The new methods currently have summary-only docstrings; update each
changed/new function (e.g., the class __init__ and restore_from_previous_log in
PreviousLogRestorer, plus the other modified functions referenced around the
file such as the methods at the other changed ranges) to use Google-style
docstrings including at minimum an "Args:" section describing parameters and
types and a "Returns:" section (or "Raises:" if applicable) so callers can rely
on structured doc metadata; ensure the docstring formatting follows Google style
(triple-quoted string immediately under the def) and include types and brief
descriptions for each parameter and the return value or None.

massgen/orchestrator_collaborators/post_evaluation_runner.py-39-44 (1)

39-44: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use Google-style docstrings on these new methods.

Both new entry points are missing the required Args/Returns sections, which makes the restart and post-eval flow harder to follow now that it lives outside Orchestrator.

As per coding guidelines, "**/*.py: For new or changed functions, include Google-style docstrings"

Also applies to: 436-443

🤖 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 `@massgen/orchestrator_collaborators/post_evaluation_runner.py` around lines 39
- 44, The two new async entry methods (post_evaluate_answer and the other new
method at lines ~436-443) are missing Google-style docstrings; update each
function (post_evaluate_answer and the other new entrypoint method name as seen
in the diff) to include a proper Google-style docstring with an overall summary
line plus an Args section that documents selected_agent_id: str and
final_answer: str (or the appropriate parameters for the second method) and a
Returns section that describes the AsyncGenerator[StreamChunk] return type and
what each yielded StreamChunk represents; ensure parameter types and return
behavior (async generator yielding StreamChunk events) are clearly described so
the restart and post-eval flows are discoverable.

massgen/orchestrator_collaborators/post_evaluation_runner.py-436-489 (1)

436-489: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear the consumed restart flag in handle_restart().

post_evaluate_answer() raises orch.restart_pending, but handle_restart() never clears it. That stale flag can make the next attempt look like it is still awaiting a restart decision and trigger another restart even after a later approval.

🤖 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 `@massgen/orchestrator_collaborators/post_evaluation_runner.py` around lines
436 - 489, post_evaluate_answer() sets orch.restart_pending but handle_restart()
never clears it, leaving a stale restart flag that can trigger another restart;
inside handle_restart (after obtaining orch or before finishing the method)
explicitly reset the flag (e.g., set orch.restart_pending = False or None) so
the orchestrator no longer appears to be awaiting a restart decision; update any
places that check the flag to expect this cleared state and ensure you reference
handle_restart and orch.restart_pending when making the change.

massgen/orchestrator_collaborators/final_presentation_runner.py-449-458 (1)

449-458: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset _presentation_started in the finally path.

Line 458 flips the guard to True, but nothing clears it afterward. After the first presentation attempt, every later call returns "Presentation already in progress", which breaks retries, restarts, and later-turn reuse of this runner.

Also applies to: 1127-1237

🤖 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 `@massgen/orchestrator_collaborators/final_presentation_runner.py` around lines
449 - 458, The guard orch._presentation_started is set to True but never
cleared, causing all subsequent calls to short-circuit; update the presentation
runner functions (the block setting orch._presentation_started = True and the
duplicate-call path that logs via _orch_mod.logger.warning and yields
StreamChunk) to ensure orch._presentation_started is reset to False in a finally
path (or via context manager) so it is cleared after success or any exception;
apply the same fix to the other occurrence around the 1127-1237 region to
restore retries/restarts correctly.

massgen/orchestrator_collaborators/workspace_lifecycle_manager.py-23-317 (1)

23-317: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use Google-style docstrings for the new workspace lifecycle methods.

This module adds several new entry points, but the docstrings still don't follow the required Args/Returns structure.

As per coding guidelines, "**/*.py: For new or changed functions, include Google-style docstrings"

🤖 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 `@massgen/orchestrator_collaborators/workspace_lifecycle_manager.py` around
lines 23 - 317, Several new public methods (clear_agent_workspaces,
archive_agent_memories, namespace_verification_memory_files,
merge_agent_memories_to_winner, seed_plan_execution_workspaces,
copy_workspace_contents) lack Google-style docstrings; update each docstring to
follow the project's Google style (one-line summary, blank line, Args: with
parameter names and types, Returns: with type and description where applicable,
and Raises: when exceptions are expected) and ensure types for
orch/session_id/agent_id/etc. are specified and brief example lines removed so
linters accept them. Locate and replace the existing triple-quoted summaries in
those functions with the new formatted docstrings and keep the current brief
descriptions as the one-line summary. Ensure seed_plan_execution_workspaces and
copy_workspace_contents include Returns or None as appropriate and
archive_agent_memories/docstrings mention side effects (file I/O) in Raises if
relevant.

massgen/orchestrator_collaborators/isolated_change_reviewer.py-33-112 (1)

33-112: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Convert the new helper and entry-point docstrings to Google style.

The extracted review/apply flow is adding new API surface, but the methods still omit the required Args/Returns sections.

As per coding guidelines, "**/*.py: For new or changed functions, include Google-style docstrings"

🤖 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 `@massgen/orchestrator_collaborators/isolated_change_reviewer.py` around lines
33 - 112, The new helper functions (_count_diff_files,
_normalize_approved_files_by_context, _normalize_approved_hunks_by_context,
_is_in_context_prefix, _format_file_list) and the entry-point method review need
Google-style docstrings including Args and Returns (and Raises when applicable);
update each function/method to keep the existing short description and add an
"Args:" section listing parameters with types and a brief description and a
"Returns:" section describing the return value and type (for review, also
include that it yields AsyncGenerator[StreamChunk] and any contextual behavior),
and add "Raises:" only if the function can intentionally raise exceptions—ensure
formatting matches other Google-style docstrings in the codebase.

massgen/orchestrator_collaborators/previous_log_restorer.py-83-85 (1)

83-85: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Sort restored snapshots by round/timestamp, not by label string.

Line 84 uses lexicographic label ordering, so revisions like agent1.10 restore before agent1.2. That can leave answers[-1] and the restored agent state pointing at an older revision after longer runs. Sort the mappings by numeric round and a monotonic field like timestamp instead.

🤖 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 `@massgen/orchestrator_collaborators/previous_log_restorer.py` around lines 83
- 85, The current loop sorts answer_snapshots by label string which causes
lexicographic order issues (e.g., "agent1.10" before "agent1.2"); change the
sort to order mappings by their numeric round and a monotonic timestamp instead:
iterate over answer_snapshots.items() sorted by a key that uses mapping['round']
(or parses the numeric round from label if round is not present) and then
mapping['timestamp'] (or another monotonic field) so restored mappings and
answers[-1] reflect true chronological/round order; update the loop that
references answer_snapshots, label, and mapping to use that sorted order.

massgen/orchestrator_collaborators/peer_answer_visibility_tracker.py-39-291 (1)

39-291: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Convert the new method docstrings to Google style.

This extracted collaborator adds a lot of new surface area, but the methods still use summary-only docstrings instead of the required Args/Returns format.

As per coding guidelines, "**/*.py: For new or changed functions, include Google-style docstrings"

massgen/orchestrator_collaborators/final_result_reporter.py-342-353 (1)

342-353: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Populate per-snapshot content from the snapshot itself.

These entries are keyed by snapshot label, but content is pulled from orch.agent_states[agent_id].answer, i.e. the agent's latest answer. Once an agent has multiple snapshots, older labels will all report the newest content, so the API no longer matches the snapshot mapping.

🤖 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 `@massgen/orchestrator_collaborators/final_result_reporter.py` around lines 342
- 353, The code currently sets content = orch.agent_states[agent_id].answer
which returns the agent's latest answer and causes older snapshot labels to show
the newest content; update the content assignment to read the per-snapshot
content from the snapshot mapping (e.g., content = mapping.get("content") or
mapping.get("snapshot_answer")) and only fall back to
orch.agent_states[agent_id].answer if the mapping has no snapshot content;
change the assignment near where mapping, agent_id, and answers are used (the
block that builds the answers list with label, agent_id, answer_path) so each
snapshot label reflects its own stored content.

massgen/orchestrator_collaborators/workspace_lifecycle_manager.py-60-71 (1)

60-71: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Skip symlinks when repopulating workspaces.

These copy loops restore top-level entries from previous/cancelled workspaces without an item.is_symlink() guard. That can reintroduce symlinks pointing outside the live workspace, even though copy_workspace_contents() explicitly avoids that later in this file.

Also applies to: 83-94

🤖 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 `@massgen/orchestrator_collaborators/workspace_lifecycle_manager.py` around
lines 60 - 71, The loop that restores top-level entries from previous workspaces
(iterating source.iterdir() and calling shutil.copy2 / shutil.copytree) lacks an
item.is_symlink() guard and can reintroduce external symlinks; update both copy
loops (the one shown and the similar block at lines ~83-94) to skip items where
item.is_symlink() is True (e.g., if item.is_symlink(): continue) before
performing shutil.copy2 or shutil.copytree, keeping consistency with
copy_workspace_contents() which deliberately avoids symlinks; optionally add a
brief debug/log statement when skipping a symlink for traceability.

massgen/orchestrator_collaborators/final_presentation_runner.py-38-444 (1)

38-444: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use Google-style docstrings on the new final-presentation methods.

This extracted runner now owns several core orchestration entry points, but the new methods still omit the required Args/Returns sections.

As per coding guidelines, "**/*.py: For new or changed functions, include Google-style docstrings"

🤖 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 `@massgen/orchestrator_collaborators/final_presentation_runner.py` around lines
38 - 444, The new final-presentation methods
(yield_existing_answer_finalization, present_final_answer,
handle_orchestrator_timeout, determine_final_agent_from_votes,
get_final_presentation) lack Google-style docstrings with Args/Returns; update
each function/class docstring to follow Google style by adding an Args section
describing parameters (types and meaning) and a Returns section describing the
AsyncGenerator or return value (including yielded StreamChunk types), and
include any raised exceptions or side-effects (e.g., workspace snapshot, event
emissions) where relevant so the docstrings for these methods conform to the
repository guideline.

massgen/orchestrator_collaborators/skills_config_validator.py-65-68 (1)

65-68: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard the built-in skills probe with is_dir() before calling iterdir().

If massgen/skills exists but is a file or broken symlink, this path raises from iterdir() and skips the clearer RuntimeError below. Mirror the external-skills check here so broken installs fail deterministically.

Suggested fix

         builtin_skills_dir = _builtin_skills_dir()
-        has_builtin_skills = builtin_skills_dir.exists() and any(
+        has_builtin_skills = (
+            builtin_skills_dir.exists()
+            and builtin_skills_dir.is_dir()
+            and any(
             builtin_skills_dir.iterdir(),
-        )
+            )
+        )

🤖 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 `@massgen/orchestrator_collaborators/skills_config_validator.py` around lines
65 - 68, The probe for built-in skills should check that builtin_skills_dir is a
directory before calling iterdir(): update the logic around builtin_skills_dir
(returned by _builtin_skills_dir()) and the has_builtin_skills assignment to
mirror the external-skills check by adding builtin_skills_dir.is_dir() as a
prerequisite to calling builtin_skills_dir.iterdir(); this ensures broken
symlinks or files named massgen/skills don't cause an exception and will instead
fall through to the existing RuntimeError handling.

massgen/orchestrator_collaborators/snapshot_manager.py-201-220 (1)

201-220: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix type hints and return type consistency.

1. Lines 204-205 use implicit Optional which violates PEP 484. Use explicit | None syntax.
2. Line 220 returns None but the method signature declares -> str. This should be -> str | None.

🔧 Proposed fix

     async def save_agent_snapshot(
         self,
         agent_id: str,
-        answer_content: str = None,
-        vote_data: dict[str, Any] = None,
+        answer_content: str | None = None,
+        vote_data: dict[str, Any] | None = None,
         is_final: bool = False,
         context_data: Any = None,
-    ) -> str:
+    ) -> str | None:
         """See Orchestrator._save_agent_snapshot."""
         orch = self._orchestrator
         logger.info(

🤖 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 `@massgen/orchestrator_collaborators/snapshot_manager.py` around lines 201 -
220, The signature of save_agent_snapshot uses implicit Optional types and
declares a non-optional return; update the type hints to use explicit
union-with-None and make the return type nullable: change the function signature
in save_agent_snapshot to return "str | None" and change parameters to
"answer_content: str | None = None", "vote_data: dict[str, Any] | None = None",
and "context_data: Any | None = None" so annotations are PEP 484-compliant and
match the code path that returns None when agent not found.

massgen/orchestrator_collaborators/subagent_lifecycle_coordinator.py-84-89 (1)

84-89: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Background task exceptions may be silently lost.

The task created by loop.create_task() is not stored, so any unhandled exception in _maybe_interrupt_background_wait_for_agent will be silently dropped. Consider adding an error handler or storing the task reference.

🛡️ Proposed fix with error callback

-        loop.create_task(
+        task = loop.create_task(
             orch._maybe_interrupt_background_wait_for_agent(
                 agent_id,
                 trigger=trigger,
             ),
         )
+        task.add_done_callback(
+            lambda t: logger.debug(
+                f"[SubagentLifecycle] Background interrupt task for {agent_id} "
+                f"{'failed: ' + str(t.exception()) if t.exception() else 'completed'}"
+            ) if t.exception() else None
+        )

🤖 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 `@massgen/orchestrator_collaborators/subagent_lifecycle_coordinator.py` around
lines 84 - 89, The created background task for
orch._maybe_interrupt_background_wait_for_agent is not stored, so exceptions can
be lost; modify the code that calls loop.create_task(...) to capture the Task
object (e.g., task = loop.create_task(...)) and attach a done callback that
checks task.exception() and logs or forwards the error (or re-raises) via the
orchestrator's logger; alternatively register the task in an in-flight tasks
collection on the orch instance so it can be awaited/cancelled on shutdown and
its exceptions handled centrally.

massgen/orchestrator_collaborators/workspace_modal_presenter.py-61-66 (1)

61-66: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Log workspace-resolution failures instead of silently dropping the workspace tab.

If get_current_workspace() or the directory probe throws here, the code quietly falls back to the answer-only modal. That changes the visible result but leaves no clue why the workspace tab disappeared. Please log the exception before falling back so regressions in filesystem-manager integration are diagnosable.

🤖 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 `@massgen/orchestrator_collaborators/workspace_modal_presenter.py` around lines
61 - 66, The try/except that calls fm.get_current_workspace() in
workspace_modal_presenter.py currently suppresses all errors; change the except
to "except Exception as e:" and log the failure before falling back (e.g., call
logger.exception or logger.error with the exception) so the message includes
context like "Failed to resolve current workspace" and the exception
information; keep the existing fallback behavior that leaves workspace_path
unset. Reference the existing call sites get_current_workspace(), Path.is_dir(),
Path.iterdir(), and the workspace_path variable when adding the log.

massgen/frontend/displays/textual_terminal_display.py-741-749 (1)

741-749: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add Google-style docstrings for the touched delegating methods.

The capability-probe change touches TextualTerminalDisplay.__init__, and the new wrapper methods here are also changed, but they still use missing or non-Google-style docstrings. Please bring these updated methods in line with the repo’s Python docstring requirement.

As per coding guidelines, **/*.py: For new or changed functions, include Google-style docstrings.

Also applies to: 832-871, 3957-4000, 4366-4384

🤖 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 `@massgen/frontend/displays/textual_terminal_display.py` around lines 741 -
749, The modified TextualTerminalDisplay class methods lack Google-style
docstrings; update TextualTerminalDisplay.__init__ and any delegating/wrapper
methods you changed to include proper Google-style docstrings (one-line summary,
blank line, Args: and Returns: sections as applicable) so they follow the repo
convention—ensure each docstring documents parameters like
env/on_error/refresh_rate or the wrapper's input/return values and side effects,
and add them to the other touched methods referenced in the review (the
delegating/wrapper methods within the same class).

massgen/orchestrator_collaborators/criteria_evolution_runner.py-434-434 (1)

434-434: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

zip() without strict=True may silently truncate if list lengths differ.

If parse_evolution_response returns an evolved list with a different length than old_criteria, the zip() will silently drop items, causing an incorrect evolved_count. Consider using strict=True (Python 3.10+) or adding an explicit length check.

Suggested fix

-        evolved_count = sum(1 for old_c, new_c in zip(evolution_data["current_criteria"], evolved) if old_c.text != new_c.text)
+        evolved_count = sum(1 for old_c, new_c in zip(evolution_data["current_criteria"], evolved, strict=True) if old_c.text != new_c.text)

Or add a defensive length check before the zip if strict mode causes issues with the expected behavior.

Also applies to: 465-465

massgen/orchestrator_collaborators/final_result_reporter.py-402-414 (1)

402-414: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Build the agent status payload by aid, not by zipping two dicts.

This assumes orch.agents.values() and orch.agent_states.values() always stay in identical order. If they ever diverge, the status response can pair one agent's backend status with another agent's coordination state.

🤖 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 `@massgen/orchestrator_collaborators/final_result_reporter.py` around lines 402
- 414, The current payload builds "agents" by zipping orch.agents.keys() with
zip(orch.agents.values(), orch.agent_states.values()), which can mis-pair agent
objects and their states if dict ordering diverges; instead iterate over
orch.agents.items() (for aid, agent in orch.agents.items()), look up the
corresponding state via orch.agent_states[aid] (or .get(aid) with a safe
fallback), and construct the map using agent.get_status(), state.answer, and
state.has_voted so each aid is guaranteed to be paired with its correct
coordination state.

massgen/orchestrator_collaborators/persona_injector.py-195-196 (1)

195-196: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against None return from get_log_session_dir().

If get_log_session_dir() returns None, the path operation on line 196 will raise a TypeError. Add a guard or early return.

🛡️ Suggested fix

             log_dir = _orch_mod.get_log_session_dir()
+            if log_dir is None:
+                logger.debug("[Orchestrator] No log session dir; skipping persona save")
+                return
             personas_file = log_dir / "generated_personas.yaml"

🤖 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 `@massgen/orchestrator_collaborators/persona_injector.py` around lines 195 -
196, Guard against get_log_session_dir() returning None before using it: check
the value of log_dir (returned by _orch_mod.get_log_session_dir()) and if it's
None either return early or raise a clear error, otherwise proceed to construct
personas_file = log_dir / "generated_personas.yaml"; ensure any downstream code
in PersonaInjector (or persona_injector.py) handles the early-return or error
case consistently.

massgen/configs/tools/web-search/parallel_search_example.yaml (1)

28-30: ⚡ Quick win

Use a cost-effective default model in this example config.

Please switch this example to a cost-effective default (or add a short note explaining why a premium model is required here).

As per coding guidelines, massgen/configs/**/*.yaml: “Prefer cost-effective models (gpt-5-nano, gpt-5-mini, gemini-2.5-flash)”.

🤖 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 `@massgen/configs/tools/web-search/parallel_search_example.yaml` around lines
28 - 30, The example uses a premium Claude model ("claude-sonnet-4-20250514");
update the example to use a cost-effective default per guidelines by replacing
the model value with a recommended cheaper model (e.g., "gpt-5-nano" or
"gpt-5-mini" or "gemini-2.5-flash") or add a one-line comment next to the model
key explaining why a premium Claude model is required; target the YAML keys
"type", "model", and the surrounding example block (where "mcp_servers" is
defined) when making the change.

Fix all unresolved CodeRabbit comments on this PR:

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