review(회차1): cancel_tx slot 제거 — dead-channel shim 회피
회차 1 의 설계 결함 지적 반영. 원래 IngestState 에 cancel_tx: Sender<()> 만 두고 receiver 는 start_ingest 안에서 즉시 drop — 실제 send() 호출 시 항상 Err(SendError) 인 dead channel 이 됨. \"slot 만 정의\" 의도였으나 실용 가치 0 + CLAUDE.md 의 backward- compat shim 금지 룰 위반. 수정: - IngestState 에서 cancel_tx field 제거. - start_ingest 의 cancel channel allocation 제거. - doc comment 갱신 — p9-fb-04 가 (cancel_tx, cancel_rx) pair 동시 추가 + receiver 를 worker thread 로 move 하는 형태로 reshape 한다고 명시. - test fresh_state helper 도 cancel_tx 인자 제거. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -177,8 +177,14 @@ impl Default for InspectState {
|
|||||||
/// the final aggregate counts stay on screen for a few seconds and
|
/// the final aggregate counts stay on screen for a few seconds and
|
||||||
/// then the slot clears.
|
/// then the slot clears.
|
||||||
///
|
///
|
||||||
/// `cancel_tx` is reserved for `p9-fb-04` (Ctrl-C / Esc cancel
|
/// `p9-fb-04` adds the cancel surface — at that point this struct
|
||||||
/// wiring); this task allocates the channel but never sends on it.
|
/// gains a real `(cancel_tx, cancel_rx)` pair (the receiver moved
|
||||||
|
/// into the worker thread alongside the progress sender). We do
|
||||||
|
/// NOT pre-define a `cancel_tx` slot here because doing so without
|
||||||
|
/// a matching receiver-bound worker would yield a dead channel
|
||||||
|
/// (`send` returning `Err(SendError)` forever) — empty slot that
|
||||||
|
/// pretends to be a future-compat shim is worse than no slot
|
||||||
|
/// (CLAUDE.md "backward-compat shim 금지").
|
||||||
pub struct IngestState {
|
pub struct IngestState {
|
||||||
pub rx: std::sync::mpsc::Receiver<kebab_app::IngestEvent>,
|
pub rx: std::sync::mpsc::Receiver<kebab_app::IngestEvent>,
|
||||||
pub counts: kebab_app::AggregateCounts,
|
pub counts: kebab_app::AggregateCounts,
|
||||||
@@ -195,10 +201,6 @@ pub struct IngestState {
|
|||||||
/// Worker thread handle. `take()`n at clear time so the join
|
/// Worker thread handle. `take()`n at clear time so the join
|
||||||
/// happens after the user has had time to read the final line.
|
/// happens after the user has had time to read the final line.
|
||||||
pub thread: Option<std::thread::JoinHandle<anyhow::Result<kebab_core::IngestReport>>>,
|
pub thread: Option<std::thread::JoinHandle<anyhow::Result<kebab_core::IngestReport>>>,
|
||||||
/// Cancel-token slot for `p9-fb-04`. Defined here so that task
|
|
||||||
/// can wire `Esc` / `Ctrl-C` without restructuring `IngestState`.
|
|
||||||
/// This task never sends on it.
|
|
||||||
pub cancel_tx: std::sync::mpsc::Sender<()>,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Seconds the final ingest status line stays on screen after a run
|
/// Seconds the final ingest status line stays on screen after a run
|
||||||
|
|||||||
@@ -10,9 +10,11 @@
|
|||||||
//! seconds (`TERMINAL_LINE_HOLD_SECS`) and then `tick_clear` returns
|
//! seconds (`TERMINAL_LINE_HOLD_SECS`) and then `tick_clear` returns
|
||||||
//! true so the run loop can drop the slot.
|
//! true so the run loop can drop the slot.
|
||||||
//!
|
//!
|
||||||
//! `cancel_tx` is allocated here but never sent on — the cancel
|
//! Cancel surface (Esc / Ctrl-C) lands in `p9-fb-04`; that task
|
||||||
//! wiring (`Esc` / `Ctrl-C`) lands in `p9-fb-04`. The slot exists
|
//! adds a `(cancel_tx, cancel_rx)` pair to `IngestState` and
|
||||||
//! today so this task does not have to reshape `IngestState` later.
|
//! threads the receiver through `kebab_app::ingest_with_config_cancellable`.
|
||||||
|
//! This task does NOT pre-allocate the channel — see the comment on
|
||||||
|
//! `IngestState` for the rationale.
|
||||||
|
|
||||||
use std::sync::mpsc;
|
use std::sync::mpsc;
|
||||||
use std::thread;
|
use std::thread;
|
||||||
@@ -37,11 +39,6 @@ pub fn start_ingest(app: &mut App) -> anyhow::Result<()> {
|
|||||||
exclude: cfg.workspace.exclude.clone(),
|
exclude: cfg.workspace.exclude.clone(),
|
||||||
};
|
};
|
||||||
let (tx, rx) = mpsc::channel::<IngestEvent>();
|
let (tx, rx) = mpsc::channel::<IngestEvent>();
|
||||||
let (cancel_tx, _cancel_rx) = mpsc::channel::<()>();
|
|
||||||
// _cancel_rx is intentionally dropped here; p9-fb-04 will wire it
|
|
||||||
// through to `ingest_with_config_cancellable` (or equivalent).
|
|
||||||
// Holding both ends today keeps the channel allocated without
|
|
||||||
// doing anything with it.
|
|
||||||
let cfg_for_thread = cfg;
|
let cfg_for_thread = cfg;
|
||||||
let thread = thread::spawn(move || {
|
let thread = thread::spawn(move || {
|
||||||
kebab_app::ingest_with_config_progress(cfg_for_thread, scope, true, Some(tx))
|
kebab_app::ingest_with_config_progress(cfg_for_thread, scope, true, Some(tx))
|
||||||
@@ -55,7 +52,6 @@ pub fn start_ingest(app: &mut App) -> anyhow::Result<()> {
|
|||||||
terminal_at: None,
|
terminal_at: None,
|
||||||
aborted: false,
|
aborted: false,
|
||||||
thread: Some(thread),
|
thread: Some(thread),
|
||||||
cancel_tx,
|
|
||||||
});
|
});
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
@@ -196,7 +192,6 @@ mod tests {
|
|||||||
|
|
||||||
fn fresh_state() -> IngestState {
|
fn fresh_state() -> IngestState {
|
||||||
let (_tx, rx) = mpsc::channel::<IngestEvent>();
|
let (_tx, rx) = mpsc::channel::<IngestEvent>();
|
||||||
let (cancel_tx, _cancel_rx) = mpsc::channel::<()>();
|
|
||||||
IngestState {
|
IngestState {
|
||||||
rx,
|
rx,
|
||||||
counts: AggregateCounts::default(),
|
counts: AggregateCounts::default(),
|
||||||
@@ -206,7 +201,6 @@ mod tests {
|
|||||||
terminal_at: None,
|
terminal_at: None,
|
||||||
aborted: false,
|
aborted: false,
|
||||||
thread: None,
|
thread: None,
|
||||||
cancel_tx,
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user