Skip to content

Harden supervisor evidence and loop stops#57

Merged
phroi merged 3 commits into
masterfrom
review/supervisor-evidence-loop
May 29, 2026
Merged

Harden supervisor evidence and loop stops#57
phroi merged 3 commits into
masterfrom
review/supervisor-evidence-loop

Conversation

@phroi
Copy link
Copy Markdown
Member

@phroi phroi commented May 29, 2026

Why

Supervisor artifacts drive live validation and stop decisions. Tx-hash evidence can appear in top-level records and nested error/skip fields, and loop stop reasons need clear operator semantics.

Changes

  • Preserve accepted tx hashes for generic early command failures while rejecting conflicting hashes within one evidence record.
  • Keep actor classifications scoped to the selected/latest relevant record so stale hashes do not inflate summaries.
  • Make loop and dynamic-loop child help passthrough visible, reject misplaced loop-owned flags, and return inspection-required exit code 3 for max-runs/stable-no-progress stops.
  • Document loop help passthrough and exit-code semantics.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the supervisor loop and dynamic loop to handle child help passthroughs, restrict loop-owned options from being passed after --, and preserve specific exit codes (such as exit code 3 for inspection-required stops). It also refines transaction hash extraction and validation during classification to catch mismatched hashes as malformed evidence. Feedback suggests refactoring spawnSupervisorHelp to use 'pipe' for stdio and capture output safely, preventing potential Node.js runtime crashes when custom streams are passed to spawnSync in test environments.

Comment thread scripts/ickb-supervisor-loop.mjs
Comment thread scripts/ickb-supervisor-loop.mjs Outdated
Comment thread scripts/ickb-supervisor-loop.test.mjs
@phroi
Copy link
Copy Markdown
Member Author

phroi commented May 29, 2026

/gemini review

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces stricter validation of transaction hashes in actor evidence, ensuring that mismatched transaction hashes within a single record are rejected as malformed evidence. It also updates the supervisor loop and dynamic loop scripts to properly handle child help passthroughs ('--help' and '-h'), prevent misplaced loop-owned options after the '--' separator, and preserve the inspection-required exit code (3) when stopping due to 'max_runs' or 'stable_no_progress'. Extensive unit tests have been added to verify these behaviors. There are no review comments, so I have no feedback to provide.

@phroi
Copy link
Copy Markdown
Member Author

phroi commented May 29, 2026

LGTM

Phroi %131

@phroi phroi merged commit 3fb92b1 into master May 29, 2026
1 check passed
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