fix: stabilize annotation CLI integration#21
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af34db97d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if process.poll() is not None: | ||
| return |
There was a problem hiding this comment.
Avoid returning before killing the process group
When cancellation happens after the process-group leader has exited but its descendants are still alive (for example a helper that spawned digested-scan workers which keep stdout open), this early return skips os.killpg(...) even though run_streaming_subprocess starts the command in a new session. In that state the CLI can report cancellation while orphaned worker processes continue running; for process_group=True, attempt to terminate the group even if poll() says the direct child is already gone.
Useful? React with 👍 / 👎.
Summary
Root cause
The integration layer previously treated MetaUmbra digest directories like repeated argv values even though MetaUmbra accepts one delimited value. Selection inference, metadata, config-relative paths, output collisions, and subprocess cancellation were also handled in separate places, which allowed contradictory or incomplete behavior.
Compatibility and impact
Validation
117 passed, 2 warningsin the focused annotation/integration suite324 passed, 31 warningsin the complete local test suitegit diff --checkpassed