Skip to content

feat: parallelize the task, on by default — and close every open issue - #156

Merged
sebyx07 merged 2 commits into
mainfrom
fix/all-open-issues
Aug 8, 2026
Merged

sebyx07 merged 2 commits into
mainfrom
fix/all-open-issues

Conversation

@sebyx07

@sebyx07 sebyx07 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Two things at once: the hive redesign the previous PR got wrong, and every open issue in the tracker.

The unit of parallelism is one task, not several

v0.1.83 shipped hive mode as batching a PR group's remaining tasks into one lead session. That was the wrong unit — the ask was "parallelize each task". So the batching is deleted, not deprecated: HiveBatch, plan_hive_batch, the TASKS COMPLETE: manifest, the multi-checkoff and the batch index math are gone; core/loop_working_hive.py and core/task_runner_hive.py are removed; core/hive.py goes 275 → 52 LOC; and loop_working_stage.py is byte-identical to its pre-#155 self.

What replaces it is smaller. The loop is what it always was — one session per task, one task checked off, TASK COMPLETE. The agent running that session is now a lead: it may cut its own single task into pieces with disjoint write sets and hand each to a hive-worker subagent, doing everything that overlaps itself. The orchestrator cannot tell a fanned-out session from a solo one except by what lands in the tree, so there is no new bookkeeping to get wrong.

Proof the restore is exact: the five SHA-256 prompt digests from before #155 were kept unchanged and pass against parallel=False.

On by default. TaskOptions.parallel = True; --parallel / --no-parallel replaces --parallel-tasks — deleted, not aliased (breaking, noted in the changelog). CLAUDETM_HIVE_MAX_PARALLEL is a ceiling, not a target: the lead sizes its own team, and zero or one worker are both legitimate answers. One shared checkout, never worktrees — the file set is the only lock, and the lead alone runs git.

