From 05550c77a5294a3600f4e7c9156c04d397613873 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Tue, 20 Oct 2026 05:46:16 -0400 Subject: [PATCH 1/2] Name every session by a typed session kind that owns its log token, stage and --session rule One module, src/session/kind.rs, now defines every kind of session thirdshift runs, writes and reads its Session-log token, gives the Command record stage it enters through an exhaustive match, and says which --session words select it. Callers pass the typed kind to the session runner; below it the token stays a plain label, so log names and Model evidence are unchanged. Co-Authored-By: Claude Opus 5.5 --- docs/code-navigation.md | 2 +- src/delivery.rs | 30 ++-- src/delivery/repair_loop.rs | 28 ++-- src/logs.rs | 29 +--- src/logs/reading/selection.rs | 8 +- src/pass.rs | 12 +- src/security/audit.rs | 4 +- src/security/fixing.rs | 5 +- src/security/reproduction.rs | 4 +- src/security/review.rs | 4 +- src/session.rs | 107 +++++++++----- src/session/execution_tests.rs | 12 +- src/session/kind.rs | 246 +++++++++++++++++++++++++++++++++ src/spec_run.rs | 6 +- 14 files changed, 379 insertions(+), 118 deletions(-) create mode 100644 src/session/kind.rs diff --git a/docs/code-navigation.md b/docs/code-navigation.md index 8bd6ad04..53f1a6c0 100644 --- a/docs/code-navigation.md +++ b/docs/code-navigation.md @@ -18,7 +18,7 @@ Start with the issue's terms in `GLOSSARY.md`, then use this index to find their | Harness, Model, Effort | `src/harness.rs` and `src/harness/` | `tests/harness_choice.rs`, `tests/harness.rs` | | Session prompt, Factory skills | `src/prompt.rs`; `src/skills.rs`; `skills/` | `src/skills/tests.rs`, `tests/packaging.rs` | | Trusted account, the issue a session gets | `src/trust.rs` — `issue_trust`, `ticket_trust`, `untrusted_edit` over `Reads`; the GitHub adapter's trust reads, `src/github/trust.rs`; `src/github/session_issue.rs` — `issue_for_session`; `src/delivery.rs` — `hand_over_issue` | Inline tests, including `src/trust.rs` and `src/prompt.rs`; `tests/fresh_run.rs`, `tests/pickup_run.rs` | -| Session log, Resume | `src/session.rs` and `src/session/` | `src/session/execution_tests.rs`, `tests/background_work.rs` | +| Session log, Resume | `src/session.rs` and `src/session/`; `src/session/kind.rs` — `Kind`, `Logged`: every session kind, its Session-log token, the Command record stage it enters and its `--session` rule (`selects`) | `src/session/execution_tests.rs`, inline tests in `src/session/kind.rs`, `tests/background_work.rs` | | Command log, Activity log | `src/logs.rs`; `src/logs/recording.rs` — `Record` | `src/logs/recording/tests.rs`, `tests/command_logs.rs`, `tests/activity_log.rs` | | Command-log allocation, Session naming, child inheritance and filename decoding | `src/logs/owner.rs` — `allocate`, `Owner`, `decode`, `session_matches`; `src/session/recording.rs` — captured Model recording | `src/logs/owner/tests.rs`, `src/logs/recording/tests.rs`, `src/session/recording/tests.rs`, `tests/command_logs.rs`, `tests/status.rs`, `tests/show_logs.rs` | | Command record, status | `src/logs/recording.rs`; `src/logs/command_record.rs`; `src/logs/reading.rs` — `Snapshot`, exact-parent trees and observations; `src/logs/reading/history.rs` — historical recovery; `src/logs/status.rs` — formatting; `src/logs/process_identity.rs` | `src/logs/reading/tests.rs`, `tests/status.rs`, `src/logs/recording/tests.rs` | diff --git a/src/delivery.rs b/src/delivery.rs index 6a81ed39..c0391844 100644 --- a/src/delivery.rs +++ b/src/delivery.rs @@ -36,7 +36,7 @@ use crate::recovery::{self, Preservation, Recovery}; use crate::run::{Goal, Reached}; use crate::run_ending::Cause; use crate::security::review::{Hold as SecurityHold, Outcome as SecurityOutcome, Review}; -use crate::session::{Logs, Sessions}; +use crate::session::{Kind, Logs, Sessions}; use crate::skills; use crate::trust::{self, IssueText}; use crate::worktree::{ForeignCommits, Merge, PendingMerge, Preserved, Worktree}; @@ -81,11 +81,11 @@ pub enum OpeningSession { } impl OpeningSession { - /// The session's kind, in its progress lines and log name. - pub fn kind(self) -> &'static str { + /// The session's kind. + pub fn kind(self) -> Kind { match self { - Self::Implementation => "implement", - Self::SpecReview => "spec-review", + Self::Implementation => Kind::Implementation, + Self::SpecReview => Kind::SpecReview, } } @@ -415,8 +415,8 @@ trait Outside { fn stage(&mut self, stage: Stage); /// Fast-forward the Issue branch to its head on origin. fn catch_up(&mut self) -> Result<()>; - /// Run the session `kind` given `prompt`. - fn session(&mut self, kind: &str, prompt: &str) -> Result<()>; + /// Run a session of `kind` given `prompt`. + fn session(&mut self, kind: Kind, prompt: &str) -> Result<()>; fn security_review(&mut self, prompt: &str) -> Result; fn record_security_review(&mut self, review: &SecurityOutcome) -> Result<()>; /// Push the Issue branch. @@ -524,7 +524,7 @@ impl Outside for InWorktree<'_> { self.worktree.fast_forward_to_origin() } - fn session(&mut self, kind: &str, prompt: &str) -> Result<()> { + fn session(&mut self, kind: Kind, prompt: &str) -> Result<()> { self.sessions.run(kind, prompt)?; self.pull_request.ensure_actionable(false) } @@ -705,7 +705,7 @@ impl repair_loop::Outside for RunOutside<'_> { }) } - fn repair(&mut self, kind: &str, repair: Repair) -> Result<()> { + fn repair(&mut self, kind: Kind, repair: Repair) -> Result<()> { let (issue, base, pr_url) = (self.issue, self.base, self.pr_url); let branch = self.worktree.branch(); let prompt = match repair { @@ -781,7 +781,7 @@ mod tests { #[derive(Debug, PartialEq, Eq)] enum Call { CatchUp, - Session(String), + Session(Kind), Push, Annotation, Checklist, @@ -841,9 +841,9 @@ mod tests { self.calls.push(Call::CatchUp); self.check(Fails::CatchUp) } - fn session(&mut self, kind: &str, _: &str) -> Result<()> { - self.calls.push(Call::Session(kind.to_string())); - self.log = Some(PathBuf::from(format!("/logs/{kind}.jsonl"))); + fn session(&mut self, kind: Kind, _: &str) -> Result<()> { + self.calls.push(Call::Session(kind)); + self.log = Some(PathBuf::from(format!("/logs/{}.jsonl", kind.token()))); self.check(Fails::Session) } fn push(&mut self) -> Result<()> { @@ -969,7 +969,7 @@ mod tests { outside.calls, [ Call::CatchUp, - Call::Session("spec-review".to_string()), + Call::Session(Kind::SpecReview), Call::Push, Call::Annotation, Call::Checklist, @@ -985,7 +985,7 @@ mod tests { assert!(deliver(&mut outside, Goal::ReadyForReview, true).is_ok()); assert_eq!( outside.calls[..2], - [Call::CatchUp, Call::Session("spec-review".to_string())] + [Call::CatchUp, Call::Session(Kind::SpecReview)] ); } diff --git a/src/delivery/repair_loop.rs b/src/delivery/repair_loop.rs index aa8320bd..dc4e4cf4 100644 --- a/src/delivery/repair_loop.rs +++ b/src/delivery/repair_loop.rs @@ -17,6 +17,7 @@ use crate::run::Goal; use crate::worktree::{ForeignCommits, Merge}; use super::Stage; +use crate::session::Kind; /// Which upstream a conflict Repair resolves a merge of. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -79,8 +80,9 @@ pub(super) trait Outside { failed: &FailedChecks, ) -> Result>; - /// Start the Repair session `kind`, as in `repair-1`, for `repair`. - fn repair(&mut self, kind: &str, repair: Repair) -> Result<()>; + /// Start the Repair session of `kind`, as in `Kind::Repair(1)`, for + /// `repair`. + fn repair(&mut self, kind: Kind, repair: Repair) -> Result<()>; /// Whether the Run compares its red checks with the Base branch's, so /// that some may be Inherited failures. @@ -173,15 +175,15 @@ struct Budgets { } impl Budgets { - /// Counts the Repair about to start, returning its kind, `repair-`, - /// and its progress line, or fails if it would be one too many. - fn next_repair(&mut self, cause: &str) -> Result<(String, String)> { + /// Counts the Repair about to start, returning its numbered kind and its + /// progress line, or fails if it would be one too many. + fn next_repair(&mut self, cause: &str) -> Result<(Kind, String)> { if self.repairs == MAX_REPAIRS { bail!("repairs exhausted: {cause}"); } self.repairs += 1; let line = format!("{cause}; starting Repair {} of {MAX_REPAIRS}", self.repairs); - Ok((format!("repair-{}", self.repairs), line)) + Ok((Kind::Repair(self.repairs), line)) } /// Counts a round taken because `upstream` moved, or fails if it would @@ -283,7 +285,7 @@ impl RepairLoop<'_, O> { let (kind, line) = self.budgets.next_repair(cause)?; self.outside.progress(line); self.outside.stage(Stage::RepairSession); - self.outside.repair(&kind, repair) + self.outside.repair(kind, repair) } /// Count a round taken because the Base branch moved `when`. @@ -457,7 +459,7 @@ mod tests { /// Gave this head its Check re-run of these checks of its own. Rerun(String, Option, Vec), /// Started the Repair session of this kind for this Repair. - Repair(String, Started), + Repair(Kind, Started), /// Took the Base fix for these Inherited failures, at this Base /// branch commit. BaseFix(String, Vec), @@ -677,7 +679,7 @@ mod tests { Ok(is.map(|is| is.ci())) } - fn repair(&mut self, kind: &str, repair: Repair) -> Result<()> { + fn repair(&mut self, kind: Kind, repair: Repair) -> Result<()> { let started = match repair { Repair::Conflict(upstream) => Started::Conflict(upstream), Repair::CiFix(failed) => { @@ -685,7 +687,7 @@ mod tests { } Repair::Review { from } => Started::Review(from.to_string()), }; - self.did.push(Did::Repair(kind.to_string(), started)); + self.did.push(Did::Repair(kind, started)); match self.script.repair_error { Some(error) => bail!(error), None => Ok(()), @@ -773,9 +775,9 @@ mod tests { Did::Watch(head.to_string(), Some("b1".to_string())) } - /// The Repair session `repair-` for `started`. + /// The numbered Repair session `n` for `started`. fn repair(n: usize, started: Started) -> Did { - Did::Repair(format!("repair-{n}"), started) + Did::Repair(Kind::Repair(n), started) } /// A CI-fix Repair given the branch's own failures `own`, and the @@ -2044,7 +2046,7 @@ mod tests { assert_eq!( budgets.next_repair(cause).unwrap(), ( - format!("repair-{n}"), + Kind::Repair(n), format!("{cause}; starting Repair {n} of {MAX_REPAIRS}") ) ); diff --git a/src/logs.rs b/src/logs.rs index 6d88ab1a..03e6ed08 100644 --- a/src/logs.rs +++ b/src/logs.rs @@ -203,31 +203,10 @@ pub fn stage(stage: Stage) { record().stage(stage, None); } -/// Keep the current Session log, including a Resume's own log. -pub fn session(kind: &str, path: &Path) { - let original = kind.strip_suffix("-resume").unwrap_or(kind); - let stage = if original == "architecture-review" { - Stage::ArchitectureReview - } else if original == "security-audit" { - Stage::SecurityAudit - } else if original.starts_with("security-reproduction-") { - Stage::SecurityReproduction - } else if original == "security-fix-publishing" { - Stage::SecurityFixPublishing - } else if kind.ends_with("-resume") { - Stage::Resume - } else if kind.starts_with("repair-") { - Stage::Repair - } else if kind == "security-review" { - Stage::SecurityReview - } else if kind == "implement" { - Stage::Implement - } else if kind == "spec-review" { - Stage::SpecReview - } else { - return; - }; - record().stage(stage, Some(path)); +/// Keep the current Session log, including a Resume's own log, as +/// `session` enters its stage. +pub fn session(session: crate::session::Logged, path: &Path) { + record().stage(session.stage(), Some(path)); } /// The selected Issue branch, retained unless completion says it was removed. diff --git a/src/logs/reading/selection.rs b/src/logs/reading/selection.rs index f43f242d..54b0c499 100644 --- a/src/logs/reading/selection.rs +++ b/src/logs/reading/selection.rs @@ -190,13 +190,7 @@ impl<'a> Observation<'a> { else { return false; }; - let original = session.strip_suffix("-resume").unwrap_or(session); - session == kind - || original == kind - || (kind == "repair" - && original - .strip_prefix("repair-") - .is_some_and(|number| number.parse::().is_ok())) + crate::session::kind::selects(kind, session) }) .with_context(|| format!("no {kind} Session log for this Command")) } diff --git a/src/pass.rs b/src/pass.rs index 141e4cd9..611f0ef3 100644 --- a/src/pass.rs +++ b/src/pass.rs @@ -30,13 +30,9 @@ use crate::logs::{self, Pass, Work}; use crate::progress; use crate::ready::{self, ReadyIssue}; use crate::run::{self, Ended, StartedBy}; -use crate::session::{Logs, Purpose, Sessions}; +use crate::session::{Kind, Logs, Purpose, Sessions}; use crate::worktree::ReviewWorktree; -/// The Architecture review session's kind, in its progress lines and log -/// name. -const REVIEW: &str = "architecture-review"; - /// What a pass dispatches, as `thirdshift ` would run it, but on /// the pass's Base branch `base`, whatever the Launch directory has checked /// out. @@ -222,7 +218,11 @@ impl Outside for LaunchAndGitHub<'_> { let harness = self.harness.clone(); let concluded = Sessions::within(&logs, worktree.path(), &harness, |sessions| { progress::step(starting); - let final_message = sessions.run_to_final_message(Purpose::Ordinary, REVIEW, prompt)?; + let final_message = sessions.run_to_final_message( + Purpose::Ordinary, + Kind::ArchitectureReview, + prompt, + )?; conclude(self, final_message.as_deref()) }); // Removed once the review is concluded. diff --git a/src/security/audit.rs b/src/security/audit.rs index 4cce6eb3..7276e2c1 100644 --- a/src/security/audit.rs +++ b/src/security/audit.rs @@ -16,7 +16,7 @@ use crate::logs; use crate::process::{self, Control, Interruption}; use crate::progress; use crate::prompt; -use crate::session::{Logs, Purpose, Sessions}; +use crate::session::{Kind, Logs, Purpose, Sessions}; use crate::skills; use crate::worktree::ReviewWorktree; @@ -121,7 +121,7 @@ pub fn run(launch: &Git, repo: &Repo, base: &str, harness: &Choice) -> Attempt { progress::step(format!("starting the Security audit of {base}")); started = true; let message = - sessions.run_to_final_message(Purpose::Security, "security-audit", &prompt)?; + sessions.run_to_final_message(Purpose::Security, Kind::SecurityAudit, &prompt)?; let line = message.as_deref().and_then(|message| { message .lines() diff --git a/src/security/fixing.rs b/src/security/fixing.rs index ae0b05f4..3914a78c 100644 --- a/src/security/fixing.rs +++ b/src/security/fixing.rs @@ -12,7 +12,7 @@ use crate::issue::{IssueUrl, Repo}; use crate::labels::Label; use crate::prompt; use crate::security::reproduction::FixSize; -use crate::session::{Logs, Purpose, Sessions}; +use crate::session::{Kind, Logs, Purpose, Sessions}; use crate::worktree::ReviewWorktree; mod publication; @@ -58,7 +58,8 @@ pub fn publish( } else { Purpose::Security }; - let message = sessions.run_to_final_message(purpose, "security-fix-publishing", &prompt)?; + let message = + sessions.run_to_final_message(purpose, Kind::SecurityFixPublishing, &prompt)?; publication::publish( &mut GitHub::new(), repo, diff --git a/src/security/reproduction.rs b/src/security/reproduction.rs index d3606bd6..51880952 100644 --- a/src/security/reproduction.rs +++ b/src/security/reproduction.rs @@ -14,7 +14,7 @@ use crate::harness::Choice; use crate::issue::Repo; use crate::logs; use crate::prompt; -use crate::session::{Logs, Purpose, Sessions}; +use crate::session::{Kind, Logs, Purpose, Sessions}; use crate::worktree::ReviewWorktree; // Declaration order is severity order, most severe first. @@ -124,7 +124,7 @@ pub fn run( Sessions::within(&logs, worktree.path(), harness, |sessions| { let message = sessions.run_to_final_message( Purpose::Security, - &format!("security-reproduction-{number}"), + Kind::SecurityReproduction(number), &prompt, )?; let message = message.as_deref().unwrap_or_default().trim_end(); diff --git a/src/security/review.rs b/src/security/review.rs index be367baa..596a7bc3 100644 --- a/src/security/review.rs +++ b/src/security/review.rs @@ -9,7 +9,7 @@ use super::reproduction::{FixSize, Outcome as ReproductionOutcome, Reproduction, use crate::github::{DraftAdvisory, FindingDraft, FindingProvenance, GitHub}; use crate::harness::interpretation::SafeguardRefusal; use crate::issue::IssueUrl; -use crate::session::{Purpose, Sessions}; +use crate::session::{Kind, Purpose, Sessions}; use crate::worktree::Worktree; use crate::{logs, progress, prompt}; @@ -41,7 +41,7 @@ pub(crate) fn run( report.display() )); let message = - sessions.run_to_final_message(Purpose::SecurityReview, "security-review", &text)?; + sessions.run_to_final_message(Purpose::SecurityReview, Kind::SecurityReview, &text)?; // A session may change commits on the Issue branch, but it cannot hand // us a replacement checkout to observe or publish findings from. worktree.head()?; diff --git a/src/session.rs b/src/session.rs index c89484c2..1473f252 100644 --- a/src/session.rs +++ b/src/session.rs @@ -24,10 +24,13 @@ use crate::progress; use crate::prompt; use crate::skills; +pub mod kind; pub mod recording; mod review_reports; use review_reports::ReviewReports; +pub use kind::{Kind, Logged}; + #[cfg(test)] mod execution_tests; @@ -88,8 +91,9 @@ impl Logs { /// Where a session's stream is logged: /// `/commands//-~-.jsonl`. - pub fn path(&self, kind: &str) -> Result { - self.recording().session_path(&self.dir, &self.name, kind) + pub fn path(&self, session: Logged) -> Result { + self.recording() + .session_path(&self.dir, &self.name, &session.token()) } /// Runs construct Logs before work starts. Capture once the first Session @@ -194,13 +198,13 @@ impl<'a> Sessions<'a> { (taken, log) } - /// Run a session given `prompt`, logged and labelled as `kind`. A - /// session that ends with background work still running, which is killed - /// with it, gets one Resume, as `-resume`. If the Resume ends the + /// Run a session of `kind` given `prompt`, logged and labelled by its + /// token. A session that ends with background work still running, which + /// is killed with it, gets one Resume, as `-resume`. If the Resume ends the /// same way, or the session has no id to resume, the work may have been /// abandoned rather than awaited: a progress line names it and this /// succeeds, leaving the steps that follow to decide the outcome. - pub fn run(&self, kind: &str, prompt: &str) -> Result<()> { + pub fn run(&self, kind: Kind, prompt: &str) -> Result<()> { // The shared review-report contract identifies code review sessions // before expansion. Retain that policy even if report setup fails. let purpose = if prompt.contains(prompt::REVIEW_REPORTS_DIRECTORY) { @@ -214,19 +218,22 @@ impl<'a> Sessions<'a> { /// Run a session as [`Sessions::run`] does, and return its final message: /// what the agent said as it ended its last turn, its Resume's if it got /// one, if anything. `purpose` selects the launch policy for both the - /// session and its Resume; `kind` only labels their logs and progress. + /// session and its Resume; `kind` names their logs, labels their + /// progress and gives their Command record stage. pub fn run_to_final_message( &self, purpose: Purpose, - kind: &str, + kind: Kind, prompt: &str, ) -> Result> { + // Progress lines name the session by its kind's token. + let label = kind.token(); let reports = if prompt.contains(prompt::REVIEW_REPORTS_DIRECTORY) { match ReviewReports::new(self.worktree) { Ok(reports) => Some(reports), Err(error) => { self.step(format!( - "{kind}: could not prepare review reports: {error:#}; carrying on" + "{label}: could not prepare review reports: {error:#}; carrying on" )); None } @@ -234,7 +241,7 @@ impl<'a> Sessions<'a> { } else { None }; - let mut ended = self.start(purpose, kind, None, prompt, reports.as_ref())?; + let mut ended = self.start(purpose, Logged::of(kind), None, prompt, reports.as_ref())?; // What ended last, as the progress line on its killed work calls it. let mut ended_last = "session"; if let (false, Some(session_id)) = (ended.killed.is_empty(), &ended.session_id) { @@ -243,11 +250,11 @@ impl<'a> Sessions<'a> { resume_prompt.push_str(prompt::REVIEW_REPORTS); } self.step(format!( - "{kind}: background work was killed as the session ended; resuming it once" + "{label}: background work was killed as the session ended; resuming it once" )); ended = self.start( purpose, - &format!("{kind}-resume"), + Logged::resume_of(kind), Some(session_id), &resume_prompt, reports.as_ref(), @@ -261,32 +268,33 @@ impl<'a> Sessions<'a> { Purpose::Security | Purpose::SecurityPublishing | Purpose::SecurityReview ) { return Err(anyhow!( - "{kind}: the {ended_last} {ending}; Security session incomplete" + "{label}: the {ended_last} {ending}; Security session incomplete" )); } self.step(format!( - "{kind}: the {ended_last} {ending}; carrying on, as it may have been abandoned" + "{label}: the {ended_last} {ending}; carrying on, as it may have been abandoned" )); self.endings_with_killed_work .borrow_mut() - .push(format!("{kind} session {ending}")); + .push(format!("{label} session {ending}")); } Ok(ended.final_message) } - /// Run one session as `kind`, logged under its own path, which becomes - /// the last log, once `before_each` holds. + /// Run `session`, logged under its own path, which becomes the last log, + /// once `before_each` holds. Below here its token is only a label. fn start( &self, purpose: Purpose, - kind: &str, + session: Logged, resume: Option<&str>, prompt: &str, reports: Option<&ReviewReports>, ) -> Result { (self.before_each)()?; - let log = self.logs.path(kind)?; - logs::session(kind, &log); + let log = self.logs.path(session)?; + logs::session(session, &log); + let kind = &session.token(); self.step(format!("logging the session to {}", log.display())); *self.last_log.borrow_mut() = Some(log.clone()); let prompt = reports.map_or_else( @@ -544,7 +552,7 @@ mod tests { dir: root.path().into(), recording: recording.clone().into(), }; - let log = logs.path("implement").unwrap(); + let log = logs.path(Logged::read("implement").unwrap()).unwrap(); recording.run( "implement", &Choice::default(), Command::new("/bin/sh").args(["-c", &format!( @@ -615,7 +623,7 @@ mod tests { kind: kind.to_string(), resume: resume.map(String::from), prompt: prompt.to_string(), - log: logs().path(kind).unwrap(), + log: logs().path(Logged::read(kind).unwrap()).unwrap(), } } @@ -623,7 +631,7 @@ mod tests { fn logging(kind: &str) -> Call { Call::Step(format!( "logging the session to {}", - logs().path(kind).unwrap().display() + logs().path(Logged::read(kind).unwrap()).unwrap().display() )) } @@ -642,7 +650,7 @@ mod tests { #[test] fn a_clean_session_runs_once_and_returns_its_final_message() { let (taken, log, calls) = take(vec![ended(Some("s1"), &[], "done")], |sessions| { - sessions.run_to_final_message(Purpose::Ordinary, "implement", "do it") + sessions.run_to_final_message(Purpose::Ordinary, Kind::Implementation, "do it") }); assert_eq!(taken.unwrap(), Some("done".to_string())); @@ -650,7 +658,10 @@ mod tests { calls, [logging("implement"), session("implement", None, "do it")] ); - assert_eq!(log, Some(logs().path("implement").unwrap())); + assert_eq!( + log, + Some(logs().path(Logged::read("implement").unwrap()).unwrap()) + ); } #[test] @@ -660,7 +671,9 @@ mod tests { ended(Some("s1"), &["cargo test"], "waiting"), ended(Some("s1"), &[], "done"), ], - |sessions| sessions.run_to_final_message(Purpose::Ordinary, "implement", "do it"), + |sessions| { + sessions.run_to_final_message(Purpose::Ordinary, Kind::Implementation, "do it") + }, ); assert_eq!(taken.unwrap(), Some("done".to_string())); @@ -681,7 +694,14 @@ mod tests { ), ] ); - assert_eq!(log, Some(logs().path("implement-resume").unwrap())); + assert_eq!( + log, + Some( + logs() + .path(Logged::read("implement-resume").unwrap()) + .unwrap() + ) + ); } #[test] @@ -700,7 +720,7 @@ mod tests { )); } let (taken, _, _) = take(endings, |sessions| { - sessions.run_to_final_message(Purpose::Security, "security-review", "review") + sessions.run_to_final_message(Purpose::Security, Kind::SecurityReview, "review") }); assert!( taken.is_err(), @@ -719,7 +739,7 @@ mod tests { |sessions| -> Result<()> { assert_eq!( sessions - .run_to_final_message(Purpose::Ordinary, "implement", "do it") + .run_to_final_message(Purpose::Ordinary, Kind::Implementation, "do it") .unwrap(), Some("still waiting".to_string()) ); @@ -738,14 +758,21 @@ mod tests { format!("{:#}", taken.unwrap_err()), format!("implement session {TESTS_KILLED}, and a later step failed: no PR found") ); - assert_eq!(log, Some(logs().path("implement-resume").unwrap())); + assert_eq!( + log, + Some( + logs() + .path(Logged::read("implement-resume").unwrap()) + .unwrap() + ) + ); } #[test] fn a_session_with_no_id_to_resume_is_not_resumed_and_carries_on() { let (taken, log, calls) = take( vec![ended(None, &["cargo test", "npm test"], "waiting")], - |sessions| sessions.run("implement", "do it"), + |sessions| sessions.run(Kind::Implementation, "do it"), ); taken.unwrap(); @@ -762,13 +789,16 @@ mod tests { ), ] ); - assert_eq!(log, Some(logs().path("implement").unwrap())); + assert_eq!( + log, + Some(logs().path(Logged::read("implement").unwrap()).unwrap()) + ); } #[test] fn a_failing_session_fails_unchanged_without_a_resume_or_an_ending() { let (taken, log, calls) = take(vec![Err(anyhow!("claude exited 3"))], |sessions| { - let failed = sessions.run("implement", "do it").unwrap_err(); + let failed = sessions.run(Kind::Implementation, "do it").unwrap_err(); assert_eq!(format!("{failed:#}"), "claude exited 3"); Err::<(), _>(failed) }); @@ -780,13 +810,16 @@ mod tests { [logging("implement"), session("implement", None, "do it")] ); // The failing session's log is still the last log. - assert_eq!(log, Some(logs().path("implement").unwrap())); + assert_eq!( + log, + Some(logs().path(Logged::read("implement").unwrap()).unwrap()) + ); } #[test] fn steps_that_succeed_after_killed_work_come_to_what_they_came_to() { let (taken, _, _) = take(vec![ended(None, &["cargo test"], "waiting")], |sessions| { - sessions.run("implement", "do it")?; + sessions.run(Kind::Implementation, "do it")?; Ok(42) }); @@ -801,8 +834,8 @@ mod tests { ended(None, &["npm test"], "waiting"), ], |sessions| -> Result<()> { - sessions.run("implement", "do it")?; - sessions.run("repair-1", "fix it")?; + sessions.run(Kind::Implementation, "do it")?; + sessions.run(Kind::Repair(1), "fix it")?; bail!("CI failed") }, ); diff --git a/src/session/execution_tests.rs b/src/session/execution_tests.rs index d6d13ebb..48dd4f0c 100644 --- a/src/session/execution_tests.rs +++ b/src/session/execution_tests.rs @@ -122,7 +122,7 @@ fn execute_session(root: &Path) -> anyhow::Error { }; let prompt = format!("/thirdshift-implement {}", "p".repeat(1024 * 1024)); let (result, log) = Sessions::within(&logs, &worktree, &choice, |sessions| { - sessions.run_to_final_message(Purpose::Ordinary, "implement", &prompt) + sessions.run_to_final_message(Purpose::Ordinary, Kind::Implementation, &prompt) }); assert!(log.unwrap().exists()); result.unwrap_err() @@ -391,7 +391,10 @@ fn with_review_sessions( fn execute_review(root: &Path, harness: Harness) -> (Result<()>, PathBuf) { let issue = IssueUrl::parse("https://github.com/acme/widgets/issues/7").unwrap(); with_review_sessions(root, harness, |sessions| { - sessions.run("implement", &prompt::fresh(&issue, "main", "issue-7"))?; + sessions.run( + Kind::Implementation, + &prompt::fresh(&issue, "main", "issue-7"), + )?; fs::write(root.join("carried-on"), "").unwrap(); Ok(()) }) @@ -537,7 +540,10 @@ fn unavailable_report_setup_warns_and_the_run_carries_on() { let exclude = root.join("worktree/.git/info/exclude"); let original_permissions = fs::metadata(&exclude).unwrap().permissions(); fs::set_permissions(&exclude, fs::Permissions::from_mode(0o444)).unwrap(); - let result = sessions.run("implement", &prompt::fresh(&issue, "main", "issue-7")); + let result = sessions.run( + Kind::Implementation, + &prompt::fresh(&issue, "main", "issue-7"), + ); fs::set_permissions(&exclude, original_permissions).unwrap(); result?; fs::write(root.join("carried-on"), "").unwrap(); diff --git a/src/session/kind.rs b/src/session/kind.rs new file mode 100644 index 00000000..b5ceb980 --- /dev/null +++ b/src/session/kind.rs @@ -0,0 +1,246 @@ +//! Every kind of session thirdshift runs: its Session-log token, the Command +//! record stage it enters, and which `thirdshift logs --session` words +//! select it. + +use crate::logs::Stage; + +/// What a session is run as. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Kind { + /// A Run's implement session, `implement`. + Implementation, + /// A Spec run's Spec review, `spec-review`. + SpecReview, + /// The numbered Repair, `repair-`. + Repair(usize), + /// `security-review`. + SecurityReview, + /// An Architect run's Architecture review, `architecture-review`. + ArchitectureReview, + /// `security-audit`. + SecurityAudit, + /// The Security reproduction of the numbered candidate, + /// `security-reproduction-`. + SecurityReproduction(usize), + /// `security-fix-publishing`. + SecurityFixPublishing, +} + +impl Kind { + /// The kind's token, in its Session log's name and progress lines. + pub fn token(self) -> String { + match self { + Self::Implementation => "implement".to_string(), + Self::SpecReview => "spec-review".to_string(), + Self::Repair(n) => format!("repair-{n}"), + Self::SecurityReview => "security-review".to_string(), + Self::ArchitectureReview => "architecture-review".to_string(), + Self::SecurityAudit => "security-audit".to_string(), + Self::SecurityReproduction(n) => format!("security-reproduction-{n}"), + Self::SecurityFixPublishing => "security-fix-publishing".to_string(), + } + } + + /// The kind `token` names, if any. + fn read(token: &str) -> Option { + let numbered = |prefix: &str| { + token + .strip_prefix(prefix) + .filter(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())) + .and_then(|n| n.parse().ok()) + }; + Some(match token { + "implement" => Self::Implementation, + "spec-review" => Self::SpecReview, + "security-review" => Self::SecurityReview, + "architecture-review" => Self::ArchitectureReview, + "security-audit" => Self::SecurityAudit, + "security-fix-publishing" => Self::SecurityFixPublishing, + _ => { + if let Some(n) = numbered("repair-") { + Self::Repair(n) + } else { + Self::SecurityReproduction(numbered("security-reproduction-")?) + } + } + }) + } +} + +/// A session as its Session log names it: of `kind`, and its Resume if +/// `resume`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Logged { + pub kind: Kind, + pub resume: bool, +} + +impl Logged { + /// The session of `kind` itself, not its Resume. + pub fn of(kind: Kind) -> Self { + Logged { + kind, + resume: false, + } + } + + /// The Resume of the session of `kind`. + pub fn resume_of(kind: Kind) -> Self { + Logged { kind, resume: true } + } + + /// The Session-log token: the kind's, with `-resume` for its Resume. + pub fn token(self) -> String { + let token = self.kind.token(); + if self.resume { + format!("{token}-resume") + } else { + token + } + } + + /// The session a Session log's `token` names, if thirdshift can read it. + pub fn read(token: &str) -> Option { + match token.strip_suffix("-resume") { + Some(kind) => Kind::read(kind).map(Self::resume_of), + None => Kind::read(token).map(Self::of), + } + } + + /// The Command record stage the session enters. A Resume of the + /// implement session, Spec review, a Repair or Security review enters + /// `Resume`; any other Resume keeps its kind's stage. + pub fn stage(self) -> Stage { + match (self.kind, self.resume) { + ( + Kind::Implementation | Kind::SpecReview | Kind::Repair(_) | Kind::SecurityReview, + true, + ) => Stage::Resume, + (Kind::Implementation, false) => Stage::Implement, + (Kind::SpecReview, false) => Stage::SpecReview, + (Kind::Repair(_), false) => Stage::Repair, + (Kind::SecurityReview, false) => Stage::SecurityReview, + (Kind::ArchitectureReview, _) => Stage::ArchitectureReview, + (Kind::SecurityAudit, _) => Stage::SecurityAudit, + (Kind::SecurityReproduction(_), _) => Stage::SecurityReproduction, + (Kind::SecurityFixPublishing, _) => Stage::SecurityFixPublishing, + } + } +} + +/// Whether `thirdshift logs --session ` selects the Session log whose +/// token is `token`: one whose token is `word`, whose kind is `word`, Resume +/// or not, or, for `repair`, any numbered Repair or its Resume. +pub fn selects(word: &str, token: &str) -> bool { + token == word + || Logged::read(token).is_some_and(|logged| { + logged.kind.token() == word + || (word == "repair" && matches!(logged.kind, Kind::Repair(_))) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Every kind, the numbered ones with a number of their own. + const KINDS: [(Kind, &str); 8] = [ + (Kind::Implementation, "implement"), + (Kind::SpecReview, "spec-review"), + (Kind::Repair(3), "repair-3"), + (Kind::SecurityReview, "security-review"), + (Kind::ArchitectureReview, "architecture-review"), + (Kind::SecurityAudit, "security-audit"), + (Kind::SecurityReproduction(12), "security-reproduction-12"), + (Kind::SecurityFixPublishing, "security-fix-publishing"), + ]; + + #[test] + fn every_kind_reads_back_from_its_token_and_its_resumes() { + for (kind, token) in KINDS { + assert_eq!(Logged::of(kind).token(), token); + assert_eq!(Logged::read(token), Some(Logged::of(kind))); + let resume = format!("{token}-resume"); + assert_eq!(Logged::resume_of(kind).token(), resume); + assert_eq!(Logged::read(&resume), Some(Logged::resume_of(kind))); + } + } + + #[test] + fn malformed_tokens_read_back_as_no_kind() { + for token in [ + "repair-", + "repair-x", + "repair-+1", + "repair--resume", + "security-reproduction-", + "security-reproduction-x", + "review", + "-resume", + "implement-resume-resume", + "", + ] { + assert_eq!(Logged::read(token), None, "{token}"); + } + } + + #[test] + fn each_session_enters_its_command_record_stage() { + let stages = [ + (Kind::Implementation, "implement", "Resume"), + (Kind::SpecReview, "Spec review", "Resume"), + (Kind::Repair(1), "Repair", "Resume"), + (Kind::SecurityReview, "Security review", "Resume"), + ( + Kind::ArchitectureReview, + "Architecture review", + "Architecture review", + ), + (Kind::SecurityAudit, "Security audit", "Security audit"), + ( + Kind::SecurityReproduction(2), + "Security reproduction", + "Security reproduction", + ), + ( + Kind::SecurityFixPublishing, + "Security fix publishing", + "Security fix publishing", + ), + ]; + for (kind, stage, resume_stage) in stages { + assert_eq!(Logged::of(kind).stage().name(), stage); + assert_eq!(Logged::resume_of(kind).stage().name(), resume_stage); + } + } + + #[test] + fn a_session_word_selects_its_token_its_kind_or_any_repair() { + assert!(selects("implement", "implement")); + assert!(selects("implement", "implement-resume")); + assert!(selects("implement-resume", "implement-resume")); + assert!(!selects("implement-resume", "implement")); + assert!(!selects("implement", "spec-review")); + + assert!(selects("repair-2", "repair-2")); + assert!(selects("repair-2", "repair-2-resume")); + assert!(!selects("repair-2", "repair-1")); + assert!(selects("repair", "repair-1")); + assert!(selects("repair", "repair-2-resume")); + assert!(!selects("repair", "repair-x")); + assert!(!selects("security-reproduction", "security-reproduction-1")); + assert!(selects( + "security-reproduction-1", + "security-reproduction-1-resume" + )); + } + + #[test] + fn an_unreadable_token_is_selected_only_by_itself() { + assert!(selects("review", "review")); + assert!(selects("repair-x", "repair-x")); + assert!(!selects("repair", "repair-x")); + assert!(selects("legacy-resume", "legacy-resume")); + assert!(!selects("legacy", "legacy-resume")); + } +} diff --git a/src/spec_run.rs b/src/spec_run.rs index a62f0198..bfbb1d37 100644 --- a/src/spec_run.rs +++ b/src/spec_run.rs @@ -495,7 +495,7 @@ mod tests { /// Started the Delivery, opening with the session of this kind and /// prompt, caught up from origin first if `catch_up`. Deliver { - kind: String, + kind: crate::session::Kind, prompt: String, catch_up: bool, }, @@ -701,7 +701,7 @@ mod tests { fn deliver(&mut self, opening: Opening, checklist: &str) -> Result { self.did.push(Did::Deliver { - kind: opening.session.kind().to_string(), + kind: opening.session.kind(), catch_up: opening.session.catches_up_from_origin(), prompt: opening.prompt, }); @@ -844,7 +844,7 @@ mod tests { fn spec_review_opening() -> Did { let spec = IssueUrl::parse("https://github.com/acme/widgets/issues/20").unwrap(); Did::Deliver { - kind: "spec-review".to_string(), + kind: crate::session::Kind::SpecReview, prompt: prompt::spec_review(&spec, "main", "issue-20", SPEC_PR), catch_up: true, } From 8c24577cae30d4d4ee8185a6af3746cb956bb0bf Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Tue, 20 Oct 2026 05:58:44 -0400 Subject: [PATCH 2/2] Keep main's --session rules for every token, and read each kind's token in one place Review follow-up: a token no kind reads is still selected without its -resume, and the repair word selects the numbers it always read. The unnumbered kinds' tokens are written once, and labels below the seam are named as labels. Co-Authored-By: Claude Opus 5.5 --- docs/code-navigation.md | 2 +- src/logs.rs | 3 +- src/session.rs | 66 +++++++++++++------------------------ src/session/kind.rs | 72 ++++++++++++++++++++++++----------------- 4 files changed, 69 insertions(+), 74 deletions(-) diff --git a/docs/code-navigation.md b/docs/code-navigation.md index 53f1a6c0..1ea27ce0 100644 --- a/docs/code-navigation.md +++ b/docs/code-navigation.md @@ -22,7 +22,7 @@ Start with the issue's terms in `GLOSSARY.md`, then use this index to find their | Command log, Activity log | `src/logs.rs`; `src/logs/recording.rs` — `Record` | `src/logs/recording/tests.rs`, `tests/command_logs.rs`, `tests/activity_log.rs` | | Command-log allocation, Session naming, child inheritance and filename decoding | `src/logs/owner.rs` — `allocate`, `Owner`, `decode`, `session_matches`; `src/session/recording.rs` — captured Model recording | `src/logs/owner/tests.rs`, `src/logs/recording/tests.rs`, `src/session/recording/tests.rs`, `tests/command_logs.rs`, `tests/status.rs`, `tests/show_logs.rs` | | Command record, status | `src/logs/recording.rs`; `src/logs/command_record.rs`; `src/logs/reading.rs` — `Snapshot`, exact-parent trees and observations; `src/logs/reading/history.rs` — historical recovery; `src/logs/status.rs` — formatting; `src/logs/process_identity.rs` | `src/logs/reading/tests.rs`, `tests/status.rs`, `src/logs/recording/tests.rs` | -| Command naming, Session selection, show, logs | `src/logs/reading/selection.rs` — resolution, captured outer owner and child-refusal target, Session inventory/selection and bounded latest events; `src/logs/status/inspect.rs` — formatting and log streaming | `src/logs/reading/tests.rs`, `tests/show_logs.rs` | +| Command naming, Session selection, show, logs | `src/logs/reading/selection.rs` — resolution, captured outer owner and child-refusal target, Session inventory/selection and bounded latest events, each `--session` word matched by `src/session/kind.rs` — `selects`; `src/logs/status/inspect.rs` — formatting and log streaming | `src/logs/reading/tests.rs`, `tests/show_logs.rs` | | Stop | `src/logs/reading.rs` — `retained_work` reload by captured record; `src/logs/reading/selection.rs` — `Selection::stop_target`; `src/logs/stop.rs` — request, signaling and waiting; `src/claim.rs` — Stop endings | `src/logs/reading/tests.rs`, `tests/stop.rs`; inline Claim and Stop tests | | User config, Setup | `src/config.rs`; `src/setup.rs` | `tests/user_config.rs`, `tests/setup.rs` | | Architecture review, Architect run | `src/architect.rs` | `tests/architect_run.rs` | diff --git a/src/logs.rs b/src/logs.rs index 03e6ed08..70611642 100644 --- a/src/logs.rs +++ b/src/logs.rs @@ -28,6 +28,7 @@ use std::sync::{LazyLock, Mutex, MutexGuard, PoisonError}; use crate::config::UserConfig; use crate::harness::Choice; use crate::issue::{IssueUrl, Repo}; +use crate::session::Logged; use activity::Kind; pub use command_record::StageName as Stage; @@ -205,7 +206,7 @@ pub fn stage(stage: Stage) { /// Keep the current Session log, including a Resume's own log, as /// `session` enters its stage. -pub fn session(session: crate::session::Logged, path: &Path) { +pub fn session(session: Logged, path: &Path) { record().stage(session.stage(), Some(path)); } diff --git a/src/session.rs b/src/session.rs index 1473f252..96b83c83 100644 --- a/src/session.rs +++ b/src/session.rs @@ -294,7 +294,7 @@ impl<'a> Sessions<'a> { (self.before_each)()?; let log = self.logs.path(session)?; logs::session(session, &log); - let kind = &session.token(); + let label = &session.token(); self.step(format!("logging the session to {}", log.display())); *self.last_log.borrow_mut() = Some(log.clone()); let prompt = reports.map_or_else( @@ -304,10 +304,10 @@ impl<'a> Sessions<'a> { let ended = self .outside .borrow_mut() - .run_session(purpose, kind, resume, &prompt, &log); + .run_session(purpose, label, resume, &prompt, &log); if let Some(reports) = reports { for line in reports.keep(&log) { - self.step(format!("{kind}: {line}")); + self.step(format!("{label}: {line}")); } } ended @@ -321,13 +321,14 @@ impl<'a> Sessions<'a> { /// How the Resume rules run a session, and where their progress lines go. trait Outside { - /// Run a session given `prompt`, labelled as `kind`, continuing the - /// session with id `resume`, if any, its stream logged to `log`. Returns - /// what it ended with once it has exited cleanly, its turn not failed. + /// Run a session given `prompt`, labelled by its kind's token `label`, + /// continuing the session with id `resume`, if any, its stream logged to + /// `log`. Returns what it ended with once it has exited cleanly, its turn + /// not failed. fn run_session( &mut self, purpose: Purpose, - kind: &str, + label: &str, resume: Option<&str>, prompt: &str, log: &Path, @@ -349,7 +350,7 @@ impl Outside for OnMachine { fn run_session( &mut self, purpose: Purpose, - kind: &str, + label: &str, resume: Option<&str>, prompt: &str, log: &Path, @@ -400,7 +401,7 @@ impl Outside for OnMachine { None }; let ended = self.recording.run( - kind, + label, &self.harness, &mut command, invocation.stdin.as_deref(), @@ -552,7 +553,7 @@ mod tests { dir: root.path().into(), recording: recording.clone().into(), }; - let log = logs.path(Logged::read("implement").unwrap()).unwrap(); + let log = logs.path(Logged::of(Kind::Implementation)).unwrap(); recording.run( "implement", &Choice::default(), Command::new("/bin/sh").args(["-c", &format!( @@ -617,22 +618,24 @@ mod tests { (taken, log, calls) } + /// Where the session whose token is `token` is logged. + fn log_of(token: &str) -> PathBuf { + logs().path(Logged::read(token).unwrap()).unwrap() + } + /// The call that runs a session as `kind`, logged under its own path. fn session(kind: &str, resume: Option<&str>, prompt: &str) -> Call { Call::Session { kind: kind.to_string(), resume: resume.map(String::from), prompt: prompt.to_string(), - log: logs().path(Logged::read(kind).unwrap()).unwrap(), + log: log_of(kind), } } /// The progress line saying where a session as `kind` is logged. fn logging(kind: &str) -> Call { - Call::Step(format!( - "logging the session to {}", - logs().path(Logged::read(kind).unwrap()).unwrap().display() - )) + Call::Step(format!("logging the session to {}", log_of(kind).display())) } /// The sessions among `calls`. @@ -658,10 +661,7 @@ mod tests { calls, [logging("implement"), session("implement", None, "do it")] ); - assert_eq!( - log, - Some(logs().path(Logged::read("implement").unwrap()).unwrap()) - ); + assert_eq!(log, Some(log_of("implement"))); } #[test] @@ -694,14 +694,7 @@ mod tests { ), ] ); - assert_eq!( - log, - Some( - logs() - .path(Logged::read("implement-resume").unwrap()) - .unwrap() - ) - ); + assert_eq!(log, Some(log_of("implement-resume"))); } #[test] @@ -758,14 +751,7 @@ mod tests { format!("{:#}", taken.unwrap_err()), format!("implement session {TESTS_KILLED}, and a later step failed: no PR found") ); - assert_eq!( - log, - Some( - logs() - .path(Logged::read("implement-resume").unwrap()) - .unwrap() - ) - ); + assert_eq!(log, Some(log_of("implement-resume"))); } #[test] @@ -789,10 +775,7 @@ mod tests { ), ] ); - assert_eq!( - log, - Some(logs().path(Logged::read("implement").unwrap()).unwrap()) - ); + assert_eq!(log, Some(log_of("implement"))); } #[test] @@ -810,10 +793,7 @@ mod tests { [logging("implement"), session("implement", None, "do it")] ); // The failing session's log is still the last log. - assert_eq!( - log, - Some(logs().path(Logged::read("implement").unwrap()).unwrap()) - ); + assert_eq!(log, Some(log_of("implement"))); } #[test] diff --git a/src/session/kind.rs b/src/session/kind.rs index b5ceb980..09c1528a 100644 --- a/src/session/kind.rs +++ b/src/session/kind.rs @@ -41,29 +41,26 @@ impl Kind { } } - /// The kind `token` names, if any. + /// The kind `token` names, if any. A numbered kind's number is read as + /// `--session repair` has always read a Repair's. fn read(token: &str) -> Option { + const UNNUMBERED: [Kind; 6] = [ + Kind::Implementation, + Kind::SpecReview, + Kind::SecurityReview, + Kind::ArchitectureReview, + Kind::SecurityAudit, + Kind::SecurityFixPublishing, + ]; let numbered = |prefix: &str| { - token - .strip_prefix(prefix) - .filter(|n| !n.is_empty() && n.bytes().all(|b| b.is_ascii_digit())) - .and_then(|n| n.parse().ok()) + let n = token.strip_prefix(prefix)?.parse::().ok()?; + Some(n as usize) }; - Some(match token { - "implement" => Self::Implementation, - "spec-review" => Self::SpecReview, - "security-review" => Self::SecurityReview, - "architecture-review" => Self::ArchitectureReview, - "security-audit" => Self::SecurityAudit, - "security-fix-publishing" => Self::SecurityFixPublishing, - _ => { - if let Some(n) = numbered("repair-") { - Self::Repair(n) - } else { - Self::SecurityReproduction(numbered("security-reproduction-")?) - } - } - }) + UNNUMBERED + .into_iter() + .find(|kind| kind.token() == token) + .or_else(|| numbered("repair-").map(Kind::Repair)) + .or_else(|| numbered("security-reproduction-").map(Kind::SecurityReproduction)) } } @@ -129,14 +126,22 @@ impl Logged { } /// Whether `thirdshift logs --session ` selects the Session log whose -/// token is `token`: one whose token is `word`, whose kind is `word`, Resume -/// or not, or, for `repair`, any numbered Repair or its Resume. +/// token is `token`: one whose token is `word`, or `word` with `-resume`, +/// or, for `repair`, any numbered Repair or its Resume. A token no kind +/// reads, as older thirdshift versions may have written, is still selected +/// by the first two rules. pub fn selects(word: &str, token: &str) -> bool { + let resumed = token.strip_suffix("-resume").unwrap_or(token); token == word - || Logged::read(token).is_some_and(|logged| { - logged.kind.token() == word - || (word == "repair" && matches!(logged.kind, Kind::Repair(_))) - }) + || resumed == word + || (word == "repair" + && matches!( + Logged::read(token), + Some(Logged { + kind: Kind::Repair(_), + .. + }) + )) } #[cfg(test)] @@ -171,7 +176,7 @@ mod tests { for token in [ "repair-", "repair-x", - "repair-+1", + "repair-4294967296", "repair--resume", "security-reproduction-", "security-reproduction-x", @@ -236,11 +241,20 @@ mod tests { } #[test] - fn an_unreadable_token_is_selected_only_by_itself() { + fn repair_selects_the_numbers_it_has_always_read_and_no_others() { + assert!(selects("repair", "repair-+1")); + assert!(selects("repair", "repair-01-resume")); + assert!(!selects("repair", "repair-4294967296")); + assert!(!selects("repair-1", "repair-01")); + } + + #[test] + fn an_unreadable_token_is_selected_by_itself_or_without_its_resume() { assert!(selects("review", "review")); assert!(selects("repair-x", "repair-x")); assert!(!selects("repair", "repair-x")); assert!(selects("legacy-resume", "legacy-resume")); - assert!(!selects("legacy", "legacy-resume")); + assert!(selects("legacy", "legacy-resume")); + assert!(!selects("legacy", "legacy-other")); } }