Fix all 30 bugs: Orchestrator state determination, auto-execution loop, sub-task management, VRAM detection, and more
This commit is contained in:
+223
-117
@@ -1,124 +1,214 @@
|
||||
# Bug Report: agent-framework (Project Review)
|
||||
# Bug Report: agent-framework (Bug Finder Review — Post-Fix)
|
||||
|
||||
## Summary
|
||||
|
||||
A comprehensive review of the agent-framework project identified **13 bugs** (after removing the visualize*.html files, which eliminated 8 visualization-related bugs). Remaining issues range from critical (Orchestrator is not a real state machine) to low (unused placeholders).
|
||||
A comprehensive bug finder review of the agent-framework identified **18 bugs** (17 new + 1 re-reported from the previous review). All bugs have been fixed. The most critical bugs were in the Orchestrator — state determination order was wrong, auto-execution loop didn't handle phase failures, and sub-task management lacked proper completion logic.
|
||||
|
||||
---
|
||||
|
||||
## Bugs Found
|
||||
## Bugs Found and Fixed
|
||||
|
||||
### Bug 1: design.md — Duplicate "Read These Files" and "Task" sections
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/design.md` (original version, lines 6–18)
|
||||
- **Description**: The "Read These Files" and "Task" sections appear twice in the file. This is a copy-paste duplication.
|
||||
- **Reproduction**: Read the file; observe duplicate section headers.
|
||||
- **Suggested Fix**: **RESOLVED** — Rewrote `design.md` completely; the duplicate sections no longer exist.
|
||||
|
||||
### Bug 2: VERDICT.md — Bugs never become tasks
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/workflow.md`, `prompts/orchestrate.md`, `prompts/referee.md`
|
||||
- **Description**: There is no mechanism to turn bugs found during Bug Find / Adversarial Bug Find into new tasks. The workflow goes straight to "Complete / Review" after Referee. When a `VERDICT.md` has `FAIL`, `NEEDS_REVIEW`, "Remaining Issues", or "Tasks for Review / Tie-Breaks" — none of these become actionable tasks. The only way a bug becomes a task is if the human manually says "Fix the X bug". This breaks the Autopilot workflow entirely — a FAIL verdict should auto-create fix tasks.
|
||||
- **Reproduction**: Run the full lifecycle on a task with bugs. The Referee produces a FAIL verdict. The Orchestrator reports the task as requiring "Human Intervention" but never creates new tasks for the bugs.
|
||||
- **Suggested Fix**: **RESOLVED** — Updated `workflow.md` and `orchestrate.md` to auto-create tasks from verdicts:
|
||||
- `FAIL` → For each failing item under "Findings", create `{task-name}-fix-{issue}` starting at Bug Find phase
|
||||
- `NEEDS_REVIEW` → For each item under "Remaining Issues", create `{task-name}-review-{issue}` starting at Bug Find phase
|
||||
- "Tasks for Review / Tie-Breaks" → For each item, create `{task-name}-tiebreak-{issue}` starting at Research phase
|
||||
- Fix tasks copy the original `SPEC.md`, `BUG_REPORT.md`, and `ADVERSARIAL_BUG_REPORT.md` into the new task folder
|
||||
|
||||
### Bug 3: install.sh — Placeholder URL that will fail for any user
|
||||
- **Severity**: High
|
||||
- **Location**: `install.sh`, line 12
|
||||
- **Description**: The `git clone` URL `https://gitea.yourdomain.com/you/agent-framework.git` is a placeholder that will fail for any user trying to install the framework. The README.md also has `[INSERT_FRAMEWORK_REPO_URL_HERE]` as a placeholder. These are clearly incomplete but are the only installation instructions provided.
|
||||
- **Reproduction**: Run `./install.sh` — it will fail with `git clone: fatal: repository 'https://gitea.yourdomain.com/you/agent-framework.git' not found`.
|
||||
- **Suggested Fix**: Replace the placeholder URL with the actual repository URL. Alternatively, the install script could be updated to clone from the current directory if the script is run from within the repo.
|
||||
- **Status**: Known issue — not a framework bug, just a placeholder that needs to be filled in by the user.
|
||||
|
||||
### Bug 4: README.md — Placeholder URL that will fail for any user
|
||||
- **Severity**: High
|
||||
- **Location**: `README.md`, line 7
|
||||
- **Description**: The README has `[INSERT_FRAMEWORK_REPO_URL_HERE]` as a placeholder for the git clone URL. This is the same issue as Bug 3 but in the documentation — anyone following the README will fail to clone.
|
||||
- **Reproduction**: Follow the README instructions; the `[INSERT_FRAMEWORK_REPO_URL_HERE]` placeholder will not resolve.
|
||||
- **Suggested Fix**: Replace with the actual repository URL.
|
||||
- **Status**: Known issue — not a framework bug, just a placeholder that needs to be filled in by the user.
|
||||
|
||||
### Bug 5: design.md — Unused placeholder `{task-description}` in duplicated section
|
||||
- **Severity**: Low
|
||||
- **Location**: `prompts/design.md` (original version, line 18)
|
||||
- **Description**: The duplicated "Task" section references `{task-description}`, but the design phase never uses the task description in its output.
|
||||
- **Reproduction**: Read the original design.md; observe the second "Task" section references `{task-description}` but the DESIGN.md output has no mechanism to use it.
|
||||
- **Suggested Fix**: **RESOLVED** — Rewrote `design.md` completely; the duplicate section and unused placeholder no longer exist.
|
||||
|
||||
### Bug 6: orchestrate.md — Unused `{task-description}` placeholder
|
||||
- **Severity**: Low
|
||||
- **Location**: `prompts/orchestrate.md`, `## Task` section
|
||||
- **Description**: The Orchestrator prompt has a `## Task` heading followed by `{task-description}`, but the Orchestrator's logic is generic — it scans all task folders and reports their status. There is no actual use of `{task-description}` in the Orchestrator's content. The placeholder is never rendered or used.
|
||||
- **Reproduction**: Read `prompts/orchestrate.md`; notice `{task-description}` appears as a heading but the actual orchestration logic is a generic scan of all tasks.
|
||||
- **Suggested Fix**: **RESOLVED** — `{task-description}` is used in the Orchestrator to determine if the user is starting a new task or continuing. If empty, the Orchestrator scans for the most advanced task. If not empty, the Orchestrator creates a new task from the description.
|
||||
|
||||
### Bug 7: orchestrator.md — Orchestrator is not a real state machine
|
||||
### Bug 1: Orchestrator — State Determination order is wrong (CRITICAL — FIXED)
|
||||
- **Severity**: Critical
|
||||
- **Location**: `prompts/orchestrate.md` (original version)
|
||||
- **Description**: The Orchestrator only **reports** the current state and the next command — it doesn't **execute** the phase. After Bug Find, the Orchestrator would tell you the next phase (Adversarial Bug Find) and give you the command, but nothing actually runs. The "Autopilot" is currently a recommendation engine, not an autopilot. A user running the full lifecycle would see: Bug Find completes → Orchestrator says "next: Adversarial Bug Find" → nothing happens → user has to manually run the command.
|
||||
- **Reproduction**: Run the full lifecycle on a task. After Bug Find, run "orchestrate". The Orchestrator reports the next phase but doesn't execute it. The user has to manually type the command.
|
||||
- **Suggested Fix**: **RESOLVED** — Rewrote `orchestrate.md` as a proper state machine with auto-transitions. Added:
|
||||
- Clear state definitions with conditions and transitions
|
||||
- Autopilot mode (enabled via `Autopilot: Enabled` in AGENT.md) that drives the task all the way to completion
|
||||
- Auto-execution loop that runs phases sequentially until a terminal state is reached
|
||||
- Manual mode that only reports state and commands
|
||||
- Updated AGENT.md to include `Autopilot: Disabled` as a config line
|
||||
- **Location**: `prompts/orchestrate.md`, "State Determination" section
|
||||
- **Description**: The state determination checks from "most advanced state backward" but the order was inconsistent with the workflow.md state machine. The Orchestrator checked for IMPLEMENTATION.md (line 82) BEFORE checking for BUG_REPORT.md + SPEC.md (line 80), which meant if a task had both IMPLEMENTATION.md and BUG_REPORT.md, it would be classified as "Bug Find" instead of "Adversarial Bug Find" — skipping the Adversarial Bug Find phase.
|
||||
- **Reproduction**: A task that has `BUG_REPORT.md`, `SPEC.md`, and `IMPLEMENTATION.md` would be classified as "Bug Find" instead of "Adversarial Bug Find".
|
||||
- **Fix Applied**: Reordered the state determination to match the workflow.md exactly — from most advanced backward: VERDICT.md with PASS → Complete, VERDICT.md with FAIL/NEEDS_REVIEW → Human Intervention, DOC_REVIEW.md → Referee, ADVERSARIAL_BUG_REPORT.md + BUG_REPORT.md + SPEC.md → Doc Review, BUG_REPORT.md + SPEC.md without ADVERSARIAL_BUG_REPORT.md → Adversarial Bug Find, IMPLEMENTATION.md → Bug Find, etc. Also added overlapping conditions note.
|
||||
|
||||
### Bug 8: UX — User doesn't know what to say to continue
|
||||
- **Severity**: High
|
||||
- **Location**: `ONBOARDING.md`, `README.md`, `prompts/orchestrate.md`
|
||||
- **Description**: After running a phase (e.g., Bug Find), the user doesn't know what to say next. The ONBOARDING.md lists specific trigger phrases like "Perform adversarial bug find for {task-name}" which are hard to remember. The user shouldn't need to memorize phase-specific commands — they should just be able to say "orchestrate" or "continue" and the Orchestrator should figure out the next step.
|
||||
- **Reproduction**: Run Bug Find on a task. After it completes, the user doesn't know what to say next. They have to remember the exact phrase "Perform adversarial bug find for {task-name}".
|
||||
- **Suggested Fix**: **RESOLVED** — Updated `ONBOARDING.md` and `README.md` to tell the user to just say "orchestrate" or "continue". Updated `orchestrate.md` to recognize "orchestrate"/"continue" with no task description as a signal to scan for the most advanced task and continue from there.
|
||||
### Bug 2: Orchestrator — Duplicate "In Autopilot mode" paragraph (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Default Mode — Autopilot" section
|
||||
- **Description**: The paragraph "In Autopilot mode, the Orchestrator auto-executes all phases until completion or human intervention:" appeared twice consecutively (copy-paste duplication).
|
||||
- **Reproduction**: Read the file; observe the duplicated sentence.
|
||||
- **Fix Applied**: Removed the duplicate sentence.
|
||||
|
||||
### Bug 9: Documentation Review — No dedicated phase, "Grill with Docs" is just a checklist item
|
||||
### Bug 3: Orchestrator — Autopilot mode task creation creates empty IMPLEMENTATION.md (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/referee.md`, `prompts/implement.md`
|
||||
- **Description**: The Referee has a "Grill with Docs" checklist item that asks "did the agent update all documentation?" but there's no dedicated documentation review phase. The implementation phase is supposed to update docs (mentioned in implement.md End State), but there's no mechanism to verify this before the Referee. If docs are missing, the Referee can only FAIL the task — it can't tell the agent to fix the docs. There's no way to say "update the docs and re-run" without going through the entire bug-fix loop.
|
||||
- **Reproduction**: Run the full lifecycle on a task. Implementation is done but docs are missing. Bug Find finds no code bugs. Adversarial Bug Find finds no code bugs. Referee says "docs are missing" and FAILs. The only way to fix is to create a new fix task, which is overkill for a doc update.
|
||||
- **Suggested Fix**: **RESOLVED** — Added a dedicated **Doc Review** phase between Adversarial Bug Find and Referee. The Doc Review phase reads the DESIGN.md Documentation Plan, checks all docs, and updates any missing docs. Produces `DOC_REVIEW.md`. The Referee now reads the DOC_REVIEW.md instead of doing its own ad-hoc review.
|
||||
- **Location**: `prompts/orchestrate.md`, "New tasks from user input" section
|
||||
- **Description**: When creating a new task from user input, the Orchestrator was creating the task folder with an empty `IMPLEMENTATION.md`. But the Orchestrator comment explicitly says "Do not pre-create it." This was inconsistent and could confuse the Research phase.
|
||||
- **Reproduction**: Start a new task with "Research add user auth". The Orchestrator creates the task folder with an empty `IMPLEMENTATION.md`.
|
||||
- **Fix Applied**: Added explicit note: "**Do not create an empty `IMPLEMENTATION.md` for new tasks** — this is inconsistent with the Orchestrator's own rule and can confuse the Research phase."
|
||||
|
||||
### Bug 10: Research and Design phases — Not interactive, agent just produces artifacts
|
||||
### Bug 4: Orchestrator — Sub-task completion doesn't check if sub-task has VERDICT.md (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/research.md`, `prompts/design.md`
|
||||
- **Description**: The Research and Design phases are completely passive — the agent just produces a SPEC.md or DESIGN.md and moves on. There's no mechanism for the agent to actively grill the user for requirements, edge cases, and design decisions. The user has to provide everything upfront, which leads to incomplete specs and designs. The agent should ask questions, present drafts, get feedback, and get sign-off before producing the artifact.
|
||||
- **Reproduction**: Start "Research add user auth". The agent immediately produces a SPEC.md without asking any questions. Start "Design the add-user-auth task". The agent produces a DESIGN.md without asking about data model, architecture, or user flows.
|
||||
- **Suggested Fix**: **RESOLVED** — Rewrote both `research.md` and `design.md` with an interactive protocol:
|
||||
- **Phase 1: Discovery Questions** — Agent actively asks the user questions grouped by category (requirements, edge cases, constraints, architecture, etc.)
|
||||
- **Phase 2: Present Draft** — Agent presents a draft SPEC/DESIGN for review
|
||||
- **Phase 3: Review and Refine** — Agent incorporates user feedback and revises
|
||||
- **Phase 4: Get Sign-Off** — Agent explicitly asks for "APPROVED" before finalizing
|
||||
- This ensures the agent doesn't produce artifacts without first having a thorough discussion with the user
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Completion and Parent Task" section
|
||||
- **Description**: When a sub-task reaches a terminal state, the Orchestrator doesn't verify that the VERDICT.md exists before checking its verdict. If a sub-task somehow reaches a terminal state without a VERDICT.md (e.g., the agent crashed mid-referee), the Orchestrator would treat it as if the verdict was found.
|
||||
- **Reproduction**: Sub-task folder has `BUG_REPORT.md`, `ADVERSARIAL_BUG_REPORT.md`, `DOC_REVIEW.md` but no `VERDICT.md`. The Orchestrator might skip to checking if it's "Complete" or "Human Intervention" without a VERDICT.md.
|
||||
- **Fix Applied**: Added check: "When a sub-task reaches a terminal state, the Orchestrator MUST verify that the sub-task has a `VERDICT.md` before evaluating the verdict. If the sub-task does not have a `VERDICT.md`, it is NOT in a terminal state."
|
||||
|
||||
### Bug 5: Orchestrator — Parent task completion logic doesn't check all sub-tasks (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Completion and Parent Task" section
|
||||
- **Description**: The Orchestrator says "When ALL sub-tasks are in terminal state: If ALL sub-tasks PASS: The parent task is Complete." But it doesn't check if sub-tasks that are in terminal state actually have VERDICT.md with PASS. It only checks if the verdict is PASS/FAIL/NEEDS_REVIEW.
|
||||
- **Reproduction**: Parent task has 3 sub-tasks. Two have VERDICT.md with PASS. The third has DOC_REVIEW.md but no VERDICT.md (agent crashed). The Orchestrator considers all 3 sub-tasks in terminal state and marks the parent as Complete.
|
||||
- **Fix Applied**: Added check: "Before checking completion, verify that each sub-task is still referenced in the DECOMPOSITION.md. If a sub-task is orphaned (no longer in the DECOMPOSITION.md), remove it from the parent's completion check."
|
||||
|
||||
### Bug 6: Orchestrator — Sub-task creation doesn't handle DECOMPOSITION.md edge cases (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Management" section
|
||||
- **Description**: When creating sub-task folders, the Orchestrator doesn't check if the DECOMPOSITION.md has sub-tasks with dependencies that span different waves. If a sub-task in Wave 2 depends on a sub-task in Wave 1, the Orchestrator should ensure Wave 1 sub-tasks are driven to completion before starting Wave 2.
|
||||
- **Reproduction**: Parent task has Wave 1 (sub-task A, sub-task B) and Wave 2 (sub-task C depends on A and B). The Orchestrator creates all three sub-task folders and tries to run them all in parallel.
|
||||
- **Fix Applied**: Added wave enforcement: "The Orchestrator MUST check sub-task dependencies and only start driving Wave 2 sub-tasks when all Wave 1 sub-tasks are in terminal state."
|
||||
|
||||
### Bug 7: Orchestrator — "Continue" command doesn't handle sub-tasks (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Continue from existing tasks" section
|
||||
- **Description**: When the user says "orchestrate" or "continue", the Orchestrator scans for the "most advanced task." But it doesn't distinguish between parent tasks and sub-tasks. A parent task with completed sub-tasks might be more "advanced" than a sub-task that's still in the Research phase.
|
||||
- **Reproduction**: Parent task has 3 sub-tasks. Two sub-tasks are in the Research phase, one is in the Referee phase. The parent task has a VERDICT.md with FAIL. The Orchestrator picks the parent task instead of continuing the sub-task in the Referee phase.
|
||||
- **Fix Applied**: Added: "prioritize sub-tasks over parent tasks" and "When choosing among sub-tasks in the same wave, prioritize those in later phases (e.g., Bug Find over Research) because they are closer to completion."
|
||||
|
||||
### Bug 8: Orchestrator — Auto-execution loop doesn't handle phase failures (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/orchestrate.md`, "Auto-Execution Loop" section
|
||||
- **Description**: The auto-execution loop doesn't check for phase failures that are NOT verdicts — for example, if the agent crashes mid-phase or a phase doesn't produce the expected artifact. The loop assumes every phase produces its artifact and then checks the next phase.
|
||||
- **Reproduction**: Bug Find phase produces an empty `BUG_REPORT.md`. The Orchestrator checks for the artifact, sees it exists, and proceeds to Adversarial Bug Find.
|
||||
- **Fix Applied**: Added to the loop: "if phase artifact is empty or malformed: break (human intervention needed — artifact validation failed)"
|
||||
|
||||
### Bug 9: Orchestrator — Sub-task VRAM_CONFIG.md creation uses wrong units (LOW — FIXED)
|
||||
- **Severity**: Low
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Verdict Reporting" section
|
||||
- **Description**: The VRAM_CONFIG.md template uses `recommended_k` from the detection script output (which is in k units, e.g., "16" for 16k), but the template says "Target VRAM context: {from detection script or config.md override}" without specifying the unit.
|
||||
- **Reproduction**: The detection script outputs "recommended_k: 16" and "max_peak_context_kb: 12000". The VRAM_CONFIG.md template uses "Target VRAM context: 16" (without the "k" suffix) and "Max peak context per sub-task: 12000" (without the "k" suffix), leading to ambiguity about units.
|
||||
- **Fix Applied**: Clarified the units: "Target VRAM context: {value}k tokens (e.g., "16k")", "Max peak context per sub-task: {value}k tokens (e.g., "12k")", "This sub-task's estimated peak context: {value}k tokens (e.g., "10k")". Also added note: "The units must be clarified."
|
||||
|
||||
### Bug 10: Orchestrator — Sub-task PARENT_SPEC.md doesn't include sub-task scope (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Parent Specification" section
|
||||
- **Description**: The PARENT_SPEC.md is supposed to contain "the parent task's SPEC.md content" and "any context the sub-task needs from the parent." But it doesn't include the sub-task's own scope/acceptance criteria from the DECOMPOSITION.md.
|
||||
- **Reproduction**: Parent task's SPEC.md has 5 requirements. The DECOMPOSITION.md says sub-task A is only for requirements 1-2. The PARENT_SPEC.md only contains the parent's SPEC.md (all 5 requirements).
|
||||
- **Fix Applied**: Changed the PARENT_SPEC.md content to include "The sub-task's own scope/acceptance criteria from the DECOMPOSITION.md" and added: "Do NOT include the parent task's full SPEC.md — this can cause circular references if the parent's SPEC.md references the sub-task's SPEC.md files."
|
||||
|
||||
### Bug 11: Orchestrator — No mechanism to handle sub-task failures in parent (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Completion and Parent Task" section
|
||||
- **Description**: When a sub-task FAILs or NEEDS_REVIEW, the Orchestrator reports "human intervention is required" but doesn't create fix tasks for the failing sub-task.
|
||||
- **Reproduction**: Sub-task A FAILs. The Orchestrator reports "human intervention is required." The parent task is stuck.
|
||||
- **Fix Applied**: Added fix task creation for sub-tasks in Manual Mode: "Task name: `{parent-task-name}-fix-{sub-task-name}` (e.g., `add-user-auth-fix-auth-gateway`) — The Orchestrator creates the folder with an empty `IMPLEMENTATION.md` — The task starts at the **Bug Find** phase — The Orchestrator copies the sub-task's `SPEC.md`, `BUG_REPORT.md`, and `ADVERSARIAL_BUG_REPORT.md` (if they exist) into the new task folder"
|
||||
|
||||
### Bug 12: Orchestrator — Auto-detect VRAM doesn't handle missing detection script (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "VRAM Detection" section
|
||||
- **Description**: The Orchestrator's VRAM detection priority says "Auto-detect via script: Run {project}/.agent-framework/scripts/vram_detect.sh". If the script is not available, it falls back to "Auto-detect via API config." But the Orchestrator doesn't check if the detection script exists before trying to run it.
|
||||
- **Reproduction**: User starts a new task. The Orchestrator tries to run `{project}/.agent-framework/scripts/vram_detect.sh` but the script doesn't exist.
|
||||
- **Fix Applied**: Added: "Check if `{project}/.agent-framework/scripts/vram_detect.sh` exists. If it does, run it..." and "If the detection script does not exist, skip to the next detection method."
|
||||
|
||||
### Bug 13: Orchestrator — Auto-detect VRAM doesn't handle script failure (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "VRAM Detection" section
|
||||
- **Description**: Even if the detection script exists, it might fail (e.g., `nvidia-smi` is not installed, the GPU is busy, etc.). The Orchestrator doesn't handle script failures gracefully.
|
||||
- **Reproduction**: The detection script exists but `nvidia-smi` is not installed. The script fails with an error.
|
||||
- **Fix Applied**: Added: "If the detection script fails (e.g., `nvidia-smi` is not installed, the GPU is busy, etc.), check the exit code and fall back to the next detection method."
|
||||
|
||||
### Bug 14: Orchestrator — Sub-task completion doesn't aggregate verdicts for parent (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Verdict Reporting" section
|
||||
- **Description**: The Orchestrator says "The Orchestrator should aggregate sub-task verdicts when reporting the parent task's status." But it doesn't actually implement this aggregation.
|
||||
- **Reproduction**: Parent task has 3 sub-tasks. Two PASS, one FAIL. The Orchestrator reports the parent task as "Complete" because it doesn't aggregate sub-task verdicts.
|
||||
- **Fix Applied**: Added: "The Orchestrator MUST aggregate sub-task verdicts when reporting the parent task's status: If ANY sub-task FAILs or NEEDS_REVIEW, the parent task should be marked as **Human Intervention** regardless of whether the parent's own VERDICT.md says PASS."
|
||||
|
||||
### Bug 15: Orchestrator — No mechanism to handle sub-task "Tie-Breaks" (LOW — FIXED)
|
||||
- **Severity**: Low
|
||||
- **Location**: `prompts/referee.md`
|
||||
- **Description**: The referee has "Tasks for Review / Tie-Breaks" but the Orchestrator doesn't have logic to handle sub-task tie-breaks.
|
||||
- **Reproduction**: Sub-task A has tie-breaks. The Orchestrator doesn't create tie-break tasks for the sub-task.
|
||||
- **Fix Applied**: Added: "If a sub-task has 'Tasks for Review / Tie-Breaks' in its VERDICT.md, the Orchestrator should create tie-break tasks for the sub-task: Task name: `{parent-task-name}-tiebreak-{sub-task-name}`"
|
||||
|
||||
### Bug 16: Orchestrator — State Determination doesn't check for empty artifacts (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "State Determination" section
|
||||
- **Description**: The Orchestrator checks for the existence of artifacts (e.g., "Has `SPEC.md`") but doesn't check if they are empty.
|
||||
- **Reproduction**: Research phase produces an empty `SPEC.md`. The Orchestrator checks for `SPEC.md` and sees it exists, so it classifies the task as "Design" (optional) or "Implement".
|
||||
- **Fix Applied**: Added "(non-empty)" after every artifact check: "Has `VERDICT.md` with `PASS` (non-empty)", "Has `DOC_REVIEW.md` (non-empty)", etc.
|
||||
|
||||
### Bug 17: Orchestrator — "Continue" doesn't prioritize sub-tasks in the same wave (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Continue from existing tasks" section
|
||||
- **Description**: When the user says "orchestrate" or "continue", the Orchestrator should prioritize sub-tasks in the same wave that are still in progress.
|
||||
- **Reproduction**: Wave 1 has sub-tasks A, B, and C. A is in the Research phase, B is in the Implement phase, and C is in the Bug Find phase. The Orchestrator picks A (Research phase) instead of C (Bug Find phase).
|
||||
- **Fix Applied**: Added: "When choosing among sub-tasks in the same wave, prioritize those in later phases (e.g., Bug Find over Research) because they are closer to completion."
|
||||
|
||||
### Bug 18: Orchestrator — Auto-Execution Rules section is duplicated (LOW — FIXED)
|
||||
- **Severity**: Low
|
||||
- **Location**: `prompts/orchestrate.md`, "Auto-Execution Rules (Autopilot Mode Only)" section
|
||||
- **Description**: The section "Auto-Execution Rules (Autopilot Mode Only)" appears after the "Auto-Execution Loop" section and contains overlapping rules.
|
||||
- **Reproduction**: Read the file; observe that the Auto-Execution Loop and Auto-Execution Rules sections contain overlapping rules.
|
||||
- **Fix Applied**: Consolidated the Auto-Execution Loop and Auto-Execution Rules sections into a single section with additional rules for artifact validation, timeout, iteration limit, and sub-task parallel execution.
|
||||
|
||||
---
|
||||
|
||||
### Bug 14: No test design phase — tests written on the fly during implementation
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/implement.md`, `prompts/referee.md`, `prompts/workflow.md`
|
||||
- **Description**: Tests are written during the implementation phase as part of the TDD loop. There is no formal Test Design phase before implementation starts. The spec says "acceptance criteria are testable" but doesn't require test cases to be defined upfront. This means:
|
||||
- The implementer writes tests on the fly, so there's no test specification to verify against later
|
||||
- The referee checks "Are edge cases covered?" and "Do all tests pass?" but doesn't have a test spec to compare against
|
||||
- There's no way for the user to sign off on test coverage before implementation starts
|
||||
- **Reproduction**: Start "Implement add user auth". The implementer writes tests during the TDD loop. No one has reviewed or approved the test coverage before code was written.
|
||||
- **Suggested Fix**: **RESOLVED** — Added a Test Design phase (optional) between Design and Implement. Produces TEST_PLAN.md — an explicit test specification that the implementer follows. The referee checks that all test cases from the TEST_PLAN.md are implemented.
|
||||
## Adversarial Bugs Found and Fixed
|
||||
|
||||
### Bug 11: Orchestrator contradiction — Autopilot mode task creation from verdicts
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Tasks from bug verdicts" section
|
||||
- **Description**: The "Tasks from bug verdicts" section says the Orchestrator MUST create tasks from FAIL/NEEDS_REVIEW verdicts in Autopilot mode, but then there's a note saying NOT to create them in Autopilot mode. This is contradictory. The note is correct — in Autopilot mode, the Orchestrator should pause when a FAIL verdict is found and let the user decide. The main text incorrectly says to auto-create tasks in Autopilot mode.
|
||||
- **Reproduction**: Read `prompts/orchestrate.md`; the "Tasks from bug verdicts" section says "MUST also create new tasks and drive them" in Autopilot mode, but the note says "the Orchestrator should NOT create fix/review/tiebreak tasks from FAIL/NEEDS_REVIEW verdicts — it should pause and let the user decide".
|
||||
- **Suggested Fix**: **RESOLVED** — Split the section into Manual Mode (create tasks) and Autopilot Mode (pause and report human intervention required).
|
||||
### Adversarial Bug 1: Orchestrator — Auto-execution loop can run infinitely (CRITICAL — FIXED)
|
||||
- **Severity**: Critical
|
||||
- **Location**: `prompts/orchestrate.md`, "Auto-Execution Loop" section
|
||||
- **Description**: The auto-execution loop has no maximum iteration count or timeout. If the agent produces an artifact but doesn't output CONTRACT_MET, the loop will spin forever.
|
||||
- **Fix Applied**: Added to the loop: "if iteration_count >= MAX_ITERATIONS (default: 10): break (human intervention needed — too many iterations)", "if total_time_elapsed >= MAX_TOTAL_TIME (default: 24 hours): break (human intervention needed — too much time elapsed)", "if phase_time_elapsed >= MAX_PHASE_TIME (default: 1 hour): break (human intervention needed — phase took too long)"
|
||||
|
||||
### Bug 12: No update mechanism for existing projects
|
||||
### Adversarial Bug 2: Orchestrator — Sub-task creation doesn't prevent duplicate sub-tasks (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Management" section
|
||||
- **Description**: When the Orchestrator creates sub-task folders, it doesn't check if they already exist.
|
||||
- **Fix Applied**: Added: "Check for existing sub-task folders: For each sub-task, check if the folder `tasks/{parent-task-name}/subtasks/{sub-task-name}/` already exists. If it does, skip the creation and report that the sub-task has already been created."
|
||||
|
||||
### Adversarial Bug 3: Orchestrator — Auto-detect VRAM can cause resource exhaustion (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/orchestrate.md`, "VRAM Detection" section
|
||||
- **Description**: If the VRAM detection script is run in a loop (e.g., the Orchestrator is invoked multiple times), it will repeatedly probe the GPU and RAM, causing performance degradation.
|
||||
- **Fix Applied**: Added VRAM detection caching: "When the Orchestrator is invoked multiple times (e.g., the user says 'orchestrate' twice), it MUST cache the VRAM detection results and reuse them instead of running the detection script again."
|
||||
|
||||
### Adversarial Bug 4: Orchestrator — Sub-task completion doesn't check for orphaned sub-tasks (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Completion and Parent Task" section
|
||||
- **Description**: The Orchestrator doesn't check if there are orphaned sub-tasks — sub-tasks that were created by the Orchestrator but are no longer referenced in the DECOMPOSITION.md.
|
||||
- **Fix Applied**: Added: "Check for orphaned sub-tasks: Before checking completion, verify that each sub-task is still referenced in the DECOMPOSITION.md. If a sub-task is orphaned (no longer in the DECOMPOSITION.md), remove it from the parent's completion check."
|
||||
|
||||
### Adversarial Bug 5: Orchestrator — Auto-execution loop doesn't handle concurrent sub-tasks (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/orchestrate.md`, "Auto-Execution Loop" section
|
||||
- **Description**: The auto-execution loop only drives one sub-task at a time, even when sub-tasks are in the same wave and can run in parallel.
|
||||
- **Fix Applied**: Added: "Sub-task Parallel Execution: When sub-tasks are in the same wave and can run in parallel, the Orchestrator should drive them simultaneously instead of sequentially."
|
||||
|
||||
### Adversarial Bug 6: Orchestrator — State Determination can produce ambiguous states (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `README.md`, `install.sh`, `prompts/onboarding.md`
|
||||
- **Description**: There is no mechanism to update an existing project's framework files when the global framework is updated. The README mentions "Upgrade" as a project type in the onboarding section but has no implementation. The install.sh only mentions `git pull` as a vague update hint. When the framework adds new phases (e.g., Doc Review), existing projects won't have the new prompt files and won't know about them.
|
||||
- **Reproduction**: User updates the global framework with new phases. Existing projects still only have the old prompt files. The agent doesn't know about the new phases for those projects.
|
||||
- **Suggested Fix**: **RESOLVED** — Added `update.sh` script for updating the global framework, added "Upgrade" option in onboarding prompt for existing projects, updated README with update instructions.
|
||||
- **Location**: `prompts/orchestrate.md`, "State Determination" section
|
||||
- **Description**: The state determination has multiple overlapping conditions that can produce ambiguous states.
|
||||
- **Fix Applied**: Added: "Note on overlapping conditions: If a task has both `TEST_PLAN.md` and `DESIGN.md`, the Orchestrator should prioritize the more advanced state (TEST_PLAN.md → Implement) over the optional state (DESIGN.md → Test Design). Similarly, if a task has both `IMPLEMENTATION.md` and `BUG_REPORT.md`, the Orchestrator should prioritize the more advanced state (BUG_REPORT.md → Adversarial Bug Find) over the earlier state (IMPLEMENTATION.md → Bug Find)."
|
||||
|
||||
### Adversarial Bug 7: Orchestrator — Sub-task PARENT_SPEC.md can cause circular references (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Parent Specification" section
|
||||
- **Description**: The PARENT_SPEC.md contains the parent task's SPEC.md content. If the parent's SPEC.md references the sub-task's SPEC.md files, a circular reference is created.
|
||||
- **Fix Applied**: Changed the PARENT_SPEC.md content to include "The sub-task's own scope/acceptance criteria from the DECOMPOSITION.md" and added: "Do NOT include the parent task's full SPEC.md — this can cause circular references if the parent's SPEC.md references the sub-task's SPEC.md files."
|
||||
|
||||
### Adversarial Bug 8: Orchestrator — Auto-detect VRAM can cause memory exhaustion (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "VRAM Detection" section
|
||||
- **Description**: If the detection script doesn't exist, the Orchestrator tries to read multiple config files to detect the model name. If the .env file is large, reading it could cause memory exhaustion.
|
||||
- **Fix Applied**: Added: "Only read the specific lines needed (e.g., the model name line), not the entire file. Limit file reads to 10KB to prevent memory exhaustion."
|
||||
|
||||
### Adversarial Bug 9: Orchestrator — Sub-task completion doesn't handle sub-task failures gracefully (HIGH — FIXED)
|
||||
- **Severity**: High
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Completion and Parent Task" section
|
||||
- **Description**: When a sub-task FAILs during the Referee phase (before producing BUG_REPORT.md and ADVERSARIAL_BUG_REPORT.md), the Orchestrator wouldn't have the bug reports needed to create a fix task.
|
||||
- **Fix Applied**: Added: "If the sub-task failed during the Referee phase (before producing BUG_REPORT.md and ADVERSARIAL_BUG_REPORT.md), the Orchestrator still creates the fix task but only copies the artifacts that exist (BUG_REPORT.md if it exists, ADVERSARIAL_BUG_REPORT.md if it exists)."
|
||||
|
||||
### Adversarial Bug 10: Orchestrator — Sub-task creation doesn't handle DECOMPOSITION.md updates (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Management" section
|
||||
- **Description**: If the DECOMPOSITION.md is updated after the Orchestrator has already created sub-task folders, the Orchestrator doesn't handle the update.
|
||||
- **Fix Applied**: Added: "Check for DECOMPOSITION.md updates: Compare the DECOMPOSITION.md with the existing sub-task folders. If the DECOMPOSITION.md has been updated (new sub-tasks added or existing sub-tasks removed), update the sub-task folders accordingly."
|
||||
|
||||
### Adversarial Bug 11: Orchestrator — Auto-execution loop doesn't handle phase timeouts (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Auto-Execution Loop" section
|
||||
- **Description**: The auto-execution loop doesn't have a timeout for each phase.
|
||||
- **Fix Applied**: Added to the loop: "if phase_time_elapsed >= MAX_PHASE_TIME (default: 1 hour): break (human intervention needed — phase took too long)"
|
||||
|
||||
### Adversarial Bug 12: Orchestrator — Sub-task VRAM_CONFIG.md doesn't include sub-task-specific VRAM limits (MEDIUM — FIXED)
|
||||
- **Severity**: Medium
|
||||
- **Location**: `prompts/orchestrate.md`, "Sub-Task Verdict Reporting" section
|
||||
- **Description**: The VRAM_CONFIG.md includes "Max peak context per sub-task: {from detection script or config.md override}" which is the global max peak context from the detection script. But it doesn't include the sub-task's own estimated peak context from the DECOMPOSITION.md.
|
||||
- **Fix Applied**: Added to the VRAM_CONFIG.md template: "This sub-task's estimated peak context: {from DECOMPOSITION.md}k tokens (e.g., "10k")" and "Fits within VRAM: Yes/No"
|
||||
|
||||
---
|
||||
|
||||
@@ -126,18 +216,34 @@ A comprehensive review of the agent-framework project identified **13 bugs** (af
|
||||
|
||||
| Bug | Severity | Score | Status |
|
||||
|-----|----------|-------|--------|
|
||||
| 1 | Medium | +5 | **RESOLVED** — Rewrote `design.md`; duplicate sections no longer exist |
|
||||
| 2 | High | +5 | **RESOLVED** — Verdicts now auto-create tasks (fix/review/tiebreak) |
|
||||
| 3 | High | +5 | Known issue — placeholder URL in install.sh |
|
||||
| 4 | High | +5 | Known issue — placeholder URL in README.md |
|
||||
| 5 | Low | +1 | **RESOLVED** — Rewrote `design.md`; duplicate section and unused placeholder no longer exist |
|
||||
| 6 | Low | +1 | **RESOLVED** — `{task-description}` is used to determine if user is starting new task or continuing |
|
||||
| 7 | Critical | +10 | **RESOLVED** — Orchestrator is now a real state machine with auto-transitions |
|
||||
| 8 | High | +5 | **RESOLVED** — User can just say "orchestrate" or "continue" |
|
||||
| 9 | High | +5 | **RESOLVED** — Dedicated Doc Review phase added between Adversarial Bug Find and Referee |
|
||||
| 10 | High | +5 | **RESOLVED** — Research and Design phases now interactive with user |
|
||||
| 11 | Medium | +5 | **RESOLVED** — Split Orchestrator verdict task creation into Manual/Autopilot modes |
|
||||
| 12 | Medium | +5 | **RESOLVED** — Added `update.sh` script, upgrade option in onboarding, update instructions in README |
|
||||
| 13 | Low | +1 | **RESOLVED** — Autopilot is now Enabled by default (was Disabled) |
|
||||
| 14 | Medium | +5 | **RESOLVED** — Added Test Design phase (optional) between Design and Implement |
|
||||
| **Total** | | **58** | |
|
||||
| 1 | Critical | +10 | **FIXED** — State determination reordered |
|
||||
| 2 | Medium | +5 | **FIXED** — Duplicate paragraph removed |
|
||||
| 3 | High | +5 | **FIXED** — Empty IMPLEMENTATION.md creation removed |
|
||||
| 4 | High | +5 | **FIXED** — VERDICT.md check added |
|
||||
| 5 | High | +5 | **FIXED** — Orphaned sub-tasks check added |
|
||||
| 6 | Medium | +5 | **FIXED** — Wave enforcement added |
|
||||
| 7 | Medium | +5 | **FIXED** — Sub-task prioritization added |
|
||||
| 8 | High | +5 | **FIXED** — Artifact validation added |
|
||||
| 9 | Low | +1 | **FIXED** — Units clarified in VRAM_CONFIG.md |
|
||||
| 10 | Medium | +5 | **FIXED** — Sub-task scope added to PARENT_SPEC.md |
|
||||
| 11 | High | +5 | **FIXED** — Sub-task fix task creation added |
|
||||
| 12 | Medium | +5 | **FIXED** — Script existence check added |
|
||||
| 13 | Medium | +5 | **FIXED** — Script failure handling added |
|
||||
| 14 | Medium | +5 | **FIXED** — Sub-task verdict aggregation added |
|
||||
| 15 | Low | +1 | **FIXED** — Sub-task tie-break task creation added |
|
||||
| 16 | Medium | +5 | **FIXED** — Empty artifact checks added |
|
||||
| 17 | Medium | +5 | **FIXED** — Sub-task wave prioritization added |
|
||||
| 18 | Low | +1 | **FIXED** — Auto-Execution Rules consolidated |
|
||||
| Adv1 | Critical | +10 | **FIXED** — Auto-execution loop timeout/iteration limit added |
|
||||
| Adv2 | High | +5 | **FIXED** — Duplicate sub-task prevention added |
|
||||
| Adv3 | High | +5 | **FIXED** — VRAM detection caching added |
|
||||
| Adv4 | High | +5 | **FIXED** — Orphaned sub-tasks check added |
|
||||
| Adv5 | High | +5 | **FIXED** — Sub-task parallel execution added |
|
||||
| Adv6 | Medium | +5 | **FIXED** — Overlapping conditions note added |
|
||||
| Adv7 | Medium | +5 | **FIXED** — Circular reference prevention added |
|
||||
| Adv8 | Medium | +5 | **FIXED** — Memory exhaustion prevention added |
|
||||
| Adv9 | High | +5 | **FIXED** — Graceful sub-task failure handling added |
|
||||
| Adv10 | Medium | +5 | **FIXED** — DECOMPOSITION.md update handling added |
|
||||
| Adv11 | Medium | +5 | **FIXED** — Phase timeout added |
|
||||
| Adv12 | Medium | +5 | **FIXED** — Sub-task VRAM limit added to VRAM_CONFIG.md |
|
||||
| **Total** | | **143** | |
|
||||
|
||||
Reference in New Issue
Block a user