Coloured subagent output. Each worker gets a colour keyed on its tool_use_id, so it keeps that colour for life and two concurrent hive-workers are separable at a glance (↳ [hive-worker#1]). Palette is disjoint from the status colours so a name never reads as success/error. NO_COLOR and non-TTY suppress it; log files never receive escapes.

Every open issue

# What was wrong
#153 merge-pr deleted the checked-out branch instead of the merged PR's — in the wild it destroyed an unrelated open PR's branch, and ran even though the merge had failed. Both call sites (CLI and orchestrator) now share one policy in core/git_branch.py: the PR's head branch only, only after a merge confirmed against GitHub, never the base, -d first with -D only when the remote ref exists and nothing is unpushed. _GitOps._delete_local_branch (unconditional -D) deleted.
#152 merge-pr printed "merged successfully" and exited 0 when the merge did not happen — verified on two PRs left OPEN with mergedAt=null. The exit code is now authoritative, the merge is verified against the repository before anything is deleted, and a branch-policy refusal names --admin.
#147 The review-stage timeout warned and proceeded toward merge while the CI-stage timeout blocked — same timer, opposite posture, and the checks still pending there are exactly the late-reporting review bots. One shared policy now: block by default, --admin force-advances. A tolerated failure can no longer cause the wait or the block.
#146 Every user- and machine-facing description said PRs merge "when CI passes and approved"; no approval concept existed anywhere in src/. Prose corrected at 13 sites, and reviewDecision is now actually read: CHANGES_REQUESTED blocks an auto-merge. Deliberately even under --admin--admin is passed on essentially every run here, so honouring it would delete the gate; it cannot deadlock, because a human clears it on GitHub and the next cycle proceeds.
#116 A 404 on a never-started job's logs surfaced as GitHubError every cycle. It is now a classified signal (CILogsUnavailableError, CIJob.never_started), and a failure GitHub has declared permanent — billing lock, Actions disabled — trips a breaker that posts one explanatory PR comment (idempotent via a marker) and stops cleanly. Partly stale: on current main that case already spent zero agent sessions; the real gaps were two wasted re-runs and a silent block.
#100 The container mounted the operator's ~/.claude, so the agent ran as — and billed — whoever owned that token. A real cross-developer leak. Every mount is gone from Dockerfile, docker-compose.yml, all five compose examples, the examples README/.env.example, the Swarm stack and the Kubernetes manifest. Auth is a scoped, rotatable api-key profile in a named volume, selected by CLAUDETM_PROFILE.

Found on the way (not in any issue)

  • A layering inversion the merge-pr deletes the checked-out local branch instead of the merged PR's branch (data loss) #153 fix introduced: core/ importing from cli_commands/. The shared branch policy now lives in core/git_branch.py and the CLI imports down into it, like every other entry layer.
  • Four compose examples never parsed. ${CLAUDETM_PASSWORD:?Error: ...} unquoted makes YAML read the list item as a map. Fixed and verified with docker compose config.
  • A Kubernetes shortcut that doesn't work. Injecting a bare ANTHROPIC_API_KEY fails CredentialManager.get_valid_token(), which short-circuits only for profile.type == "api-key". The manifest uses an initContainer + profile, and the doc says why so nobody "simplifies" it back.

Deliberately not done

core/stages/review_stage.py still imports cli_commands.ci_helpers — the same inversion, but pre-existing and untouched by this work.

Gate

pytest 5957 passed, 3 skipped · ruff check . ✓ · ruff format . ✓ · mypy .421 files · claudetm doctor ✓ · docker compose config ✓ on all six compose files · verify_docs_links 291/291

Built by nine path-disjoint agents in one checkout. Every slice's tests were written failure-case-first and verified to fail before the change; the merge-pr and branch-cleanup regressions run end-to-end against a real temp git repo.

Closes #153, closes #152, closes #147, closes #146, closes #116, closes #100

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • New Features

    • Parallel task fan-out is now enabled by default with --parallel, with --no-parallel available to disable it.
    • Added optional PR creation through merge-pr --create-pr.
    • Added clearer per-worker output labels and colors.
    • Added safer merged-branch cleanup and merge verification.
  • Bug Fixes

    • Auto-merge now correctly blocks unresolved review feedback and CHANGES_REQUESTED.
    • Improved handling of unavailable or permanently blocked CI runs.
    • Review-stage timeouts now block by default unless administrative override is used.
  • Documentation

    • Updated Docker guidance to use scoped, rotatable API-key profiles instead of personal credentials.

**The unit of parallelism is one task, not several.** v0.1.83 shipped hive
mode batching a PR group's remaining tasks into one lead session. That was
the wrong unit: the ask was "parallelize each task". The batching, the
`TASKS COMPLETE:` manifest, the multi-checkoff and the batch index math are
DELETED — `core/loop_working_hive.py` and `core/task_runner_hive.py` are
gone, `core/hive.py` is 275 → 52 LOC, and `loop_working_stage.py` is
byte-identical to its pre-#155 self. The loop is what it always was: one
session per task, one task checked off, `TASK COMPLETE`.

What replaces it is smaller. The agent running a task session is a **lead**:
it may cut *its own single task* into pieces with disjoint write sets and
hand each to a `hive-worker` subagent, doing everything that overlaps
itself. The orchestrator cannot tell a fanned-out session from a solo one
except by what lands in the tree — so there is no new bookkeeping.

**On by default** (`TaskOptions.parallel`, `--parallel`/`--no-parallel`,
replacing `--parallel-tasks`; deleted, not aliased). `CLAUDETM_HIVE_MAX_PARALLEL`
is a ceiling, not a target — the lead sizes its own team, and zero or one
worker are both legitimate answers. One shared checkout, never worktrees;
the file set is the only lock and the lead alone runs git.

Subagent output is now coloured per worker instance, keyed on its
`tool_use_id` so a worker keeps its colour for life and two concurrent
`hive-worker`s are separable at a glance. `NO_COLOR`/non-TTY suppress it;
log files never receive escapes.

Closes every open issue:

- **#153** `merge-pr` deleted the *checked-out* branch, not the merged PR's
  — it destroyed an unrelated open PR's branch in the wild. Both call sites
  fixed (CLI and orchestrator) against one shared policy in
  `core/git_branch.py`: the PR's head branch only, only after a merge
  confirmed against GitHub, never the base, `-d` first with `-D` only when
  nothing is unpushed. `_GitOps._delete_local_branch` (unconditional `-D`)
  deleted.
- **#152** `merge-pr` printed "merged successfully" on a failed merge and
  exited 0, then ran the destructive cleanup anyway. The exit code is now
  authoritative, the merge is verified against GitHub before anything is
  deleted, and a policy refusal names `--admin`.
- **#147** the review-stage timeout warned and proceeded toward merge while
  the CI-stage timeout blocked — same timer, opposite posture. One shared
  policy now: block by default, `--admin` force-advances. Tolerated
  failures can no longer cause the wait or the block.
- **#146** every description claimed PRs merge "when CI passes and
  approved"; no approval concept existed. Prose corrected everywhere, and
  `reviewDecision` is now read: `CHANGES_REQUESTED` blocks an auto-merge
  (deliberately even under `--admin`, since `--admin` is passed on every
  run here — a human clears it on GitHub, so it cannot deadlock).
- **#116** a 404 on a never-started job's logs surfaced as `GitHubError`
  every cycle. It is now a classified signal, and a CI failure GitHub has
  declared permanent (billing lock, Actions disabled) trips a breaker that
  posts one explanatory PR comment and stops. The issue was partly stale:
  on current main that case already spent zero sessions.
- **#100** the container mounted the operator's `~/.claude`, so the agent
  ran as and billed whoever owned that token — a real cross-developer leak.
  Every mount is gone from `Dockerfile`, `docker-compose.yml`, all five
  compose examples, the Swarm stack and the Kubernetes manifest; auth is a
  scoped, rotatable api-key profile in a named volume.

Also fixes a layering inversion introduced along the way (`core/` importing
from `cli_commands/`) and quotes four compose `${VAR:?msg}` lines that made
YAML parse a list item as a map — those examples never parsed.

Closes #153, closes #152, closes #147, closes #146, closes #116, closes #100

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 10ce914d-c8cf-45a3-9717-9ec0a79dfe77

📥 Commits

Reviewing files that changed from the base of the PR and between 0742991 and 09bfdbb.

📒 Files selected for processing (14)
  • CHANGELOG.md
  • docs/docker.md
  • examples/docker-compose/.env.example
  • examples/docker-compose/README.md
  • examples/docker-compose/basic-production.yml
  • examples/docker-compose/development.yml
  • examples/docker-compose/production-monitoring.yml
  • examples/docker-compose/production-with-caddy.yml
  • examples/docker-compose/production-with-nginx.yml
  • src/claude_task_master/api/models_common.py
  • src/claude_task_master/api/models_task.py
  • tests/api/test_routes_config.py
  • tests/api/test_routes_task.py
  • tests/core/test_agent_message.py
📝 Walkthrough

Walkthrough

This PR replaces PR-group hive batching with default-on per-task worker fan-out. It adds merge verification, safe branch cleanup, review and CI timeout policies, permanent external CI detection, scoped Docker API-key profiles, and corresponding API, CLI, MCP, documentation, and test updates.

Changes

Parallel execution and public contracts

Layer / File(s) Summary
Per-task fan-out
src/claude_task_master/core/*, src/claude_task_master/api/*, src/claude_task_master/cli_commands/*, src/claude_task_master/mcp/*
parallel replaces parallel_tasks and defaults to enabled. A single task can split across hive-worker subagents with disjoint write sets and shared-checkout rules.
Batch removal and validation
src/claude_task_master/core/hive.py, src/claude_task_master/core/prompts_working_hive.py, tests/core/*
Hive batch planning, manifests, and batch completion handling are removed. Prompt, worker, cursor, and completion behavior now operates per task.
Worker output markers
src/claude_task_master/core/console.py, src/claude_task_master/core/agent_message.py
Workers receive stable ordinal labels and optional ANSI colors. Lead output remains unmarked, and worker completion markers are excluded from accumulated output.

Merge and CI controls

Layer / File(s) Summary
PR resolution and merge verification
src/claude_task_master/cli_commands/pr_resolution.py, src/claude_task_master/cli_commands/merge_finalize.py, src/claude_task_master/cli_commands/fix_pr.py
PR inputs can resolve from numbers, URLs, current branches, or optional PR creation. Merge success requires confirmed GitHub state before checkout or cleanup.
Safe branch cleanup and review gating
src/claude_task_master/core/git_branch.py, src/claude_task_master/core/stages/merge_stage.py, src/claude_task_master/github/client_pr_*
Cleanup targets the verified PR head branch. CHANGES_REQUESTED blocks auto-merge, and GitHub review decisions are normalized into PRStatus.
Timeout and external CI handling
src/claude_task_master/core/stages/base.py, src/claude_task_master/core/stages/review_stage.py, src/claude_task_master/core/stages/pr_fix_stage.py, src/claude_task_master/github/ci_infra.py, src/claude_task_master/github/ci_logs.py
Review timeouts block unless admin mode overrides them. Never-started CI jobs use a distinct error path. Permanent external CI blocks stop retries and can produce one explanatory PR notice.

Deployment and documentation

Layer / File(s) Summary
Scoped container authentication
Dockerfile, docker-compose.yml, docs/docker.md, examples/docker-compose/*, README.md
Docker deployments use the claudetm-profiles volume and CLAUDETM_PROFILE instead of personal ~/.claude credentials. Kubernetes creates per-pod profiles from API-key Secrets.
Workflow documentation
CHANGELOG.md, CLAUDE.md, docs/api-reference.md, docs/mcp-tools.md, README.md
Documentation describes default-on per-task parallelism, review gating, timeout behavior, the concurrency safety ceiling, and auto-merge without an approval requirement.

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

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The task-level hive redesign, worker coloring, and related parallel-execution changes lack corresponding requirements in the linked issues. Split the parallel-execution redesign into a separate pull request or link an issue that defines its requirements and acceptance criteria.
✅ 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 identifies the primary change: task parallelization enabled by default, with related issue fixes also present.
Linked Issues check ✅ Passed The changes satisfy issues [#153], [#152], [#147], [#146], [#116], and [#100], including merge safety, timeout policy, review checks, CI handling, and scoped credentials.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 60.00%.
✨ 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 fix/all-open-issues

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

🧹 Nitpick comments (9)
tests/core/test_prompts_working.py (1)

1279-1293: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider moving the fan-out tests into their own module.

The new classes push this file to roughly 1530 lines. The house limit is 500 lines per file. A focused module such as tests/core/test_prompts_working_hive.py would hold TestParallelOffIsByteIdentical, TestParallelIsPurelyAdditive, TestFanOutRules, and TestMaxParallelPlumbing without changing any assertion. The digest baselines are a nice touch, so keeping them grouped in one place helps.

Based on learnings: "In the Python durability test suite (tests/core/*.py), keep each test module under the 500-line limit... split scenarios into focused modules rather than growing a single file". As per coding guidelines: "Keep each file at or below 500 lines of code; split larger modules according to single responsibility and SOLID principles."

🤖 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/core/test_prompts_working.py` around lines 1279 - 1293, Move the
fan-out test section from test_prompts_working.py into a focused module such as
test_prompts_working_hive.py, including TestParallelOffIsByteIdentical,
TestParallelIsPurelyAdditive, TestFanOutRules, and TestMaxParallelPlumbing.
Preserve all assertions, digest baselines, imports, and shared helpers required
by those tests, and keep each test module under the 500-line limit.

Sources: Coding guidelines, Learnings

src/claude_task_master/cli_commands/merge_finalize.py (2)

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

Small polish for checkout_and_pull.

Two optional points:

  1. The docstring has no Args/Returns sections. Other public functions in this module use the full Google style, so this one stands out.
  2. run_git("pull", timeout=120) depends on the base branch having an upstream. If it does not, the pull fails, the function returns False, and cleanup is skipped. That is a safe outcome, so this is only a readability note.
♻️ Optional docstring expansion
 def checkout_and_pull(branch: str) -> bool:
-    """Checkout a branch and pull latest changes. Returns True on success."""
+    """Checkout a branch and pull the latest changes.
+
+    Args:
+        branch: Branch to check out.
+
+    Returns:
+        True when both the checkout and the pull succeed.
+    """

As per coding guidelines: "Use Google-style docstrings for public APIs".

🤖 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/claude_task_master/cli_commands/merge_finalize.py` around lines 191 -
201, Expand the checkout_and_pull docstring to Google style by documenting the
branch argument and the boolean return value, while preserving the existing
checkout and pull behavior.

Source: Coding guidelines


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

Consider whether "not authorized" belongs in POLICY_REFUSAL_MARKERS.

A "not authorized" error usually means the token lacks permission. --admin cannot help in that case, because --admin also requires admin rights. The hint text does mention "requires admin rights on the repo", so the advice is not wrong, only sometimes unhelpful. Keep it if you prefer a hint over silence.

🤖 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/claude_task_master/cli_commands/merge_finalize.py` around lines 46 - 62,
Review POLICY_REFUSAL_MARKERS and remove "not authorized" unless the merge error
handling can reliably distinguish a base-branch policy refusal from a
token-permission failure. Ensure ADMIN_HINT is shown only when --admin could
plausibly override the refusal, while preserving the existing markers and
case-insensitive matching.
tests/cli_commands/test_pr_resolution.py (1)

22-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Optional: add a case for the #52 input form.

parse_pr_input accepts a leading #, but no test exercises that branch. A one-line case would cover it.

🧪 Optional test addition
     def test_url_is_parsed(self) -> None:
         client = MagicMock()
         assert resolve_pr_number(client, "https://github.com/owner/repo/pull/52") == 52
+
+    def test_hash_prefixed_number_is_parsed(self) -> None:
+        client = MagicMock()
+        assert resolve_pr_number(client, "`#52`") == 52

As per path instructions: "Review test coverage and edge cases."

🤖 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/cli_commands/test_pr_resolution.py` around lines 22 - 36, Add a test
case in TestExplicitInput covering resolve_pr_number with the “#52” input form,
and assert it returns 52 without requiring Git or branch lookup.

Source: Path instructions

tests/cli_commands/test_merge_finalize.py (1)

113-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Optional: checkout_and_pull has no direct test.

It is the last gate before delete_merged_branch runs, so its failure paths matter. A short test for the "checkout fails" and "pull fails" cases would lock in the False return.

🧪 Optional test sketch
class TestCheckoutAndPull:
    def test_checkout_failure_returns_false(self) -> None:
        with patch.object(merge_finalize, "run_git", return_value=_proc(returncode=1)):
            assert merge_finalize.checkout_and_pull("main") is False

    def test_pull_failure_returns_false(self) -> None:
        with patch.object(
            merge_finalize, "run_git", side_effect=[_proc(), _proc(returncode=1)]
        ):
            assert merge_finalize.checkout_and_pull("main") is False

As per path instructions: "Review test coverage and edge cases."

🤖 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/cli_commands/test_merge_finalize.py` around lines 113 - 122, Add direct
coverage for merge_finalize.checkout_and_pull: test that a failed checkout
returns False, and that a successful checkout followed by a failed pull also
returns False. Patch merge_finalize.run_git with the appropriate return values,
using the existing process-result helper, while leaving successful behavior
unchanged.

Source: Path instructions

src/claude_task_master/cli_commands/pr_resolution.py (1)

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

Optional: the fallback message can confuse a user who already passed --create-pr.

If create_pr is true and the current branch is a default branch, the code falls through to Line 165 and suggests claudetm merge-pr --create-pr, which the user just used. A branch-specific message would explain the real reason. The CLI normally blocks this earlier through validate_not_default_branch(), so this only affects direct callers of resolve_pr_number.

♻️ Optional message split
     branch = get_current_branch()
-    if create_pr and branch and branch not in DEFAULT_BRANCHES:
-        created = open_pr_for_current_branch(branch)
-        if created is not None:
-            return created
-        raise typer.Exit(1)
+    if create_pr:
+        if branch and branch not in DEFAULT_BRANCHES:
+            created = open_pr_for_current_branch(branch)
+            if created is not None:
+                return created
+            raise typer.Exit(1)
+        console.error(f"Cannot open a PR from '{branch or 'an unknown branch'}'.")
+        console.info("Checkout a feature branch first: git checkout -b <branch>")
+        raise typer.Exit(1)
🤖 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/claude_task_master/cli_commands/pr_resolution.py` around lines 156 - 166,
Update the fallback messaging in resolve_pr_number so create_pr=true on a
default branch reports that PR creation is unavailable for the current branch
instead of suggesting --create-pr again. Preserve the existing generic no-PR
guidance for other cases and retain the current exit behavior.
src/claude_task_master/cli_commands/fix_pr.py (1)

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

Optional: the fallback normalization here duplicates verify_merged.

verify_merged already normalizes head_branch and base_branch through _text, so both are str | None by construction. The isinstance guards at Lines 135-139 only protect against a PRStatus mock supplying a non-string base_branch. Also, merge_failure_hint returns None whenever admin is true, so the extra not admin on Line 119 is already implied. Neither is harmful; simplifying would keep the branch logic easier to read.

♻️ Optional simplification
-        if hint is None and not admin and verification.state == "OPEN":
+        if hint is None and verification.state == "OPEN":
🤖 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/claude_task_master/cli_commands/fix_pr.py` around lines 113 - 139,
Optionally simplify the post-verification branch in the fix_pr command: rely on
verify_merged’s normalized str | None base_branch and head_branch results
instead of repeating isinstance guards and fallback normalization, and remove
the redundant not admin condition when checking a missing merge_failure_hint.
Preserve the existing fallback behavior and admin-specific messaging.
tests/github/test_client_pr_review_decision.py (1)

39-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the configured test-name pattern.

Rename added test methods to test_<function>_<scenario>_<expected_result>. This makes failures identify the tested operation and expected result.

  • tests/github/test_client_pr_review_decision.py#L39-L79: include _parse_pr_status_response and the expected parsed decision in each test name.
  • tests/core/test_stages_review_timeout.py#L110-L204: include the reviewed stage method, timeout condition, and expected transition in each test name.
  • tests/core/test_pr_context.py#L352-L404: rename the test to identify save_ci_failures, the never-started job condition, and the warning result.
  • tests/core/test_git_branch_cleanup.py#L30-L137: include delete_merged_branch or current_branch, the branch condition, and the expected safe action.

Based on coding guidelines, “One test file per module with descriptive test names following pattern: test_function_scenario_expected_result.”

🤖 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/github/test_client_pr_review_decision.py` around lines 39 - 79, Rename
the added tests to follow test_<function>_<scenario>_<expected_result>. In
tests/github/test_client_pr_review_decision.py lines 39-79, include
_parse_pr_status_response and the expected parsed decision; in
tests/core/test_stages_review_timeout.py lines 110-204, include the reviewed
stage method, timeout condition, and expected transition; in
tests/core/test_pr_context.py lines 352-404, identify save_ci_failures, the
never-started job condition, and warning result; and in
tests/core/test_git_branch_cleanup.py lines 30-137, identify
delete_merged_branch or current_branch, the branch condition, and expected safe
action.

Source: Coding guidelines

src/claude_task_master/github/ci_infra.py (1)

36-51: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider narrowing the broadest phrases.

"quota" and "payment" are short substrings. An annotation such as "disk quota exceeded" or "payment gateway unreachable" would match and trip a terminal block.

The zero-steps precondition in external_block_reason already keeps most ordinary failures away from this list, so the risk is small. Still, the consequence of a false positive here is a permanent stop rather than a bounded retry, so a slightly more specific phrase is cheap insurance.

♻️ Suggested narrowing
     "actions is disabled",
     "actions are disabled",
     "workflows are disabled",
     "has been suspended",
-    "payment",
-    "quota",
+    "payment method",
+    "spending quota",
+    "usage quota",
🤖 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/claude_task_master/github/ci_infra.py` around lines 36 - 51, Narrow the
broad matches in _PERMANENT_PHRASES by replacing the generic "quota" and
"payment" entries with more specific platform-side phrases that identify account
quota or billing/payment problems. Preserve the existing terminal-block matching
behavior and all other phrases unchanged.
🤖 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 `@CHANGELOG.md`:
- Line 20: Update the changelog entry for `#146` to accurately state that the code
handles reviewDecision by blocking merges when changes are requested, while an
approving review is not required and does not gate merging.

In `@docs/docker.md`:
- Around line 1127-1130: Update the “Agent Credential Not Found” troubleshooting
section to state that a missing CLAUDETM_PROFILE profile causes
ProfileManager.resolve_active to raise ProfileError, not an OAuth fallback;
revise the recovery steps to address correcting or removing the invalid profile,
and reserve OAuth fallback guidance for runs with no selected profile.
- Around line 753-760: Update the standalone Compose example containing the
claudetm-profiles mount to add a top-level volumes declaration for
claudetm-profiles with external enabled, so Docker Compose recognizes it as an
existing external volume.
- Around line 879-892: Update the claudetm service startup configuration around
CLAUDETM_PASSWORD and the claudetm_password secret so the mounted secret at
/run/secrets/claudetm_password populates and exports CLAUDETM_PASSWORD when no
deployer-provided password is set, before claudetm-server starts. Preserve an
explicitly configured CLAUDETM_PASSWORD and ensure the existing secret remains
available to the service.
- Around line 997-1000: Update the pod securityContext for the agent-profile
emptyDir volume to grant writable access to claudetm (UID 1000), preferably by
adding fsGroup: 1000; otherwise prepare the volume ownership in a root init
container while keeping application containers non-root.

In `@examples/docker-compose/basic-production.yml`:
- Around line 38-42: Replace the hard-coded external profile volume with the
required CLAUDETM_PROFILES_VOLUME variable in
examples/docker-compose/basic-production.yml:38-42,
production-with-nginx.yml:42-46, production-with-caddy.yml:42-46, and
production-monitoring.yml:36-40; use a distinct development value in
development.yml:39-43. Update all setup and verification commands in
examples/docker-compose/README.md:23-35 to require a unique per-deployment
volume value and reflect the development value.

In `@README.md`:
- Line 728: Update the Docker deployment command’s volume mount to map
~/workspace to /app/project instead of /home/claudetm/workspace, matching the
Dockerfile WORKDIR and ensuring the server operates on the intended checkout.
- Line 727: Replace every host GitHub CLI configuration mount with a dedicated
least-privilege agent credential supplied through the deployment platform’s
secret mechanism. Apply this consistently at README.md:727-727,
docker-compose.yml:58-61, docs/docker.md:49-50, docs/docker.md:74-75,
docs/docker.md:753-760, and all five files under examples/docker-compose/;
remove any instructions requiring ~/.config/gh mounts.

In `@src/claude_task_master/api/models_task.py`:
- Around line 171-176: Update the TaskInitRequest model to configure Pydantic
with ConfigDict(extra="forbid"), ensuring unknown fields such as parallel_tasks
are rejected with a 422 response instead of being discarded. Add a regression
test for the request route that submits parallel_tasks and verifies the 422
validation response.

In `@src/claude_task_master/core/state_models.py`:
- Around line 60-66: Update src/claude_task_master/core/state_models.py:60-66 to
accept legacy options.parallel_tasks during state loading, while keeping
model_dump() output canonical under options.parallel and preserving the default
for states without either key. Add a regression test in
tests/core/test_state.py:77-100 through StateManager.load_state() that loads
parallel_tasks: false and verifies the resulting options.parallel is false.

In `@src/claude_task_master/mcp/tool_handlers_control.py`:
- Line 196: Update the state-loading/schema migration flow associated with the
control tool’s parallel setting to recognize legacy parallel_tasks, map its
value to parallel before unknown fields are discarded, and preserve the existing
default for states without the legacy field. Add a regression test covering an
older state.json containing parallel_tasks and verify resumed behavior uses the
migrated value.

In `@tests/core/test_agent_message.py`:
- Around line 875-878: Update the logger assertion loop in the test to inspect
the raw arguments from log_tool_use and log_tool_result instead of applying
ANSI_RE to repr(call). Assert directly that the expected arguments contain no
ANSI escape sequences, preserving coverage for both call_args_list collections.
- Around line 705-953: Split the oversized test files while preserving all
existing scenarios and assertions: move the worker-coloring helpers and
TestSubagentColoring/TestSubagentColoringDisabled from
tests/core/test_agent_message.py:705-953 into a focused module; move the
color-gating and SubagentPalette scenarios from
tests/core/test_console.py:1195-1406 into focused module(s); and move the
configuration-option scenarios from tests/mcp/test_tools_control.py:401-432 into
a focused module. Ensure each resulting Python file stays within the 500-line
repository limit, or document an approved exception if the
one-test-file-per-module rule prevents the split.

---

Nitpick comments:
In `@src/claude_task_master/cli_commands/fix_pr.py`:
- Around line 113-139: Optionally simplify the post-verification branch in the
fix_pr command: rely on verify_merged’s normalized str | None base_branch and
head_branch results instead of repeating isinstance guards and fallback
normalization, and remove the redundant not admin condition when checking a
missing merge_failure_hint. Preserve the existing fallback behavior and
admin-specific messaging.

In `@src/claude_task_master/cli_commands/merge_finalize.py`:
- Around line 191-201: Expand the checkout_and_pull docstring to Google style by
documenting the branch argument and the boolean return value, while preserving
the existing checkout and pull behavior.
- Around line 46-62: Review POLICY_REFUSAL_MARKERS and remove "not authorized"
unless the merge error handling can reliably distinguish a base-branch policy
refusal from a token-permission failure. Ensure ADMIN_HINT is shown only when
--admin could plausibly override the refusal, while preserving the existing
markers and case-insensitive matching.

In `@src/claude_task_master/cli_commands/pr_resolution.py`:
- Around line 156-166: Update the fallback messaging in resolve_pr_number so
create_pr=true on a default branch reports that PR creation is unavailable for
the current branch instead of suggesting --create-pr again. Preserve the
existing generic no-PR guidance for other cases and retain the current exit
behavior.

In `@src/claude_task_master/github/ci_infra.py`:
- Around line 36-51: Narrow the broad matches in _PERMANENT_PHRASES by replacing
the generic "quota" and "payment" entries with more specific platform-side
phrases that identify account quota or billing/payment problems. Preserve the
existing terminal-block matching behavior and all other phrases unchanged.

In `@tests/cli_commands/test_merge_finalize.py`:
- Around line 113-122: Add direct coverage for merge_finalize.checkout_and_pull:
test that a failed checkout returns False, and that a successful checkout
followed by a failed pull also returns False. Patch merge_finalize.run_git with
the appropriate return values, using the existing process-result helper, while
leaving successful behavior unchanged.

In `@tests/cli_commands/test_pr_resolution.py`:
- Around line 22-36: Add a test case in TestExplicitInput covering
resolve_pr_number with the “#52” input form, and assert it returns 52 without
requiring Git or branch lookup.

In `@tests/core/test_prompts_working.py`:
- Around line 1279-1293: Move the fan-out test section from
test_prompts_working.py into a focused module such as
test_prompts_working_hive.py, including TestParallelOffIsByteIdentical,
TestParallelIsPurelyAdditive, TestFanOutRules, and TestMaxParallelPlumbing.
Preserve all assertions, digest baselines, imports, and shared helpers required
by those tests, and keep each test module under the 500-line limit.

In `@tests/github/test_client_pr_review_decision.py`:
- Around line 39-79: Rename the added tests to follow
test_<function>_<scenario>_<expected_result>. In
tests/github/test_client_pr_review_decision.py lines 39-79, include
_parse_pr_status_response and the expected parsed decision; in
tests/core/test_stages_review_timeout.py lines 110-204, include the reviewed
stage method, timeout condition, and expected transition; in
tests/core/test_pr_context.py lines 352-404, identify save_ci_failures, the
never-started job condition, and warning result; and in
tests/core/test_git_branch_cleanup.py lines 30-137, identify
delete_merged_branch or current_branch, the branch condition, and expected safe
action.
🪄 Autofix

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.yml

Review profile: CHILL

Plan: Pro

Run ID: 2ddd5784-25b9-4419-b035-b3c4224c8f09

📥 Commits

Reviewing files that changed from the base of the PR and between db3202b and 0742991.

📒 Files selected for processing (81)
  • CHANGELOG.md
  • CLAUDE.md
  • Dockerfile
  • README.md
  • docker-compose.yml
  • docs/api-reference.md
  • docs/docker.md
  • docs/mcp-tools.md
  • examples/docker-compose/.env.example
  • examples/docker-compose/README.md
  • examples/docker-compose/basic-production.yml
  • examples/docker-compose/development.yml
  • examples/docker-compose/production-monitoring.yml
  • examples/docker-compose/production-with-caddy.yml
  • examples/docker-compose/production-with-nginx.yml
  • src/claude_task_master/api/models_task.py
  • src/claude_task_master/api/routes_info.py
  • src/claude_task_master/api/routes_task.py
  • src/claude_task_master/cli_commands/control.py
  • src/claude_task_master/cli_commands/fix_pr.py
  • src/claude_task_master/cli_commands/merge_finalize.py
  • src/claude_task_master/cli_commands/pr_resolution.py
  • src/claude_task_master/cli_commands/workflow_start.py
  • src/claude_task_master/core/agent.py
  • src/claude_task_master/core/agent_message.py
  • src/claude_task_master/core/agent_phases.py
  • src/claude_task_master/core/console.py
  • src/claude_task_master/core/git_branch.py
  • src/claude_task_master/core/hive.py
  • src/claude_task_master/core/loop_working_hive.py
  • src/claude_task_master/core/loop_working_stage.py
  • src/claude_task_master/core/pr_context_ci.py
  • src/claude_task_master/core/prompts_working.py
  • src/claude_task_master/core/prompts_working_hive.py
  • src/claude_task_master/core/stages/base.py
  • src/claude_task_master/core/stages/ci_stage.py
  • src/claude_task_master/core/stages/git_ops.py
  • src/claude_task_master/core/stages/merge_stage.py
  • src/claude_task_master/core/stages/pr_fix_stage.py
  • src/claude_task_master/core/stages/review_stage.py
  • src/claude_task_master/core/state_models.py
  • src/claude_task_master/core/subagents.py
  • src/claude_task_master/core/task_runner.py
  • src/claude_task_master/core/task_runner_hive.py
  • src/claude_task_master/core/task_runner_session.py
  • src/claude_task_master/github/ci_infra.py
  • src/claude_task_master/github/ci_logs.py
  • src/claude_task_master/github/client_pr_helpers.py
  • src/claude_task_master/github/client_pr_models.py
  • src/claude_task_master/mcp/server_specs.py
  • src/claude_task_master/mcp/tool_handlers_control.py
  • src/claude_task_master/mcp/tool_handlers_task.py
  • tests/api/test_routes_task.py
  • tests/cli/test_start_command.py
  • tests/cli_commands/test_control.py
  • tests/cli_commands/test_fix_pr.py
  • tests/cli_commands/test_merge_finalize.py
  • tests/cli_commands/test_merge_pr_cleanup.py
  • tests/cli_commands/test_merge_pr_loop.py
  • tests/cli_commands/test_pr_resolution.py
  • tests/core/test_agent_message.py
  • tests/core/test_console.py
  • tests/core/test_git_branch_cleanup.py
  • tests/core/test_hive.py
  • tests/core/test_hive_loop.py
  • tests/core/test_pr_context.py
  • tests/core/test_prompts_working.py
  • tests/core/test_stages_approval.py
  • tests/core/test_stages_ci_breaker.py
  • tests/core/test_stages_ci_rerun.py
  • tests/core/test_stages_merged_branch_cleanup.py
  • tests/core/test_stages_review_timeout.py
  • tests/core/test_state.py
  • tests/core/test_subagents.py
  • tests/core/test_task_runner.py
  • tests/core/test_workflow_stages.py
  • tests/github/test_ci_infra.py
  • tests/github/test_ci_logs.py
  • tests/github/test_client_pr_review_decision.py
  • tests/mcp/test_tools_control.py
  • tests/mcp/test_tools_task_management.py
💤 Files with no reviewable changes (4)
  • src/claude_task_master/core/task_runner_hive.py
  • src/claude_task_master/core/stages/git_ops.py
  • tests/core/test_hive_loop.py
  • src/claude_task_master/core/loop_working_hive.py

Comment thread CHANGELOG.md Outdated
Comment thread docs/docker.md
Comment thread docs/docker.md
Comment thread docs/docker.md
Comment on lines +997 to +1000
# Per-pod scratch: the init container recreates the profile on every start,
# so replicas never contend for one writable volume.
- name: agent-profile
emptyDir: {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- docs/docker.md: deployment example ---'
sed -n '940,1025p' docs/docker.md

printf '%s\n' '--- Dockerfile references ---'
fd -i 'Dockerfile' . --type f | while IFS= read -r file; do
  printf '\n### %s\n' "$file"
  rg -n -C 4 'claudetm|\.claudetm|USER|profiles\.json' "$file" || true
done

printf '%s\n' '--- Kubernetes-related context in the document ---'
rg -n -C 5 'initContainers|securityContext|fsGroup|emptyDir|agent-profile|profiles\.json|claudetm' docs/docker.md

Repository: developerz-ai/claude-task-master

Length of output: 31316


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- profile implementation ---'
rg -n -C 8 'profiles\.json|profile add|class .*Profile|def .*profile|mkdir|write_text|open\(' \
  --glob '*.py' --glob '*.ts' --glob '*.js' --glob '*.tsx' --glob '*.jsx' .

printf '%s\n' '--- image user and Kubernetes manifest structure ---'
python3 - <<'PY'
from pathlib import Path
import re

dockerfile = Path("Dockerfile").read_text()
for pattern in (r"USER\s+\S+", r"useradd[^\n]+", r"chown[^\n]+"):
    print(pattern, re.findall(pattern, dockerfile))

text = Path("docs/docker.md").read_text()
start = text.index("### Kubernetes Deployment")
end = text.index("Create the two Secrets", start)
block = text[start:end]
print("\nKubernetes block has securityContext:", "securityContext:" in block)
print("Kubernetes block has runAsUser:", "runAsUser:" in block)
print("Kubernetes block has fsGroup:", "fsGroup:" in block)
print("Kubernetes block has emptyDir:", "emptyDir:" in block)
PY

Repository: developerz-ai/claude-task-master

Length of output: 50389


🌐 Web query:

Kubernetes emptyDir default ownership permissions non-root container fsGroup official documentation

💡 Result:

For Kubernetes pods, an emptyDir volume is initially created by the kubelet [1]. By default, this directory is owned by the root user (UID 0) [2]. Regarding ownership and permissions for non-root containers: 1. fsGroup behavior: While the fsGroup field in a Pod's securityContext is designed to help manage volume permissions for non-root users, official documentation states that fsGroupChangePolicy—which controls how Kubernetes manages these permissions—has no effect on ephemeral volume types such as emptyDir [3][4]. Although some earlier or specific environment-dependent tests may suggest fsGroup can influence emptyDir in certain contexts [5][6], the official Kubernetes documentation excludes emptyDir from the fsGroupChangePolicy mechanism [3]. 2. Default permissions: Without additional configuration, the directory is typically created with permissions that may prevent a non-root container from writing to it if the container user does not belong to the root group [2]. 3. Managing emptyDir permissions: To explicitly control the permissions of an emptyDir volume, Kubernetes introduced an optional mode field within the EmptyDirVolumeSource specification [7]. This field allows users to set the permission bits (e.g., 0777) for the directory. If this field is not specified, it defaults to 0777, though this may interact with other security settings [7]. In summary, for non-root containers, relying solely on fsGroup to manage emptyDir ownership is not supported by the official fsGroupChangePolicy mechanism [3][4]. Users requiring specific access for non-root containers should consider the mode field if available in their cluster version, or configure the container environment/entrypoint to handle the directory permissions appropriately [7][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- candidate profile files ---'
fd -i 'profile|credential|config' . --type f \
  | grep -vE '(^|/)(tests|node_modules|\.git)/' \
  | head -200

printf '%s\n' '--- exact profiles.json references outside tests ---'
rg -n -C 6 'profiles\.json|profile add|api-key profile' . \
  -g '!tests/**' -g '!node_modules/**' -g '!*lock*' \
  | head -300

printf '%s\n' '--- relevant package layout ---'
fd -t d -d 3 . | head -150

Repository: developerz-ai/claude-task-master

Length of output: 25229


🏁 Script executed:

#!/bin/bash
set -eu

python3 - <<'PY'
from pathlib import Path
import re

dockerfile = Path("Dockerfile").read_text()
doc = Path("docs/docker.md").read_text()

assert re.search(r"USER\s+claudetm\b", dockerfile), "image does not switch to claudetm"
assert re.search(r"useradd\s+--uid\s+1000\s+--gid\s+1000\s+.*claudetm", dockerfile), \
    "claudetm UID/GID 1000 not found"
assert "chown -R claudetm:claudetm /home/claudetm /app" in dockerfile, \
    "image ownership setup not found"

start = doc.index("### Kubernetes Deployment")
end = doc.index("Create the two Secrets", start)
manifest = doc[start:end]

checks = {
    "init container mounts profile path": (
        "initContainers:" in manifest
        and "mountPath: /home/claudetm/.claudetm" in manifest
    ),
    "application container mounts profile path": (
        manifest.count("mountPath: /home/claudetm/.claudetm") == 2
    ),
    "profile volume is emptyDir": "emptyDir: {}" in manifest,
    "pod has no fsGroup": "fsGroup:" not in manifest,
    "pod has no pod-level runAsUser": "runAsUser:" not in manifest,
}
for name, result in checks.items():
    print(f"{name}: {'PASS' if result else 'FAIL'}")

if not all(checks.values()):
    raise SystemExit("manifest assumptions did not hold")

print("\nEffective image user: claudetm (UID/GID 1000)")
print("Mounted emptyDir replaces the image directory at /home/claudetm/.claudetm")
print("The init container and application container both use that mounted path")
print("No manifest-level group ownership or writable-volume override is configured")
PY

Repository: developerz-ai/claude-task-master

Length of output: 623


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- src/claude_task_master/core/profiles.py outline ---'
ast-grep outline src/claude_task_master/core/profiles.py

printf '%s\n' '--- profile registry writes ---'
rg -n -C 12 'profiles\.json|write_text|mkdir|_save|save' src/claude_task_master/core/profiles.py src/claude_task_master/cli_commands/profile.py

printf '%s\n' '--- relevant profile command flow ---'
rg -n -C 10 'profile_add|add_profile|ProfileStore|Profiles|\.add\(' \
  src/claude_task_master/cli_commands/profile.py src/claude_task_master/core/profiles.py

Repository: developerz-ai/claude-task-master

Length of output: 24804


🌐 Web query:

site:kubernetes.io/docs emptyDir fsGroup ownership permissions non-root writable emptyDir

💡 Result:

In Kubernetes, managing ownership and permissions for an emptyDir volume to ensure non-root writability is primarily handled through the Pod's securityContext [1][2]. Key mechanisms and behaviors: 1. fsGroup and Volume Ownership: By default, when you specify an fsGroup in the Pod's securityContext, Kubernetes recursively changes the ownership and permissions of the contents of the volume to match that fsGroup [1]. This ensures that the volume is readable and writable by containers running with that group ID [1]. 2. Limitations with emptyDir: While fsGroup works for many volume types, it is important to note that the fsGroupChangePolicy field, which allows you to control how ownership is changed (e.g., to improve performance), does not take effect on ephemeral volume types like emptyDir [1]. 3. Default Permissions: If you do not rely on fsGroup, an emptyDir volume is created by the Kubelet. The resulting permissions and ownership depend on the Kubelet's operation. If you need specific permissions, you can use the defaultMode field within the EmptyDirVolumeSource to set the octal mode bits for files created in the directory (default is 0644) [3]. However, this field does not affect directory permissions [3]. 4. Non-root Writable Strategy: To ensure a non-root container can write to an emptyDir: - Use the fsGroup field in the Pod's securityContext to set the volume's group ownership [1]. - Ensure the container process runs with a group ID that matches the fsGroup (or is a member of it) [1]. - If using User Namespaces, the UIDs and GIDs specified in the pod spec (like fsGroup or runAsUser) refer to the IDs inside the container, and Kubernetes ensures the volume is mapped correctly for those IDs [4][5]. For scenarios where you need granular control over volume permissions that fsGroup does not provide, you may need to use an init container to change the permissions (e.g., chmod/chown) of the volume path before the main application container starts, as the main container may lack the privileges to modify root-owned directories [1].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- profile registry implementation ---'
sed -n '1,220p' src/claude_task_master/core/profiles.py
printf '%s\n' '--- profile registry write paths ---'
rg -n -C 10 'profiles\.json|write_text|mkdir|save|_save' \
  src/claude_task_master/core/profiles.py \
  src/claude_task_master/cli_commands/profile.py

Repository: developerz-ai/claude-task-master

Length of output: 23762


Make the profile volume writable by claudetm.

emptyDir hides the image-owned directory. ProfileManager.save() must create profiles.json.tmp, but both containers run as non-root claudetm (UID 1000). Add fsGroup: 1000 to the pod securityContext, or prepare the volume in a root init container while keeping the application non-root.

🤖 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 `@docs/docker.md` around lines 997 - 1000, Update the pod securityContext for
the agent-profile emptyDir volume to grant writable access to claudetm (UID
1000), preferably by adding fsGroup: 1000; otherwise prepare the volume
ownership in a root init container while keeping application containers
non-root.

Source: Path instructions

Comment thread docs/docker.md Outdated
Comment on lines +171 to 176
parallel: bool | None = Field(
default=None,
description=(
"Whether to run a PR group's remaining tasks as one 'hive' session that fans "
"disjoint-write-set tasks out to subagents (off by default)"
"Whether a work session may split its one task across 'hive-worker' subagents "
"with disjoint write sets (on by default)"
),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 3 'model_config|ConfigDict|extra\s*=|parallel_tasks|parallel' \
  src/claude_task_master/api tests/api

Repository: developerz-ai/claude-task-master

Length of output: 7609


🌐 Web query:

According to the official Pydantic 2.12 documentation, what is the default behavior for unknown fields on BaseModel, and how can a model reject them with validation errors?

💡 Result:

According to the official Pydantic 2.12 documentation, the default behavior for unknown fields—data provided during model initialization that is not explicitly defined in the BaseModel—is to ignore them [1][2]. These extra fields are not stored in the model's instance dictionary [3]. To reject unknown fields and raise a validation error, you must configure the model to forbid them using the extra parameter within model_config [1][2]. This is achieved by setting extra='forbid' in a ConfigDict [2][4]: from pydantic import BaseModel, ConfigDict class Model(BaseModel): x: int model_config = ConfigDict(extra='forbid') When a model is configured this way, any attempt to initialize or validate it with unknown fields will raise a ValidationError [2][4]. Additionally, you can override this behavior for a single validation call without changing the model's permanent configuration by passing the extra argument directly to validation methods, such as model_validate() [1][2]: Model.model_validate({"x": 1, "y": 2}, extra="forbid")

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- models_task.py structure ---'
ast-grep outline src/claude_task_master/api/models_task.py
printf '%s\n' '--- model definitions and configuration ---'
rg -n -C 5 'class .*BaseModel|model_config|ConfigDict|extra|parallel_tasks|class .*Request' \
  src/claude_task_master/api/models_task.py src/claude_task_master/api
printf '%s\n' '--- route request parsing ---'
sed -n '1,145p' src/claude_task_master/api/routes_task.py
printf '%s\n' '--- relevant model sections ---'
sed -n '1,190p' src/claude_task_master/api/models_task.py
sed -n '240,325p' src/claude_task_master/api/models_task.py
printf '%s\n' '--- tests for unknown fields and legacy names ---'
rg -n -C 4 'unknown|extra|422|parallel_tasks|parallel' tests/api
printf '%s\n' '--- standalone Pydantic behavior probe ---'
python3 - <<'PY'
try:
    import pydantic
    from pydantic import BaseModel
    print("pydantic_version=", pydantic.__version__)

    class TaskInit(BaseModel):
        goal: str
        parallel: bool | None = None

    for payload in (
        {"goal": "x", "parallel_tasks": False},
        {"goal": "x", "parallel": False, "parallel_tasks": True},
    ):
        model = TaskInit.model_validate(payload)
        print("payload=", payload, "model_dump=", model.model_dump(),
              "fields_set=", sorted(model.model_fields_set))
except Exception as exc:
    print(type(exc).__name__, str(exc))
PY

Repository: developerz-ai/claude-task-master

Length of output: 50388


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- request model and route ---'
sed -n '1,45p' src/claude_task_master/api/models_task.py
sed -n '202,280p' src/claude_task_master/api/models_task.py
sed -n '80,130p' src/claude_task_master/api/routes_task.py

printf '%s\n' '--- default and propagation paths ---'
rg -n -C 5 'TaskInitRequest|TaskOptions\(|parallel\s*=' src/claude_task_master tests/api

printf '%s\n' '--- focused Pydantic probe ---'
python3 - <<'PY'
from pydantic import BaseModel

class TaskInitRequest(BaseModel):
    goal: str
    parallel: bool | None = None

payload = {"goal": "x", "parallel_tasks": False}
model = TaskInitRequest.model_validate(payload)
print("model_dump:", model.model_dump())
print("parallel:", model.parallel)
print("extra retained:", getattr(model, "parallel_tasks", "<absent>"))
print("default extra policy:", TaskInitRequest.model_config.get("extra"))
PY

Repository: developerz-ai/claude-task-master

Length of output: 29398


Reject unknown parallel_tasks request fields.

TaskInitRequest defaults parallel to True, so Pydantic discards "parallel_tasks": false and the route starts parallel execution. Set ConfigDict(extra="forbid") and add a 422 regression test.

🤖 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/claude_task_master/api/models_task.py` around lines 171 - 176, Update the
TaskInitRequest model to configure Pydantic with ConfigDict(extra="forbid"),
ensuring unknown fields such as parallel_tasks are rejected with a 422 response
instead of being discarded. Add a regression test for the request route that
submits parallel_tasks and verifies the 422 validation response.

Comment on lines +60 to +66
# Let a work session's agent split its ONE task across `hive-worker`
# subagents when the pieces have disjoint write sets. The task stays the
# unit of work — one session, one task checked off — so this only ever adds
# a fan-out brief to the work prompt. On by default: fanning out is the
# lead's judgement call and zero workers is a legitimate answer, so the flag
# grants permission rather than mandating parallelism.
parallel: bool = True

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find persisted-state loading and any existing schema migrations.
rg -n -C 4 --glob='*.py' \
  'parallel_tasks|model_validate|model_validate_json|TaskOptions\(' \
  src tests

# Find checked-in state fixtures or documentation that still use the legacy key.
rg -n -C 3 'parallel_tasks' .

Repository: developerz-ai/claude-task-master

Length of output: 50389


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- state model ---'
sed -n '1,140p' src/claude_task_master/core/state_models.py

printf '%s\n' '--- state loading methods ---'
rg -n -C 10 --glob='*.py' \
  'def (load_state|save_state|initialize)|state_path|json\.load|model_validate|TaskState\(' \
  src/claude_task_master/core

printf '%s\n' '--- exact legacy-key references ---'
rg -n --glob='*.py' 'parallel_tasks' src tests || true

Repository: developerz-ai/claude-task-master

Length of output: 50388


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- migration implementation ---'
sed -n '330,410p' src/claude_task_master/core/state_save_load.py

printf '%s\n' '--- state manager composition and exports ---'
sed -n '1,120p' src/claude_task_master/core/state.py
rg -n --glob='*.py' 'class StateManager|StateSaveLoad|TaskState\(\*\*|TaskOptions' \
  src/claude_task_master/core/state*.py tests/core/test_state*.py

printf '%s\n' '--- focused legacy-key search ---'
rg -n --glob='*.py' --glob='*.json' --glob='*.md' 'parallel_tasks' . || true

printf '%s\n' '--- read-only source invariant probe ---'
python3 - <<'PY'
from pathlib import Path
import re

models = Path("src/claude_task_master/core/state_models.py").read_text()
loader = Path("src/claude_task_master/core/state_save_load.py").read_text()

migration_empty = bool(re.search(
    r"_STATE_MIGRATIONS:\s*dict\[int,\s*Callable\[\[dict\[str,\s*Any\]\],\s*dict\[str,\s*Any\]\]\]\s*=\s*\{\}",
    models,
))
migration_before_validation = loader.find("data = self._migrate_state(data)") < loader.find(
    "return TaskState(**data)"
)
legacy_key_in_model = "parallel_tasks" in models
print({
    "current_migrations_empty": migration_empty,
    "migration_before_task_state_validation": migration_before_validation,
    "legacy_key_in_task_options_model": legacy_key_in_model,
})
PY

Repository: developerz-ai/claude-task-master

Length of output: 17181


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- changelog context ---'
sed -n '1,30p' CHANGELOG.md

printf '%s\n' '--- commit introducing the current field ---'
git log --all --oneline -S'parallel_tasks' -- src tests CHANGELOG.md | head -20

printf '%s\n' '--- recent state-model history ---'
git log --all --oneline -- src/claude_task_master/core/state_models.py | head -12

printf '%s\n' '--- current commit summary and parent diff summary ---'
git show --no-patch --oneline HEAD
git diff --stat HEAD^ HEAD -- src/claude_task_master/core/state_models.py tests/core/test_state.py CHANGELOG.md

Repository: developerz-ai/claude-task-master

Length of output: 8487


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- previous persisted model ---'
git show HEAD^:src/claude_task_master/core/state_models.py | sed -n '25,85p'

printf '%s\n' '--- previous serialization test ---'
git show HEAD^:tests/core/test_state.py | sed -n '70,110p'

printf '%s\n' '--- current model configuration ---'
rg -n -C 3 'model_config|extra\s*=|parallel\s*:|parallel_tasks' \
  src/claude_task_master/core/state_models.py \
  tests/core/test_state.py

printf '%s\n' '--- standalone Pydantic behavior probe ---'
python3 - <<'PY'
try:
    from pydantic import BaseModel
except ImportError as exc:
    print(f"pydantic unavailable: {exc}")
    raise SystemExit(0)

class Options(BaseModel):
    parallel: bool = True

legacy = Options.model_validate({"parallel_tasks": False})
print({
    "parsed_parallel": legacy.parallel,
    "serialized": legacy.model_dump(),
    "legacy_key_retained": "parallel_tasks" in legacy.model_dump(),
})
PY

Repository: developerz-ai/claude-task-master

Length of output: 5088


Migrate legacy parallel_tasks when loading state.

Pre-change state.json files persist options.parallel_tasks. The current migration table does not map this key, so Pydantic ignores it and options.parallel defaults to True. Add a migration or validation-only alias, keep model_dump() canonical as parallel, and add a StateManager.load_state() regression test with parallel_tasks: false.

📍 Affects 2 files
  • src/claude_task_master/core/state_models.py#L60-L66 (this comment)
  • tests/core/test_state.py#L77-L100
🤖 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/claude_task_master/core/state_models.py` around lines 60 - 66, Update
src/claude_task_master/core/state_models.py:60-66 to accept legacy
options.parallel_tasks during state loading, while keeping model_dump() output
canonical under options.parallel and preserving the default for states without
either key. Add a regression test in tests/core/test_state.py:77-100 through
StateManager.load_state() that loads parallel_tasks: false and verifies the
resulting options.parallel is false.

enable_release: bool | None = None,
enable_verification: bool | None = None,
parallel_tasks: bool | None = None,
parallel: bool | None = None,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Find residual references to the old parallel_tasks option name.
rg -n -C2 'parallel_tasks|--parallel-tasks|no-parallel-tasks'

Repository: developerz-ai/claude-task-master

Length of output: 170


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- tracked files related to schemas, state, migrations, and documentation ---'
git ls-files | rg -i '(schema|state|migration|docs?|readme|tool|config)' | head -200
printf '%s\n' '--- all tracked references to parallel and related option forms ---'
rg -n -i -C2 'parallel[_-]?tasks|parallel' --glob '!*.lock' --glob '!*.pyc' . | head -300
printf '%s\n' '--- target handler structure ---'
ast-grep outline src/claude_task_master/mcp/tool_handlers_control.py
printf '%s\n' '--- target handler context ---'
sed -n '160,270p' src/claude_task_master/mcp/tool_handlers_control.py

Repository: developerz-ai/claude-task-master

Length of output: 46084


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- exact old-name references ---'
git grep -n -i -E 'parallel_tasks|--parallel-tasks|no-parallel-tasks' -- . ':!examples/orchestrator-workers.ipynb' || true
printf '%s\n' '--- live parallel declarations and call sites ---'
git grep -n -E '(^|[^[:alnum:]_])parallel([^[:alnum:]_]|$)' -- 'src/**' 'tests/**' 'docs/**' 'README.md' 'CLAUDE.md' | head -400
printf '%s\n' '--- state and model definitions ---'
rg -n -C3 'class TaskOptions|TaskOptions\(|options|model_dump|model_validate|extra=' src/claude_task_master/core src/claude_task_master/mcp src/claude_task_master/api
printf '%s\n' '--- relevant MCP model and tool registration ---'
sed -n '1,260p' src/claude_task_master/mcp/tool_models.py
rg -n -C5 'update_config|initialize_task|parallel' src/claude_task_master/mcp src/claude_task_master/api src/claude_task_master/cli_commands

Repository: developerz-ai/claude-task-master

Length of output: 50389


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- state model ---'
sed -n '1,130p' src/claude_task_master/core/state_models.py
printf '%s\n' '--- state load/save paths ---'
rg -n -C4 'json\.loads|model_validate|State\(|TaskState|load_state|save_state' \
  src/claude_task_master/core/state*.py src/claude_task_master/core/state.py
printf '%s\n' '--- API option models ---'
sed -n '140,330p' src/claude_task_master/api/models_task.py
printf '%s\n' '--- MCP schemas and registration ---'
sed -n '100,225p' src/claude_task_master/mcp/server_specs.py
sed -n '270,340p' src/claude_task_master/mcp/tool_handlers_task.py
printf '%s\n' '--- focused tests for old-name behavior or state compatibility ---'
git grep -n -i -E 'legacy|migration|backward|compat|extra|parallel_tasks|parallel-tasks' -- \
  'tests/**' 'src/claude_task_master/core/**' 'src/claude_task_master/mcp/**' \
  'src/claude_task_master/api/**' | head -250

Repository: developerz-ai/claude-task-master

Length of output: 50389


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- migration and load implementation ---'
sed -n '300,430p' src/claude_task_master/core/state_save_load.py
printf '%s\n' '--- state option update implementation ---'
sed -n '430,490p' src/claude_task_master/core/state.py
printf '%s\n' '--- focused migration tests ---'
rg -n -C5 '_migrate_state|schema_version|unknown|extra|legacy|migration' \
  tests/core tests/cli tests/mcp | head -300
printf '%s\n' '--- compact exact-name inventory by tracked file ---'
git grep -l -i -E 'parallel_tasks|--parallel-tasks|no-parallel-tasks' -- . \
  ':!examples/orchestrator-workers.ipynb' | sort

Repository: developerz-ai/claude-task-master

Length of output: 31296


🏁 Script executed:

#!/bin/bash
set -e
python3 - <<'PY'
from pydantic import BaseModel

class TaskOptions(BaseModel):
    parallel: bool = True

legacy = TaskOptions.model_validate({"parallel_tasks": False})
print("serialized:", legacy.model_dump())
print("parallel:", legacy.parallel)
print("old_name_retained:", "parallel_tasks" in legacy.model_dump())
PY
printf '%s\n' '--- old-name references outside historical changelog entries ---'
git grep -n -i -E 'parallel_tasks|--parallel-tasks|no-parallel-tasks' -- . \
  ':!CHANGELOG.md' ':!examples/orchestrator-workers.ipynb' || true
printf '%s\n' '--- migration registry and schema version ---'
git grep -n -C2 -E 'CURRENT_SCHEMA_VERSION|_STATE_MIGRATIONS|parallel_tasks' -- \
  src/claude_task_master/core/state_models.py \
  src/claude_task_master/core/state_save_load.py

Repository: developerz-ai/claude-task-master

Length of output: 291


Add a migration for legacy parallel_tasks state. parallel_tasks is absent from live code and appears only in historical CHANGELOG.md, but existing state.json files can still contain it. With no schema migration, the field is discarded and parallel defaults to True, which can change behavior when resuming an older run. Handle the legacy field explicitly and add a regression test.

🤖 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/claude_task_master/mcp/tool_handlers_control.py` at line 196, Update the
state-loading/schema migration flow associated with the control tool’s parallel
setting to recognize legacy parallel_tasks, map its value to parallel before
unknown fields are discarded, and preserve the existing default for states
without the legacy field. Add a regression test covering an older state.json
containing parallel_tasks and verify resumed behavior uses the migrated value.

Comment on lines +705 to +953
# =============================================================================
# Per-worker coloring
# =============================================================================


class _TtyStdout:
"""stdout stand-in that claims to be a terminal, to force color on."""

def isatty(self) -> bool:
return True


class _PipedStdout:
"""stdout stand-in for a pipe/redirect — the log-file case."""

def isatty(self) -> bool:
return False


def _enable_color(monkeypatch) -> None:
"""Make color_enabled() report True for the rest of this test.

Must be called from inside the test *body*, never from a fixture: pytest's
capture manager re-installs ``sys.stdout`` between the setup and call
phases, so a stdout patched during setup is silently replaced and the test
would quietly measure the uncolored path instead.
"""
monkeypatch.delenv("NO_COLOR", raising=False)
monkeypatch.setattr("sys.stdout", _TtyStdout())


def _spawn(block_id: str, subagent_type: str = "hive-worker") -> MagicMock:
"""Build the lead's own Agent tool call that spawns a worker."""
msg = _make_assistant_message(
[_make_tool_use_block("Agent", {"subagent_type": subagent_type}, block_id=block_id)]
)
msg.parent_tool_use_id = None
return msg


class TestSubagentColoring:
"""Concurrent workers are separable from each other, not just from the lead.

Color is keyed to the worker's ``tool_use_id`` — stable for its whole life —
because the several ``hive-worker``s a lead runs share one agent name, so
coloring by name would repaint them all identically.
"""

def test_two_concurrent_workers_are_colored_differently(self, monkeypatch):
"""Two live hive-workers must not render as one undifferentiated stream."""
_enable_color(monkeypatch)
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(_spawn("toolu_b"), "")
proc.process_message(
_make_subagent_message([_make_text_block("A works")], "toolu_a"), ""
)
first = mock_console.claude_text.call_args[0][0]
proc.process_message(
_make_subagent_message([_make_text_block("B works")], "toolu_b"), ""
)
second = mock_console.claude_text.call_args[0][0]

assert ANSI_RE.findall(first) != ANSI_RE.findall(second)

def test_two_concurrent_workers_are_labelled_differently(self):
"""Same agent name, different instance → a visible discriminator."""
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(_spawn("toolu_b"), "")
proc.process_message(_make_subagent_message([_make_text_block("a")], "toolu_a"), "")
first = mock_console.claude_text.call_args[0][0]
proc.process_message(_make_subagent_message([_make_text_block("b")], "toolu_b"), "")
second = mock_console.claude_text.call_args[0][0]

assert "hive-worker#1" in first
assert "hive-worker#2" in second

def test_ordinals_follow_spawn_order_not_speaking_order(self, monkeypatch):
"""#n counts workers as the reader watched them spawn."""
_enable_color(monkeypatch)
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_first"), "")
proc.process_message(_spawn("toolu_second"), "")
# The second-spawned worker happens to speak first.
proc.process_message(
_make_subagent_message([_make_text_block("x")], "toolu_second"), ""
)

assert "hive-worker#2" in mock_console.claude_text.call_args[0][0]

def test_same_worker_keeps_its_color_across_messages(self, monkeypatch):
"""A color that changes mid-run is worse than no color at all."""
_enable_color(monkeypatch)
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(_spawn("toolu_b"), "")
proc.process_message(_make_subagent_message([_make_text_block("one")], "toolu_a"), "")
first = mock_console.claude_text.call_args[0][0]
proc.process_message(_make_subagent_message([_make_text_block("two")], "toolu_b"), "")
proc.process_message(_make_subagent_message([_make_text_block("three")], "toolu_a"), "")
third = mock_console.claude_text.call_args[0][0]

assert ANSI_RE.findall(first) == ANSI_RE.findall(third)
assert "hive-worker#1" in third

def test_tool_lines_share_the_workers_color(self, monkeypatch):
"""Tool activity is colored like the worker's text, not separately."""
_enable_color(monkeypatch)
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(_make_subagent_message([_make_text_block("hi")], "toolu_a"), "")
text_line = mock_console.claude_text.call_args[0][0]
proc.process_message(
_make_subagent_message(
[_make_tool_use_block("Read", {"file_path": "/x"})], "toolu_a"
),
"",
)
tool_line = mock_console.tool.call_args[0][0]

assert ANSI_RE.findall(text_line) == ANSI_RE.findall(tool_line)

def test_lead_output_stays_plain(self, monkeypatch):
"""The lead's own line is byte-identical to before: no marker, no color."""
_enable_color(monkeypatch)
proc = MessageProcessor()
msg = _make_assistant_message([_make_text_block("mine")])
msg.parent_tool_use_id = None
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(msg, "")

assert mock_console.claude_text.call_args[0][0] == "mine"

def test_subagent_text_still_not_accumulated_when_colored(self, monkeypatch):
"""Coloring is cosmetic: the isolation of result_text is untouched."""
_enable_color(monkeypatch)
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console"):
proc.process_message(_spawn("toolu_a"), "")
out = proc.process_message(
_make_subagent_message([_make_text_block("TASK COMPLETE")], "toolu_a"),
"lead ",
)

assert out == "lead "
assert "TASK COMPLETE" not in out

def test_logger_receives_no_escape_sequences(self, monkeypatch):
"""Log files must stay free of ANSI — the marker is console-only."""
_enable_color(monkeypatch)
mock_logger = MagicMock()
proc = MessageProcessor(logger=mock_logger)
with patch("claude_task_master.core.agent_message.console"):
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(
_make_subagent_message(
[_make_tool_use_block("Read", {"file_path": "/foo.py"})], "toolu_a"
),
"",
)
proc.process_message(
_make_subagent_message([_make_tool_result_block("tu_1")], "toolu_a"), ""
)

for call in (
mock_logger.log_tool_use.call_args_list + mock_logger.log_tool_result.call_args_list
):
assert ANSI_RE.search(repr(call)) is None

def test_logger_calls_are_unchanged(self, monkeypatch):
"""The exact log_tool_use/log_tool_result payloads are as before."""
_enable_color(monkeypatch)
mock_logger = MagicMock()
proc = MessageProcessor(logger=mock_logger)
with patch("claude_task_master.core.agent_message.console"):
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(
_make_subagent_message(
[_make_tool_use_block("Read", {"file_path": "/foo.py"})], "toolu_a"
),
"",
)

mock_logger.log_tool_use.assert_called_with("Read", {"file_path": "/foo.py"})

def test_reset_clears_color_assignments(self, monkeypatch):
"""Ordinals restart per query instead of growing across a long run."""
_enable_color(monkeypatch)
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(_spawn("toolu_b"), "")
proc.process_message(_make_subagent_message([_make_text_block("b")], "toolu_b"), "")
assert "hive-worker#2" in mock_console.claude_text.call_args[0][0]

proc.reset_result_state()
proc.process_message(_spawn("toolu_c"), "")
proc.process_message(_make_subagent_message([_make_text_block("c")], "toolu_c"), "")
assert "hive-worker#1" in mock_console.claude_text.call_args[0][0]


class TestSubagentColoringDisabled:
"""With color disabled the marker is plain text — logs stay readable."""

def test_no_color_env_suppresses_escapes(self, monkeypatch):
"""NO_COLOR wins even on a terminal."""
monkeypatch.setenv("NO_COLOR", "1")
monkeypatch.setattr("sys.stdout", _TtyStdout())
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(_make_subagent_message([_make_text_block("hi")], "toolu_a"), "")

printed = mock_console.claude_text.call_args[0][0]
assert ANSI_RE.search(printed) is None
assert printed == "↳ [hive-worker#1] hi"

def test_redirected_output_has_no_escapes(self, monkeypatch):
"""A non-TTY (pipe, redirect into a log file) gets plain text."""
monkeypatch.delenv("NO_COLOR", raising=False)
monkeypatch.setattr("sys.stdout", _PipedStdout())
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(_make_subagent_message([_make_text_block("hi")], "toolu_a"), "")

assert ANSI_RE.search(mock_console.claude_text.call_args[0][0]) is None

def test_workers_still_labelled_distinctly_without_color(self, monkeypatch):
"""Without color the discriminator alone keeps workers apart."""
monkeypatch.setenv("NO_COLOR", "1")
proc = MessageProcessor()
with patch("claude_task_master.core.agent_message.console") as mock_console:
proc.process_message(_spawn("toolu_a"), "")
proc.process_message(_spawn("toolu_b"), "")
proc.process_message(_make_subagent_message([_make_text_block("a")], "toolu_a"), "")
first = mock_console.claude_text.call_args[0][0]
proc.process_message(_make_subagent_message([_make_text_block("b")], "toolu_b"), "")
second = mock_console.claude_text.call_args[0][0]

assert first != second
assert "hive-worker#1" in first
assert "hive-worker#2" in second

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Split the oversized test modules.

The repository rule limits every Python file to 500 lines. These modules exceed that limit after this change. Move focused scenarios into smaller test modules, or document an approved exception for the conflicting one-test-file-per-module rule.

  • tests/core/test_agent_message.py#L705-L953: move the worker-coloring scenarios and helpers into a focused module.
  • tests/core/test_console.py#L1195-L1406: move color-gating and SubagentPalette scenarios into focused modules.
  • tests/mcp/test_tools_control.py#L401-L432: move configuration-option scenarios into a focused module.
📍 Affects 3 files
  • tests/core/test_agent_message.py#L705-L953 (this comment)
  • tests/core/test_console.py#L1195-L1406
  • tests/mcp/test_tools_control.py#L401-L432
🤖 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/core/test_agent_message.py` around lines 705 - 953, Split the oversized
test files while preserving all existing scenarios and assertions: move the
worker-coloring helpers and TestSubagentColoring/TestSubagentColoringDisabled
from tests/core/test_agent_message.py:705-953 into a focused module; move the
color-gating and SubagentPalette scenarios from
tests/core/test_console.py:1195-1406 into focused module(s); and move the
configuration-option scenarios from tests/mcp/test_tools_control.py:401-432 into
a focused module. Ensure each resulting Python file stays within the 500-line
repository limit, or document an approved exception if the
one-test-file-per-module rule prevents the split.

Source: Coding guidelines

Comment thread tests/core/test_agent_message.py Outdated
Seven of thirteen findings fixed, six skipped with reasons (posted on the PR).

Real bug found: pydantic silently dropped the removed `parallel_tasks` key,
and `parallel` now defaults to True — so a REST client asking for no
parallelism would silently get it. `_RemovedFieldGuard` now 422s with the
replacement name instead. Targeted rather than `extra="forbid"`, because the
README documents a `project_dir` key on /task/init that no model declares.

Also: a vacuous ANSI-leak test (`repr()` renders ESC as the four characters
`\\x1b`, so the regex could never match) now walks raw logger args; the
missing-profile troubleshooting said OAuth fallback where `resolve_active`
actually raises `ProfileNotFoundError`; the Swarm example populated
`CLAUDETM_PASSWORD` from a secret it never read, leaving auth middleware
uninstalled on a 0.0.0.0 bind; an undeclared external volume; a shared
credential volume across all five compose examples; and the CHANGELOG's own
claim that no `reviewDecision` handling exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sebyx07

sebyx07 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks — all 13 findings verified against the tree. 7 fixed, 6 skipped, pushed in 09bfdbb.

One was a real bug worth calling out: pydantic silently drops the removed parallel_tasks key, and parallel now defaults to True — so a REST client explicitly asking for no parallelism would silently get parallelism. Now a 422 naming the replacement.

Fixed

  1. CHANGELOG.md:20 — the entry claimed no reviewDecision handling exists, while this PR adds it. Corrected: approval never gates merging, CHANGES_REQUESTED does.
  2. docs/docker.md:760 — the named volume was mounted with no top-level declaration; docker compose config would have errored.
  3. docs/docker.md:892 — the Swarm example set CLAUDETM_PASSWORD from a secret nothing reads. Nothing in src/ reads /run/secrets or any *_FILE var (auth/password.py reads only CLAUDETM_PASSWORD/_HASH), and an empty password means the auth middleware is never installed on a server bound to 0.0.0.0. Replaced with an entrypoint shim.
  4. docs/docker.md:1130 — troubleshooting claimed an OAuth fallback; ProfileManager.resolve_active actually raises ProfileNotFoundError. Split into the two real symptoms; same fix applied to the identical sentence in the examples README.
  5. examples/docker-compose/* — all five resolved to one host volume; now ${CLAUDETM_PROFILES_VOLUME:-…}, with development.yml defaulting separately.
  6. api/models_task.py:176 — the guard above.
  7. tests/core/test_agent_message.py:878 — the test was vacuous: repr() renders ESC as the four characters \x1b, so the regex could never match a real 0x1B byte. Now walks raw logger args, plus a call-count assert so an empty list can't make it vacuous again.

Skipped, with reasons

  • docs/docker.md:1000 (fsGroup: 1000) — kubelet creates an emptyDir at mode 0777, so the non-root user already writes there. Adding a securityContext would imply a constraint that does not exist.
  • README.md:727 (~/.config/gh mount) — real concern, but not introduced here: this PR only moved the container-side path from /root to /home/claudetm. It is a different credential class from the Claude OAuth token Stop bind-mounting host ~/.claude into the agent container; auth via Meridian #100 is about, and replacing it needs a GH_TOKEN design applied across 8 files. Filed separately rather than smuggled into a 81-file PR.
  • README.md:728 (mount repo at /app/project) — would break the example. That docker run is the repo-clone workflow: POST /repo/clone writes under DEFAULT_WORKSPACE_BASE and _validate_within_workspace rejects paths outside it, so /home/claudetm/workspace is correct and /app/project would put clones outside both the confined base and the persisted volume.
  • core/state_models.py:66 and mcp/tool_handlers_control.py:196 (migrate legacy parallel_tasks) — the migration would be actively harmful. In 0.1.83 parallel_tasks defaulted to False, so essentially every state.json from that release carries false because the user never opted in; mapping it would silence the new on-by-default for every resumed run. For the only value reflecting a real choice (true) it is a no-op against the new default. The two flags also name different mechanisms — batching a group's tasks vs a lead splitting its own task — and the removal is documented as BREAKING and unaliased.
  • tests/core/test_agent_message.py:953 (split oversized test modules) — the 500-LOC rule is not applied to tests anywhere here; 40+ modules already exceed it (test_workflow_stages.py is 3068). An unrequested refactor of three of them inside a review PR would be inconsistent.

Gate after the fixes: 5961 passed, 3 skipped (was 5957 — 4 new tests, none weakened) · ruff ✓ · mypy . ✓ 421 files · docker compose config ✓ on all six compose files plus the extracted Swarm block.

@sebyx07
sebyx07 merged commit 60bdd2f into main Aug 8, 2026
5 checks passed
@sebyx07
sebyx07 deleted the fix/all-open-issues branch August 8, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment