Skip to content

Harden review parsing and background monitor status - #3

Closed
BoldNewMedia wants to merge 2 commits into
yanchuk:mainfrom
BoldNewMedia:harden-review-json-and-agents-monitor
Closed

BoldNewMedia wants to merge 2 commits into
yanchuk:mainfrom
BoldNewMedia:harden-review-json-and-agents-monitor

Conversation

@BoldNewMedia

Copy link
Copy Markdown

Summary

  • Read background session lifecycle with claude agents --json instead of the TTY-oriented claude agents output.
  • Use the matching background session state to mark jobs completed when Claude reports state: done.
  • Tolerate Claude JSON envelopes whose result begins with <function_calls>...</function_calls> before the review JSON.
  • Keep direct review payload validation strict, so arbitrary leading prose is still rejected.

Why

In non-interactive Codex sessions, claude agents can fail with a TTY error while claude agents --json works. Also, Claude can return a successful JSON envelope whose result string includes tool-call markup before the requested review JSON, which currently causes strict parsing to fail despite the review findings being present.

Verification

  • node --test tests/runtime.test.mjs → 19/19 pass
  • node --test tests/commands.test.mjs → 26/26 pass
  • node --test tests/*.test.mjs → 45/45 pass
  • node tests/validate-plugin.mjs → plugin metadata ok
  • git diff --check → clean

Use claude agents --json when monitoring background jobs so non-TTY Codex sessions can read lifecycle state and mark completed sessions done.

Also tolerate Claude JSON envelopes whose result string begins with Claude tool-call markup before the review JSON. Direct review payload validation remains strict so arbitrary leading prose is still rejected.

Tests cover agents JSON completion and tool-call-prefixed review output.
Handle Claude JSON envelopes whose result begins with status prose before the requested review JSON.

The direct review payload validator remains strict; this recovery is limited to Claude CLI envelope results. Adds a regression test for a result beginning with 'Now I have enough context to review.'

Copy link
Copy Markdown
Author

Follow-up pushed in 283018f after another real Codex/Claude run hit a stricter variant of the same parser issue:

Unexpected token 'N', "Now I have"... is not valid JSON

That happens when the Claude CLI JSON envelope is valid, but its result string contains a short status/prose line before the requested review JSON. The new commit extracts the first complete JSON object from the envelope result after removing optional tool-call markup. Direct review payload validation remains strict.

Re-verified:

  • node --test tests/runtime.test.mjs → 20/20 pass
  • node --test tests/commands.test.mjs → 26/26 pass
  • node --test tests/*.test.mjs → 46/46 pass
  • node tests/validate-plugin.mjs → plugin metadata ok
  • git diff --check → clean

Copy link
Copy Markdown
Author

Superseded by #6. The consolidated release candidate retains the parser and monitor work, then adds completed-session discovery with --all, ambiguous-object rejection, secure state permissions and cancellation failure handling.

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.

1 participant