review(회차1): SIGNAL_COUNT lifetime 명시 + cancel-mid race 코멘트
회차 1 actionable 2건 반영 + 1건 (CLI Ctrl-C integration test) 은 본 PR 에서 별도 task 로 미룸 (signal handler subprocess test 의 flaky 위험 + facade 3 PASS + tui lib 3 PASS 가 안정 surface). - cancel.rs::install_sigint_cancel: SIGNAL_COUNT 위에 process-lifetime invariant 코멘트 — multi-install 차단 (ctrlc::set_handler) 덕분에 reset 불필요. 미래 다중 caller 가 같은 cancel token 공유하려면 install 함수 분리 필요. - ingest_cancel.rs::cancel_mid_loop: redundant `report.new == 1 || 0 || 2` 제거, race timing 의도 코멘트로 대체 (0=listener 승, 1=first only, 2=extra slipped in 모두 valid; 3 = cancel never propagated = 유일한 fail). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -80,11 +80,12 @@ fn cancel_mid_loop_after_first_asset_keeps_idempotent_resume() {
|
||||
let report = run_with(&env, cancel, Some(tx));
|
||||
listener.join().unwrap();
|
||||
|
||||
// Exactly 1 asset committed; remaining 2 skipped (untouched).
|
||||
assert!(
|
||||
report.new == 1 || report.new == 0 || report.new == 2,
|
||||
"non-deterministic but must be < 3: {report:?}"
|
||||
);
|
||||
// cancel-mid is timing-dependent: the listener flips cancel
|
||||
// after the first AssetFinished, but the loop may have started
|
||||
// 1 more asset by the time the next iteration check runs.
|
||||
// 0 (race won by listener), 1 (first only), or 2 (one extra
|
||||
// slipped in) are all valid outcomes; report.new == 3 means
|
||||
// cancel never propagated and is the only failure mode.
|
||||
assert!(report.new < 3, "loop should have broken: {report:?}");
|
||||
|
||||
// Idempotent re-ingest finishes the job.
|
||||
|
||||
@@ -38,6 +38,14 @@ pub fn install_sigint_cancel() -> anyhow::Result<Arc<AtomicBool>> {
|
||||
let cancel_for_handler = cancel.clone();
|
||||
// Per-process count of received SIGINTs. Static so the closure
|
||||
// owns no extra state; first signal flips cancel, second exits.
|
||||
//
|
||||
// Process-lifetime: never reset. ctrlc::set_handler rejects
|
||||
// multi-install with `Err(MultipleHandlers)`, so this counter
|
||||
// is effectively single-use per `kebab` invocation. A future
|
||||
// command that needs its own cancel token (e.g. `kebab eval
|
||||
// run --with-cancel`) must factor the install path into a
|
||||
// helper that takes the token as an arg and shares it across
|
||||
// callers — not call `install_sigint_cancel` twice.
|
||||
static SIGNAL_COUNT: AtomicU8 = AtomicU8::new(0);
|
||||
ctrlc::set_handler(move || {
|
||||
let prev = SIGNAL_COUNT.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
Reference in New Issue
Block a user