Skip to content

Address lightweight follow-up issues #266, #269, #271, #274, #275#277

Merged
wwind123 merged 1 commit into
mainfrom
codex/fix-issues-266-269-271-274-275
Jun 7, 2026
Merged

Address lightweight follow-up issues #266, #269, #271, #274, #275#277
wwind123 merged 1 commit into
mainfrom
codex/fix-issues-266-269-271-274-275

Conversation

@wwind123

@wwind123 wwind123 commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Summary

This PR addresses five lightweight follow-up issues from earlier planning and PR reviews.

#266 — stderr routing uniform across all backends

Verified that run_with_log already uses stderr=subprocess.STDOUT for all three agent backends (Claude, Codex, Gemini). Added a comment in runner.py documenting this intentional, uniform behavior.

#269 — Non-compact plan revision prompt missing workdir guidance

Added _coder_workdir_guidance(config, implementation=False) to the non-compact build_plan_revision_prompt return value (line 1187 in prompts.py), matching the compact path that already included it via _compact_plan_stable_prefix.

#271 — coder_followup path through attempt_envelope_normalization untested

Added three unit tests covering the coder_followup path through attempt_envelope_normalization: duplicate AGENT_STATE footer (fixed), trailing prose after signature (fixed), and prose before footer (returns None).

#274 — Combined envelope+disposition strip

When attempt_envelope_normalization fixes a structural defect but validate(normalized) raises UnknownPriorItemDispositionError, the orchestrator now attempts a combined deterministic strip from the normalized candidate. This resolves both defects in one pass without falling through to the full repair pass. Added an integration test that exercises this combined path.

#275 — json.dumps tight-packing fix

strip_unknown_prior_item_dispositions now inserts a \n separator between the re-encoded JSON and the trailing footer/signature when the original had no whitespace between them. Added tests verifying tight-packed inputs produce valid output and survive parse_structured_pr_review validation.

Test plan

Closes #266, #269, #271, #274, #275

🤖 Generated with Claude Code

- #266: Document that run_with_log uses stderr=subprocess.STDOUT uniformly
  for all agent backends (Claude, Codex, Gemini)

- #269: Add workdir guidance to the non-compact build_plan_revision_prompt
  path, matching the compact path that already included it

- #271: Add unit tests covering the coder_followup path through
  attempt_envelope_normalization (duplicate footer, trailing prose, prose
  before footer)

- #274: When envelope normalization fixes a structural defect but
  validate(normalized) raises UnknownPriorItemDispositionError, attempt a
  combined deterministic strip from the normalized candidate so both defects
  are resolved in one pass

- #275: Ensure strip_unknown_prior_item_dispositions inserts a newline
  separator between re-encoded JSON and the trailing footer when the
  original was tightly packed; add tests verifying tight-pack output
  validates correctly

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@wwind123

wwind123 commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

Opened PR #277 addressing all five issues.

#266 — stderr routing verified uniform: All three agent backends (Claude, Codex, Gemini) call run_with_log, which already uses stderr=subprocess.STDOUT at the Popen layer. Added a comment in runner.py documenting this intentional, uniform behavior.

#269 — Non-compact plan revision prompt gains workdir guidance: Added _coder_workdir_guidance(config, implementation=False) to the non-compact build_plan_revision_prompt return value (prompts.py), matching the compact path that already included it via _compact_plan_stable_prefix.

#271 — coder_followup path through attempt_envelope_normalization now tested: Added three unit tests: duplicate AGENT_STATE footer (normalized), trailing prose after signature (stripped), prose before footer (returns None).

#274 — Combined envelope+disposition strip: When attempt_envelope_normalization fixes a structural defect but validate(normalized) raises UnknownPriorItemDispositionError, the orchestrator now tries strip_unknown_prior_item_dispositions on the normalized candidate in the same step, resolving both defects without falling through to the full repair pass. Only activates when the original error was structural (not itself an UnknownPriorItemDispositionError), preserving the existing block-2 flow for the simpler case.

#275 — Tight-packing newline fix: strip_unknown_prior_item_dispositions now inserts a \n separator between the re-encoded JSON and the trailing footer when absent. Added two tests: one verifying the separator is present, one confirming the result passes parse_structured_pr_review validation.

Tests: python3 -m pytest tests/test_agent_loop.py -q — 707 passed (8 new tests added).

-- Anthropic Claude

@wwind123

wwind123 commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

Review verdict: Approved

No blocking issues found in the focused fixes for prompt workdir guidance, deterministic response repair, stderr capture documentation, or the added regression tests.

-- OpenAI Codex

@wwind123

wwind123 commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

Review verdict: Approved

Review complete. The PR successfully addresses the follow-up issues, including adding necessary workdir guidance to the non-compact plan revision prompt, correctly handling combined repair paths, and accurately applying tighter spacing in stripped payloads.

-- Google Gemini

@wwind123 wwind123 merged commit aa7b37d into main Jun 7, 2026
1 check passed
@wwind123 wwind123 deleted the codex/fix-issues-266-269-271-274-275 branch June 7, 2026 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow up future plan-review note: The run_with_log path in Runner writes to a log file but does not capture or expose

1 participant