OPE46-00035: OPE46-00035: QoL: Add kill by name pattern - #11
Conversation
|
[REVIEW] GO — #11 @ 7beed8f — lens: correctness+security+gates, reviewer Augustus (1 of 1) What I ran, with exit codes:
What I read:
Blocking P0/P1 findings:
Non-blocking follow-ups:
|
|
Merge disposition after GO review:
PR remains open. The branch needs to be updated onto current |
…ld refactor Merge origin/main (be8cb6d) into drain/ope46-00035-ms66bcvw. One conflicted file, src/cli/index.ts, in three regions; both sides kept in all three. - imports: main added the SystemSnapshot type import while this branch added createInterface and LocalCollector. Kept all three. - helpers: main added parsePercentThresholdOption/parseLoadThresholdOption alongside this branch's confirmProcessKills. Purely additive on both sides. - monitor kill: this branch made the pid argument optional and added --name <regex>, --yes and --json; main replaced the inline `opts.machine ?? "local"` with resolveMachineId(), which resolves config aliases and rejects unknown machines. Kept this branch's optional-pid structure and adopted resolveMachineId, moved below the fail() helper and wrapped so an unknown machine still surfaces as a JSON error on the --json path rather than an unhandled throw. resolveMachineId rejects earlier and with different wording than getCollectorForMachine did, so the assertion in kill.test.ts for the unknown-machine case now expects main's vocabulary ('Unknown machine or alias "missing-machine"'). The test's intent — status 1 plus a JSON error naming the machine — is unchanged. Agent: vibius
|
[REBASED] #11 — GO INVALIDATED, needs re-review @ afcba86 The prior
Conflicts and how each was resolvedOne conflicted file,
One test assertion changed, and it is a consequence of resolution 3. Verification — and an honest account of the redMeasured unpiped ( The suite does not come back green, and that red is pre-existing rather than caused by this merge. I did not assert that — I measured it against a control:
So: Worktree: Agent: vibius |
|
Follow-up to the CI on this head is green. So there are now two independent lines of evidence that the local failures were contention and not this merge: the For completeness: None of this changes the substance of the comment above: the GO at Agent: vibius |
|
[REVIEW] GO — #11 @ afcba86 — lens: correctness+security+gates, reviewer Augustus (1 of 1) What I ran:
What I read:
Blocking P0/P1 findings:
Non-blocking follow-ups:
|
|
[MERGE] blocked — #11 @ afcba86 The review verdict is GO and local gates were green, but the merge did not land. I refreshed Merge command run:
Failure:
Read-only conflict probe:
Required next step before landing:
|
|
[REVIEW] GO — #11 @ afcba86 — lens: correctness+security+gates, reviewer Augustus (1 of 1) What I ran, with exit codes:
What I read:
Blocking P0/P1 findings: none. The current head preserves PID validation and rate limiting through ProcessManager.kill, forwards remote name-match kills through the collector runCommand contract instead of the previous private SSH cast, handles local machine aliases for name-match kills, rejects missing/invalid name patterns, confirms multi-match destructive action unless Non-blocking follow-ups:
|
|
[MERGE] blocked — #11 @ afcba86 Review verdict is GO and local gates were green, but the requested merge did not land. Merge command run:
Failure:
Current PR state from
Required next step before landing:
|
|
[REVIEW] GO — #11 @ afcba86 — lens: correctness+security+gates, reviewer Augustus (1 of 1) What I actually ran, with exit codes:
What I read:
Blocking P0/P1 findings:
Non-blocking follow-ups:
|
|
[MERGE] blocked — #11 @ afcba86 Review verdict is GO and the requested local gates were green, but the requested merge did not land. Merge command run:
Failure:
Current PR state from
Read-only conflict probe:
Required next step before landing:
|
Refuse oversized name-based kill batches before sending any signal when the process-manager rate limit cannot accept the full matched set. Agent: Augustus
|
[REVIEW] NO_GO — #11 @ afcba86 — lens: correctness+security+gates, reviewer Augustus (1 of 1) What I ran, with exit codes:
What I read:
Blocking P0/P1 findings:
Non-blocking follow-ups:
Disposition:
|
|
[REVIEW] GO — #11 @ 7db28e5 — lens: correctness+security+gates, reviewer Augustus (1 of 1) Focused re-review scope:
What I ran, with exit codes:
Blocking P0/P1 findings:
Non-blocking follow-ups:
|
Resolve the current main conflict in the CLI import block while preserving config/compare commands and kill-by-name support. Agent: Augustus
|
[REVIEW] GO — #11 @ 6d8701d — lens: correctness+security+gates, reviewer Augustus (1 of 1) What I ran, with exit codes:
What I read:
Blocking P0/P1 findings:
Non-blocking follow-ups:
|
|
[MERGE] merged — #11 @ 6d8701d Merge command:
Pre-merge gates and guards:
Post-merge verification:
|
1 participant
Automated task drain for
OPE46-00035.Task: OPE46-00035: QoL: Add kill by name pattern
Agent verdict: DONE
Added regex name/command process selection with dry-run JSON previews, multi-match confirmation/--yes, completions, and offline CLI tests.
Independent review: APPROVE
The submitted implementation had defects, all fixed during review:
monitor kill --name worker --machine local-alias --json: it reported askippedaction and exited 0 without killing the match. The CLI now distinguishes local collectors from remote collectors and forwards remote execution through Collector.runCommand (src/process-manager/index.ts:285).monitor kill --name worker --machine missing-machine --jsonpreviously threw outside the strThe reviewer made corrections, committed separately.
Opened by the e2b-task-drain fleet (codex gpt-5.6-sol, effort xhigh).
Not auto-merged — review before landing.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.