fix(planner): keep continued answers whole and never save a partial or empty plan - #2589
Conversation
- planValidation.ts: one dependency-free validator script (JSON array, complete title/body/implementation, and, against an original, no reworded, dropped or invented content) that agents run in their workspace and ProPR re-runs on fresh copies; fragment detection. - planFileAgent.ts: runs an agent task in a throwaway workspace under the shared worktree root and accepts only a plan.json that passes ProPR's own validation. - Agent task logs can be attributed to plan work instead of implementation through task metadata (all five agents). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…r empty plan
Plan 027d8f35 was regenerated with Opus 5.5 (129,562 output tokens) and
saved as one empty task: Claude Code's result line carries only the last
assistant message, so a reply continued after the output-token limit
reached the parser as a 3,542-character tail. JSON repair then wrapped
that fragment into a single {content} item, which nothing validated.
- Claude analysis text joins every assistant message of the final answer
when the result line is its continued tail, and logs a warning.
- A response that is not a whole JSON array is rejected, not repaired.
- Malformed but whole plans are repaired by the default coding agent in
a scratch workspace: it edits plan.json until the validator passes,
and ProPR accepts it only if its own copy of the validator confirms
the plan is complete and keeps the original content.
- Generated plans with tasks lacking a title, body or implementation
fail the generation instead of being saved.
- Agent tasks can raise the turn limit (plan agents get 200), since hosts
may set CLAUDE_MAX_TURNS low.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
/review astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — astraOverall EvaluationThe PR addresses continued responses, incomplete tasks, and file-based JSON repair with useful focused tests. It needs changes before merge: fragment detection can still admit truncated plans, and the preservation validator both accepts content loss and rejects some unchanged content. ✅ Independent validation — ProPR validates fresh copies of the result and original, preventing edits to workspace validation files from bypassing its checks. ✅ Task completeness — Generation now rejects tasks missing a non-empty title, body, or implementation before enforcing granularity. The supplied current-head checks report 24 passed and no failures. This review used static analysis only; no commands were run. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F1: 🔴 Duplicated tasks can conceal dropped content
F2: 🔴 Brackets inside strings admit truncated plans
F3: 🔴 Literal Unicode escapes fail unchanged repairs
SuggestionsThese are optional follow-ups and are not sent to S1: 🟢 Exercise a real repair agentA follow-up integration test using an actual configured agent would validate the scratch repository, shared Docker path, file permissions, and validator invocation together. The fake-agent tests establish the runner's orchestration but bypass these integration boundaries; no concrete failure in those boundaries is established here, so this is optional. ScoreThe implementation has useful safeguards and passing current checks, but demonstrated validation defects undermine the central guarantee that repairs preserve complete plans. These require correction before merge. Score: 5/10 🤖 Review Details
🤖 Review by ProPR |
|
/fix |
|
✅ Applied the requested follow-up changes in commit 2566461
Summary of ChangesImplemented F1–F3:
Added focused regressions. All 45 relevant tests and the core TypeScript check pass. Separate discovery, left unchanged: the existing generic JSON parser strips embedded triple-backtick fences from task text. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
![propr-dev[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
Implemented F1–F3: - Validation preserves task occurrences, order, and field associations. - Boundary checks reject truncated plans before parsing or repair. - Escape decoding preserves literal `\\u0041`. Added focused regressions. All 45 relevant tests and the core TypeScript check pass. Separate discovery, left unchanged: the existing generic JSON parser strips embedded triple-backtick fences from task text. PR: #2589 Comment by: @integry (ID: 5878174096) Model: gpt-6-astra
|
CI failed: Validate Changes Please investigate and fix this CI failure.
Failure evidence
|
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
|
✅ Applied the requested follow-up changes in commit e18d99a Summary of ChangesFixed the CI failure by removing an unnecessary regex escape in Validation: ESLint passes, all 29 focused tests pass, and The Ubuntu migration notice was unrelated. No visual changes or previews. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…e in `planValidation.ts:71`, preserving behavior. Fixed the CI failure by removing an unnecessary regex escape in `planValidation.ts:71`, preserving behavior. Validation: ESLint passes, all 29 focused tests pass, and `git diff --check` passes. The Ubuntu migration notice was unrelated. No visual changes or previews. PR: #2589 Comment by: @propr-dev[bot] (ID: 5878331682) Model: gpt-6-astra
|
ℹ️ Analyzed the follow-up request by @github-actions[bot] Analysis SummaryThe reported lint issue is already fixed on this branch in Verified:
No visual preview is needed for this regex-only fix. No code changes were necessary based on the current state of the branch. 🤖 Analysis Details
💡 ProPR Slash Commands
🤖 Generated by ProPR |
|
/review astra |
|
✅ AI Code Review Complete requested by @integry Posted 1 review: |
🔍 AI Code Review — astraOverall EvaluationThe PR addresses lost continuation text and introduces file-based repair with independent validation. It needs changes before merge: the fragment detector rejects some complete, repairable plans, and OpenCode’s exception path loses plan attribution. ✅ Independent validation — ProPR validates fresh copies of the result and original with its own validator, preventing workspace edits from bypassing the checks. ✅ Incomplete-task rejection — Generation now rejects tasks without non-empty titles, bodies, and implementation sections before saving. The supplied current-head status shows no failures and three pending checks. This review used static analysis only; no commands were run. Merge blockersEvery finding below was introduced by this PR and must be resolved before merging. F4: 🔴 Allow repairable quotes through fragment detection
F5: 🔴 Preserve attribution on OpenCode exceptions
SuggestionsThese are optional follow-ups and are not sent to S2: 🟢 Exercise a real repair workspaceA smoke test using a supported agent would complement the fake-agent tests by checking container mounts, workspace permissions, Git initialization, and validator execution together. This is optional integration coverage; the supplied code does not establish a separate defect in those paths. ScoreThe independent validation and focused tests are strong, but two localized correctness gaps remain. Current checks report no failures; pending checks alone do not reduce the score. Score: 6/10 🤖 Review Details
🤖 Review by ProPR |
|
/ultrafix |
|
🔄 Ultrafix loop started (goal: 8/10, max cycles: 10) First action:
|
|
✅ Applied the requested follow-up changes in commit 72fa562
Summary of ChangesImplemented F4 and F5 only.
Validation: 105 tests passed; core type checking passed. Five new regressions fail against the original code. Separate observation, unchanged: other agents’ exception handlers omit LLM failure-log persistence entirely. No visual changes; no previews generated. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
Implemented F4 and F5 only. - Repairable quoted delimiters now reach syntax-only repair; fragment and trailing-task rejection remain covered. - OpenCode exception logs retain plan execution type, draft reference, and metadata. Validation: 105 tests passed; core type checking passed. Five new regressions fail against the original code. Separate observation, unchanged: other agents’ exception handlers omit LLM failure-log persistence entirely. No visual changes; no previews generated. PR: #2589 Comment by: @integry (ID: 5878711924) Model: gpt-6-astra
|
CI failed: Validate Changes Please investigate and fix this CI failure.
Failure evidence
|
Checks FailedValidation failed during setup, tests, CLI packaging, linting, or build checks. View Logs |
|
✅ Applied the requested follow-up changes in commit 5935d93 Processed 2 comments:
Summary of ChangesFixed the CI lint failure by extracting the delimiter scan from Verified locally:
No visual preview needed; this is an internal refactor. 🤖 Implementation Details
Undo Changes • View Task Execution 💡 ProPR Slash Commands
🤖 Generated by ProPR |
…from `extractWholeJsonArray` into a helper, preserving parsing behavior and meeting the complexity limit. Fixed the CI lint failure by extracting the delimiter scan from `extractWholeJsonArray` into a helper, preserving parsing behavior and meeting the complexity limit. Verified locally: - Core lint passes with zero warnings. - Core build passes. - All 28 plan validation and JSON repair tests pass. No visual preview needed; this is an internal refactor. PR: #2589 Comment by: @propr-dev[bot] (ID: 5878907133) Comment by: @github-actions[bot] (ID: 5878907354) Model: gpt-6-astra
|
✅ AI Code Review Complete requested by @propr-ultrafix Posted 1 review: |
🔍 AI Code Review — codex:gpt-6-astraOverall EvaluationThe PR addresses continued Claude responses, incomplete tasks, and file-based JSON repair with clear validation boundaries. It is merge-ready within the supplied scope; I found no verified merge blocker. This was a static review of the supplied code; no commands were run. ✅ Independent repair validation — ProPR validates fresh copies of the resulting plan and original text, so edits to the workspace validator or original cannot bypass acceptance checks. ✅ Task completeness checks — Generation rejects tasks missing a non-empty title, body, or implementation before granularity enforcement and persistence. ✅ Focused regression coverage — The supplied tests cover fragment rejection, content preservation, workspace cleanup, and repair-log attribution. Current head checks report 24 passed and no failures. Merge blockersNo merge blockers. SuggestionsThese are optional follow-ups and are not sent to S3: 🟢 Capture real continuation outputAdd a sanitized Claude stream fixture when available. S4: 🟢 Clarify supported syntax repairsThe repair prompt permits missing or extra commas, brackets, and braces, while the original-content parser deliberately supports narrower tolerances and rejects ambiguous input. Documenting those limits—or adding fixtures for additional unambiguous syntax errors—would make repair expectations clearer. The current refusal preserves content safety, so broader repair support is optional. ScoreThe implementation provides meaningful safeguards and targeted tests, with all current checks passing. Real-agent repair and continuation fixtures would further strengthen confidence. Score: 8/10 🤖 Review Details
🤖 Review by ProPR |
2 participants
What happened
Plan 027d8f35 was regenerated with Opus 5.5 (granular) and saved as one empty task.
resultline holds only the last assistant message. When a reply reaches the output-token limit, Claude Code has the model continue in a new message, so only the tail of the plan survived.{content: …}item, and nothing checked that tasks were complete.Changes
getClaudeAnalysisTextjoins every assistant text block after the last tool result when the result line is the tail of that combined answer. Ordinary results are unchanged, and a warning is logged when a continuation is used.plan.json,original.txtandvalidate-plan.mjs.title/body/implementationin every task, and no content reworded, dropped or invented compared with the original.AgentTaskOptions.maxTurnsis new. Plan agents get 200, because this host setsCLAUDE_MAX_TURNS=10.The shared validator and workspace runner are also the base for #2588 (file-based plan generation, stacked on this PR).
Tests
test/planValidation.test.ts:test/claudeContinuedAnswer.test.ts: continued replies, repeated stream lines, tool-result boundaries, unchanged ordinary results.test/planGenerationJsonRepair.test.ts, rewritten for the new flow: agentic repair wiring, fragment rejected, incomplete tasks rejected, repair failure propagated, valid plan untouched.previewMediaProjection, also fails on main in this environment.Not verified with a real agent run: the repair path uses fake agents in tests.
🤖 Generated with Claude Code