From b26821d0186e04c42d1fb3f8354e785a395b5025 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Fri, 9 Oct 2026 11:21:55 -0400 Subject: [PATCH 1/3] Store Session logs beside their owning Command log (#599) --- README.md | 26 +++++---- src/asks.rs | 1 + src/child_run.rs | 28 +++++++++- src/child_run/execution_tests.rs | 1 + src/logs.rs | 46 ++++++++++++++- src/logs/recording.rs | 17 +++++- src/logs/recording/tests.rs | 10 +++- src/main.rs | 10 ++-- src/session.rs | 12 ++-- src/session/execution_tests.rs | 10 +++- tests/activity_log.rs | 2 +- tests/agy_sessions.rs | 4 +- tests/architect_run.rs | 7 ++- tests/background_work.rs | 2 +- tests/ci.rs | 2 +- tests/codex_sessions.rs | 5 +- tests/command_logs.rs | 96 +++++++++++++++++++++----------- tests/continuation.rs | 2 +- tests/failed_run.rs | 4 +- tests/fresh_run.rs | 10 ++-- tests/grok_sessions.rs | 8 +-- tests/harness_choice.rs | 2 +- tests/merge_run.rs | 2 +- tests/mergeable.rs | 2 +- tests/muse_sessions.rs | 2 +- tests/opencode_sessions.rs | 2 +- tests/pickup_run.rs | 9 ++- tests/progress_lines.rs | 8 +-- tests/run_notification.rs | 4 +- tests/security_run.rs | 32 +++++++++-- tests/setup.rs | 2 +- tests/support/mod.rs | 12 ++++ tests/user_config.rs | 6 +- 33 files changed, 276 insertions(+), 110 deletions(-) diff --git a/README.md b/README.md index 17d1a567..de6b2e60 100644 --- a/README.md +++ b/README.md @@ -420,24 +420,26 @@ Everything thirdshift logs goes under `~/.thirdshift/logs/`, or the `logs.dir` s ``` ~/.thirdshift/logs/// ├── activity.log the Activity log -├── sessions/ Session logs └── commands/ - ├── architect/ .log - ├── pickup/ -.log - └── issue/ -.log + ├── architect/ .log and Session logs + ├── pickup/ -.log and Session logs + ├── secure/ .log and Session logs + └── issue/ -.log and Session logs ``` The folder already names the repository, so no file name repeats it. A **Session log** is one session's full transcript, as Claude Code's `stream-json` output: ``` -~/.thirdshift/logs///sessions/--implement.jsonl -~/.thirdshift/logs///sessions/--repair-.jsonl -~/.thirdshift/logs///sessions/architect--architecture-review.jsonl +~/.thirdshift/logs///commands/issue/--implement.jsonl +~/.thirdshift/logs///commands/issue/--repair-.jsonl +~/.thirdshift/logs///commands/architect/architect--architecture-review.jsonl ``` +Session logs share the owning command's folder: a Pickup run's dispatched Run logs in `commands/pickup/`, an Architect run's in `commands/architect/`, and a Security run's in `commands/secure/`. Tickets and Base fixes inherit that folder too. + A Resume is logged as its session's kind plus `-resume`, e.g. `implement-resume.jsonl`. -A **Command log** is everything one command printed, stderr and stdout in the order printed, while the terminal still shows all of it. Its folder is the command typed: `thirdshift `, a Run or a Spec run, in `issue/`, `thirdshift pickup` in `pickup/`, named for the issue it took, and `thirdshift architect` in `architect/`. A Spec run's covers its Tickets' Runs, a Run's covers its [Base fix](#base-fix), and a Pass's covers the Spec run or Run it dispatched: none of those keeps one of its own. So `ls -t ~/.thirdshift/logs/acme/widgets/commands/pickup | head` lists the recent passes on acme/widgets that took an issue. A command keeps its Command log once it starts work: a Pass skipped before doing any work keeps none, nor does a Run that fails before it starts work, as on the [Origin match](#what-a-run-does), and `setup`, `email-test`, `update`, `version` and `help` never keep one. Lines printed before the Command log's name is known, such as a Pickup run's lines on the issues it [passed over](#why-an-issue-was-passed-over) before it took one, are written first, and once it is created a progress line says `logging this command to `. A Command log that can't be written, as when its folder can't be created or the disk is full, is one `warning:` line on stderr, and the command carries on with the same outcome, stdout and exit code. +A **Command log** is everything one command printed, stderr and stdout in the order printed, while the terminal still shows all of it. Its folder is the command typed: `thirdshift `, a Run or a Spec run, in `issue/`, `thirdshift pickup` in `pickup/`, named for the issue it took, and `thirdshift architect` in `architect/`. A Spec run's covers its Tickets' Runs, a Run's covers its [Base fix](#base-fix), and a Pass's covers the Spec run or Run it dispatched: none of those keeps one of its own. So `ls -t ~/.thirdshift/logs/acme/widgets/commands/pickup/*.log | head` lists the recent passes on acme/widgets that took an issue. A command keeps its Command log once it starts work: a Pass skipped before doing any work keeps none, nor does a Run that fails before it starts work, as on the [Origin match](#what-a-run-does), and `setup`, `email-test`, `update`, `version` and `help` never keep one. Lines printed before the Command log's name is known, such as a Pickup run's lines on the issues it [passed over](#why-an-issue-was-passed-over) before it took one, are written first, and once it is created a progress line says `logging this command to `. A Command log that can't be written produces one `warning:` line on stderr. A failure confined to the Command log leaves the outcome, stdout and exit code unchanged; if the shared folder also prevents writing a Session log, the Run fails before starting that session. The stamp is the local time the command started, with its UTC offset, as in `20261003T120000-0400`, in the machine's time zone, or `TZ`'s if set, so it agrees with `date` and `ls -l`. A command's Command log and all of its Session logs, its Tickets' Runs' and its Base fix's included, share that one stamp, so they sort together and each can be found from the other. When a Run fails, stderr ends with the path of its most recent Session log, the place to start looking, then that of its Command log. @@ -450,7 +452,7 @@ The **Activity log**, `activity.log`, is a short running record of what the fact 2026-10-03 10:00:01 Pickup run skipped: no Ready issue on acme/widgets ``` -Each line is appended whole, so passes that run at once on one repository never garble it. One that can't be written is one `warning:` line on stderr, and the command carries on as for a Command log. thirdshift never rotates it: collapsed skips keep it small. Logs written before this layout, under `sessions/` and `commands/` at the root of the logs, are not moved. +Each line is appended whole, so passes that run at once on one repository never garble it. One that can't be written is one `warning:` line on stderr, and the command carries on as for a Command log. thirdshift never rotates it: collapsed skips keep it small. Existing logs, including Session logs in `//sessions/` and logs from before the per-repository layout, are not moved. With `quiet_skips = true` in the `[activity]` section of the [User config](#user-config), a skipped Pass prints nothing on stdout or stderr, its dated first line included, and leaves only its Activity log line. A pass that does work, or fails, prints as ever, so a scheduler's log file catches only what went wrong. Without it, a skipped pass prints as it always has, for a pass you type by hand. @@ -536,7 +538,7 @@ thirdshift: 03:12:40 CI red on test, which also fails on main at 362b9ca; fix ma thirdshift: 03:12:40 Base check: test: https://github.com/acme/widgets/actions/runs/1/job/2 thirdshift: 03:12:40 Retry with: thirdshift https://github.com/acme/widgets/issues/7 base-fix thirdshift: 03:12:40 Or set: base.fix = true in ~/.thirdshift/config.toml, to allow a Base fix for every Run on this machine -thirdshift: 03:12:40 session log: ~/.thirdshift/logs/acme/widgets/sessions/7-….jsonl +thirdshift: 03:12:40 session log: ~/.thirdshift/logs/acme/widgets/commands/issue/7-….jsonl thirdshift: 03:12:40 command log: ~/.thirdshift/logs/acme/widgets/commands/issue/7-….log ``` @@ -572,7 +574,7 @@ thirdshift --parallel 1 https://github.com/acme/widgets/issues/20 # one at a tim When nothing is left to run and any Ticket is not done, the Spec run is a **Failed spec run**: it leaves the Spec PR a draft, its checklist showing what's missing, prints its URL on stdout (if any Ticket has landed, so there is one), exits `1`, and lists on stderr each Ticket that landed, with its pull request, and each one not done, with why: ``` -thirdshift: 03:12:40 #21 failed: claude exited 1 (session log: ~/.thirdshift/logs/acme/widgets/sessions/21-….jsonl) +thirdshift: 03:12:40 #21 failed: claude exited 1 (session log: ~/.thirdshift/logs/acme/widgets/commands/issue/21-….jsonl) thirdshift: 03:12:40 #22 blocked by #21 thirdshift: 03:12:40 #23 landed with https://github.com/acme/widgets/pull/1 thirdshift: 03:12:40 #24 unready: labelled needs-info @@ -770,7 +772,7 @@ A Security run checks its gates in order: another **Pass** on the repository run The Security audit runs in a throwaway worktree detached at origin's Base branch head, leaving the Launch directory and local work untouched, including when `launch.pull` is set. Its Session prompt runs `thirdshift-security-audit` in full audit mode with the `quick` profile, auditing the whole repository except vendored and third-party code. It names a `SECURITY.md` or conventional threat-model document when present, reads compatible earlier runs and ends `incomplete` instead of asking for input. -Each audit keeps a new output directory under `///audits/`, outside the worktree. The audit artifacts and private advisory descriptions contain evidence; keep the logs directory private. thirdshift runs both embedded validators before recording anything. A missing final line, an incomplete session or an invalid report fails the run and names its Session log. Command logs are in `commands/secure/`; Session logs are named `secure--security-audit.jsonl`; the Activity log records the start and ending as for other Passes. +Each audit keeps a new output directory under `///audits/`, outside the worktree. The audit artifacts and private advisory descriptions contain evidence; keep the logs directory private. thirdshift runs both embedded validators before recording anything. A missing final line, an incomplete session or an invalid report fails the run and names its Session log. Command logs and Session logs are in `commands/secure/`; Session logs are named `secure--security-audit.jsonl`; the Activity log records the start and ending as for other Passes. A finding's title becomes the advisory summary. Its description preserves its write-up, trace, evidence and validation plan, with the fingerprint and audited commit. thirdshift claims no severity, CWE or affected version range. It uses the package in the repository's manifest, or the `other` ecosystem when none is identified. Matching fingerprints in any advisory state are kept rather than recorded again. Rejected candidates stay in the local report and produce no advisory. Draft advisories require GitHub repository security manager or administrator access. On a private repository whose advisory endpoint is unavailable, the private record is an issue labelled `security-finding` and `needs-triage`. It is never replaced by a public finding issue. diff --git a/src/asks.rs b/src/asks.rs index 0b398917..01f479e9 100644 --- a/src/asks.rs +++ b/src/asks.rs @@ -463,6 +463,7 @@ mod tests { /// fix, its sessions on Claude's `sonnet`. fn given(kind: Kind, base_fix: BaseFixAsk) -> Given { Given { + command: crate::logs::CommandKind::Issue, security: crate::security::Options::default(), kind, stamp: "20261003T120000-0400".to_string(), diff --git a/src/child_run.rs b/src/child_run.rs index 4c5267db..6d528b63 100644 --- a/src/child_run.rs +++ b/src/child_run.rs @@ -70,6 +70,8 @@ pub struct Given { /// The start stamp of the command that started it, which its Session /// logs take. pub stamp: String, + /// The owning command, whose folder its Session logs inherit. + pub command: logs::CommandKind, /// What it is asked about a Base fix. pub base_fix: BaseFixAsk, pub security: Options, @@ -91,6 +93,9 @@ const BASE_FIX_INTO: &str = "--base-fix-into"; /// The hidden argument followed by the command's start stamp. const STAMP: &str = "--stamp"; +/// The hidden argument followed by the owning command's log folder. +const LOGS_COMMAND: &str = "--logs-command"; + /// The hidden argument that asks a child Run [`BaseFixAsk::Allow`]. const ALLOW_BASE_FIX: &str = "--allow-base-fix"; const SECURITY_REVIEW: &str = "--run-security-review"; @@ -123,6 +128,8 @@ impl Given { self.kind.base(), STAMP, &self.stamp, + LOGS_COMMAND, + self.command.folder(), ]; match &self.base_fix { BaseFixAsk::Allow => args.push(ALLOW_BASE_FIX), @@ -153,6 +160,7 @@ impl Given { pub struct Reader { kind: Option, stamp: Option, + command: Option, base_fix: Option, security: Options, harness: Option, @@ -199,6 +207,13 @@ impl Reader { not_yet_given(&self.stamp, arg)?; self.stamp = Some(value("stamp")?); } + LOGS_COMMAND => { + not_yet_given(&self.command, arg)?; + let name = value("logs command")?; + self.command = Some(logs::CommandKind::named(&name).with_context(|| { + format!("{arg} must name issue, pickup, architect or secure, not {name}") + })?); + } ALLOW_BASE_FIX | OFFER_BASE_FIX => { if let Some(given) = &self.base_fix && matches!(given, BaseFixAsk::Allow) != (arg == ALLOW_BASE_FIX) @@ -239,7 +254,8 @@ impl Reader { /// arguments a kind. pub fn finish(self) -> Result> { let Some(kind) = self.kind else { - if self.stamp.is_some() + if self.command.is_some() + || self.stamp.is_some() || self.base_fix.is_some() || self.security.review || self.security.fix @@ -249,7 +265,7 @@ impl Reader { { bail!( "only a child Run is given {STAMP}, {ALLOW_BASE_FIX}, {OFFER_BASE_FIX}, \ - {SESSIONS_HARNESS}, {SESSIONS_MODEL} or {SESSIONS_EFFORT}" + {SESSIONS_HARNESS}, {SESSIONS_MODEL}, {SESSIONS_EFFORT} or {LOGS_COMMAND}" ); } return Ok(None); @@ -261,6 +277,9 @@ impl Reader { Ok(Some(Given { kind, stamp, + command: self + .command + .with_context(|| format!("missing {LOGS_COMMAND}"))?, base_fix: self.base_fix.unwrap_or(BaseFixAsk::Forbid), security: self.security, harness: Choice { @@ -330,6 +349,7 @@ pub fn start( let given = Given { kind, stamp: logs::stamp(), + command: logs::command_kind(), base_fix, security, harness: harness.clone(), @@ -708,6 +728,7 @@ mod tests { let given = Given { kind: ticket(), stamp: STAMPED.to_string(), + command: logs::CommandKind::Issue, base_fix: BaseFixAsk::Forbid, security: Options::default(), harness: Choice::default(), @@ -766,6 +787,7 @@ mod tests { let given = Given { kind: kind.clone(), stamp: STAMPED.to_string(), + command: logs::CommandKind::Issue, base_fix, security: Options::default(), harness: harness.clone(), @@ -792,7 +814,7 @@ mod tests { /// What a Run given a hidden argument only a child Run is given, without /// a kind, is rejected with. const ONLY_A_CHILD_RUN: &str = "only a child Run is given --stamp, --allow-base-fix, \ - --offer-base-fix, --sessions-harness, --sessions-model or --sessions-effort"; + --offer-base-fix, --sessions-harness, --sessions-model, --sessions-effort or --logs-command"; #[test] fn hidden_arguments_that_are_repeated_have_no_value_or_lack_a_kind_or_stamp_are_rejected() { diff --git a/src/child_run/execution_tests.rs b/src/child_run/execution_tests.rs index 4681924a..394ce7f1 100644 --- a/src/child_run/execution_tests.rs +++ b/src/child_run/execution_tests.rs @@ -166,6 +166,7 @@ impl Fixture { spec_branch: "issue-237".to_string(), }, stamp: "20261003T120000-0400".to_string(), + command: logs::CommandKind::Issue, base_fix: BaseFixAsk::Forbid, security: crate::security::Options::default(), harness: Choice::default(), diff --git a/src/logs.rs b/src/logs.rs index 9b541922..415e9f6b 100644 --- a/src/logs.rs +++ b/src/logs.rs @@ -28,6 +28,36 @@ use activity::Kind; use effects::OnMachine; use recording::Record; +/// The command typed, which owns its Session logs and all child Runs' logs. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CommandKind { + Issue, + Architect, + Pickup, + Secure, +} + +impl CommandKind { + pub fn folder(self) -> &'static str { + match self { + Self::Issue => "issue", + Self::Architect => "architect", + Self::Pickup => "pickup", + Self::Secure => "secure", + } + } + + pub fn named(name: &str) -> Option { + match name { + "issue" => Some(Self::Issue), + "architect" => Some(Self::Architect), + "pickup" => Some(Self::Pickup), + "secure" => Some(Self::Secure), + _ => None, + } + } +} + /// How a command begins. pub enum Begin<'a> { /// `thirdshift `, a Run or a Spec run on this issue. @@ -35,7 +65,7 @@ pub enum Begin<'a> { /// A child Run, a Ticket's Run or a Base fix, with the stamp the command /// that started it gave it. It keeps no Command log and writes no /// Activity log line: the command that started it does. - ChildRun(&'a str), + ChildRun(&'a str, CommandKind), /// `thirdshift architect`, a pass. ArchitectRun, /// `thirdshift pickup`, a pass. @@ -144,6 +174,20 @@ pub fn root(repo: &Repo) -> PathBuf { record().root(repo) } +/// The command whose folder holds this process's Session logs. +pub fn command_kind() -> CommandKind { + record().command_kind() +} + +/// Where this command and all its child Runs keep Session logs on `repo`. +pub fn session_dir(repo: &Repo) -> PathBuf { + let record = record(); + record + .root(repo) + .join("commands") + .join(record.command_kind().folder()) +} + /// The command's start stamp, as in `20261003T120000-0400`, which names its /// Command log and its Session logs. pub fn stamp() -> String { diff --git a/src/logs/recording.rs b/src/logs/recording.rs index 4adeb294..8a77ccf8 100644 --- a/src/logs/recording.rs +++ b/src/logs/recording.rs @@ -10,7 +10,7 @@ use anyhow::{Context, Result}; use super::activity::{self, Kind}; use super::effects::{Outside, Stream}; -use super::{Begin, Pass, Work, root_under}; +use super::{Begin, CommandKind, Pass, Work, root_under}; use crate::config::UserConfig; use crate::harness::Choice; use crate::issue::Repo; @@ -23,6 +23,7 @@ pub(super) struct Record { stamp: Option, logs_dir: Option, child: bool, + command: CommandKind, started: bool, command_log: Log, activity_started: Option, @@ -66,6 +67,7 @@ impl Record { stamp: None, logs_dir: None, child: false, + command: CommandKind::Issue, started: false, command_log: Log::NotKept, activity_started: None, @@ -74,8 +76,15 @@ impl Record { } pub(super) fn begin(&mut self, begin: Begin) { + self.command = match &begin { + Begin::Run(_) => CommandKind::Issue, + Begin::ChildRun(_, command) => *command, + Begin::ArchitectRun => CommandKind::Architect, + Begin::PickupRun => CommandKind::Pickup, + Begin::SecurityRun => CommandKind::Secure, + }; let (starting, quiet) = match begin { - Begin::ChildRun(stamp) => { + Begin::ChildRun(stamp, _) => { self.child = true; self.stamp.get_or_insert_with(|| stamp.to_string()); return; @@ -168,6 +177,10 @@ impl Record { root_under(logs_dir, repo) } + pub(super) fn command_kind(&self) -> CommandKind { + self.command + } + pub(super) fn stamp(&mut self) -> String { self.stamp .get_or_insert_with(|| self.outside.now().format(STAMP_FORMAT).to_string()) diff --git a/src/logs/recording/tests.rs b/src/logs/recording/tests.rs index f0d90543..3c8de6ec 100644 --- a/src/logs/recording/tests.rs +++ b/src/logs/recording/tests.rs @@ -337,7 +337,10 @@ fn a_child_inherits_its_parent_stamp_and_relays_output_without_a_beginning_or_lo let mut record = Record::new(memory.clone()); let issue = issue(); let repo = issue.repo(); - record.begin(Begin::ChildRun("parent-stamp")); + record.begin(Begin::ChildRun( + "parent-stamp", + crate::logs::CommandKind::Issue, + )); assert_eq!(record.stamp(), "parent-stamp"); // Child starts are ignored even before configuration. for work in [ @@ -384,7 +387,10 @@ fn lazy_stamps_are_owned_stable_and_independent_of_other_recordings() { let mut second = Record::new(memory.clone()); assert_eq!(second.stamp(), "20261004T130102+0530"); - second.begin(Begin::ChildRun("ignored-after-lazy-query")); + second.begin(Begin::ChildRun( + "ignored-after-lazy-query", + crate::logs::CommandKind::Issue, + )); assert_eq!(second.stamp(), "20261004T130102+0530"); assert_eq!(second.command_log_path(), None); let mut other_config = config(true); diff --git a/src/main.rs b/src/main.rs index 7b4021e2..d957e3a5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -369,11 +369,13 @@ With launch.pull set, every Run first fast-forwards the checked-out Base branch logs.dir sets the root of the logs instead of ~/.thirdshift/logs: an absolute path, or one under ~/. Each repository's logs go in // under it, named for the GitHub repository, in -folders thirdshift creates as it needs them. Session logs go in sessions/, as ---.jsonl for a Run and architect--.jsonl for an Architect run. +folders thirdshift creates as it needs them. Session logs go beside their Command log in +commands//, as --.jsonl for a Run, architect--.jsonl +for an Architect run and secure--.jsonl for a Security run. Dispatched Runs, +Tickets and Base fixes share the command's folder. Existing logs are not moved. Command logs, everything a Run, a Spec run or a Pass printed, go in commands/issue/-.log, commands/pickup/-.log, named for the issue taken, -and commands/architect/.log. A Pass skipped before any work +commands/architect/.log and commands/secure/.log. A Pass skipped before any work keeps no Command log, nor does a Run that fails before any work, as on the Origin match. [logs] @@ -440,7 +442,7 @@ fn main() -> ExitCode { Err(error) => return argument_error(format_args!("{error:#}")), }; let begin = match &given { - Some(given) => Begin::ChildRun(&given.stamp), + Some(given) => Begin::ChildRun(&given.stamp, given.command), None => Begin::Run(&issue), }; let asks_of = |config: &UserConfig| match &given { diff --git a/src/session.rs b/src/session.rs index d69c7bab..91cdd8be 100644 --- a/src/session.rs +++ b/src/session.rs @@ -39,8 +39,8 @@ pub enum Purpose { SecurityReview, } -/// Where a Run's or an Architect run's Session logs go: in `sessions/` under -/// the root of its repository's logs, under the User config's `logs.dir` or +/// Where Session logs go: beside the owning Command log in +/// `commands//` under the repository's logs, under the User config's `logs.dir` or /// `~/.thirdshift/logs`, each named for what is run and stamped with the /// command's start stamp, which its Command log shares. Only once the logs /// are [`logs::configured`]. @@ -56,7 +56,7 @@ impl Logs { pub fn of_run(issue: &IssueUrl) -> Self { Logs { name: issue.number.to_string(), - dir: logs::root(&issue.repo()).join("sessions"), + dir: logs::session_dir(&issue.repo()), } } @@ -64,7 +64,7 @@ impl Logs { pub fn of_architect_run(repo: &Repo) -> Self { Logs { name: "architect".to_string(), - dir: logs::root(repo).join("sessions"), + dir: logs::session_dir(repo), } } @@ -72,12 +72,12 @@ impl Logs { pub fn of_security_run(repo: &Repo) -> Self { Logs { name: "secure".to_string(), - dir: logs::root(repo).join("sessions"), + dir: logs::session_dir(repo), } } /// Where a session's stream is logged: - /// `/sessions/--.jsonl`. + /// `/commands//--.jsonl`. pub fn path(&self, kind: &str) -> PathBuf { self.dir .join(format!("{}-{}-{kind}.jsonl", self.name, logs::stamp())) diff --git a/src/session/execution_tests.rs b/src/session/execution_tests.rs index f0fc8668..134d5582 100644 --- a/src/session/execution_tests.rs +++ b/src/session/execution_tests.rs @@ -92,7 +92,10 @@ impl Drop for Fixture { fn execute_session(root: &Path) -> anyhow::Error { interrupt::install().unwrap(); - logs::begin(logs::Begin::ChildRun("fixture")); + logs::begin(logs::Begin::ChildRun( + "fixture", + crate::logs::CommandKind::Issue, + )); let worktree = root.join("worktree"); fs::create_dir(&worktree).unwrap(); assert!( @@ -304,7 +307,10 @@ fn with_review_sessions( harness: Harness, steps: impl FnOnce(&Sessions) -> Result<()>, ) -> (Result<()>, PathBuf) { - logs::begin(logs::Begin::ChildRun("fixture")); + logs::begin(logs::Begin::ChildRun( + "fixture", + crate::logs::CommandKind::Issue, + )); let worktree = root.join("worktree"); fs::create_dir(&worktree).unwrap(); assert!( diff --git a/tests/activity_log.rs b/tests/activity_log.rs index 6ab151d6..c9cbe2cd 100644 --- a/tests/activity_log.rs +++ b/tests/activity_log.rs @@ -68,7 +68,7 @@ fn activity(scenario: &Scenario) -> Vec { /// The name of the one Command log in `commands/`. fn the_one_command_log(scenario: &Scenario, folder: &str) -> String { - let names = scenario.entries(&format!("{LOGS_DIR}/acme/widgets/commands/{folder}")); + let names = scenario.log_files(&format!("{LOGS_DIR}/acme/widgets/commands/{folder}"), "log"); assert_eq!(names.len(), 1, "{names:?}"); format!("commands/{folder}/{}", names[0]) } diff --git a/tests/agy_sessions.rs b/tests/agy_sessions.rs index 49deb0aa..fc163a53 100644 --- a/tests/agy_sessions.rs +++ b/tests/agy_sessions.rs @@ -93,8 +93,8 @@ echo '{{"event":"step_update","step_update":{{"step_index":2,"state":"DONE","ste .unwrap() .contains("Built with agy · gemini-3.8-flash · medium") ); - let sessions = "home/.thirdshift/logs/acme/widgets/sessions"; - let logs = scenario.entries(sessions); + let sessions = "home/.thirdshift/logs/acme/widgets/commands/issue"; + let logs = scenario.log_files(sessions, "jsonl"); let log = std::fs::read_to_string(scenario.path(&format!("{sessions}/{}", logs[0]))).unwrap(); assert!(log.contains("\"event\": \"init\""), "{log}"); assert!(log.contains("\"status\": \"SUCCESS\""), "{log}"); diff --git a/tests/architect_run.rs b/tests/architect_run.rs index c6b64ceb..fe371b63 100644 --- a/tests/architect_run.rs +++ b/tests/architect_run.rs @@ -197,14 +197,17 @@ fn assert_failed(scenario: &Scenario, result: &RunResult, cause: &str) { "expected {cause:?} in stderr: {}", result.stderr ); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files( + "home/.thirdshift/logs/acme/widgets/commands/architect", + "jsonl", + ); assert_eq!(logs.len(), 1, "logs: {logs:?}"); assert!( logs[0].starts_with("architect-") && logs[0].ends_with("-architecture-review.jsonl"), "logs: {logs:?}" ); let log = scenario - .path("home/.thirdshift/logs/acme/widgets/sessions") + .path("home/.thirdshift/logs/acme/widgets/commands/architect") .join(&logs[0]); assert_eq!( before_command_log(&result.stderr).last(), diff --git a/tests/background_work.rs b/tests/background_work.rs index 0244ec13..4a329460 100644 --- a/tests/background_work.rs +++ b/tests/background_work.rs @@ -68,7 +68,7 @@ fn a_session_whose_background_work_was_killed_is_resumed_once_and_the_run_goes_o result.stderr ); } - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); assert_eq!(logs.len(), 2, "logs: {logs:?}"); assert!(logs.iter().any(|log| log.ends_with("-implement.jsonl"))); assert!( diff --git a/tests/ci.rs b/tests/ci.rs index 8162a52b..7e0b6cef 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -122,7 +122,7 @@ fn red_ci_is_handed_to_a_ci_fix_repair_whose_fix_turns_it_green() { "Initial commit".to_string(), ]) ); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); assert!( logs.iter().any(|name| name.ends_with("-repair-1.jsonl")), "logs: {logs:?}" diff --git a/tests/codex_sessions.rs b/tests/codex_sessions.rs index 82e87072..4b6112ca 100644 --- a/tests/codex_sessions.rs +++ b/tests/codex_sessions.rs @@ -187,12 +187,13 @@ fn a_runs_implement_session_and_its_repair_run_on_codex_with_the_model_and_effor body.ends_with("Built with codex · gpt-6.1-sol · max \n"), "{body}" ); - let logs = scenario.entries(&format!("{LOGS}/sessions")); + let logs = scenario.log_files(&format!("{LOGS}/commands/issue"), "jsonl"); let implement = logs .iter() .find(|log| log.ends_with("-implement.jsonl")) .unwrap_or_else(|| panic!("no implement session log: {logs:?}")); - let log = fs::read_to_string(scenario.path(&format!("{LOGS}/sessions/{implement}"))).unwrap(); + let log = + fs::read_to_string(scenario.path(&format!("{LOGS}/commands/issue/{implement}"))).unwrap(); assert!( log.starts_with(r#"{"type": "thread.started", "thread_id": "fake-thread-1"}"#), "{log}" diff --git a/tests/command_logs.rs b/tests/command_logs.rs index 07132f12..e5eb79b4 100644 --- a/tests/command_logs.rs +++ b/tests/command_logs.rs @@ -1,7 +1,7 @@ //! Command logs: each Run, Spec run, Architect run and Pickup run keeps a file //! of everything it printed, stderr and stdout in order, under //! `///commands//`, with its Session logs -//! under `///sessions/`, all stamped with the +//! beside them in that folder, all stamped with the //! command's start in local time and its UTC offset. A child Run keeps none: its lines are in the //! Command log of what started it. A Pickup run or Architect run skipped //! before doing any work keeps none, nor does a Run that fails before it @@ -64,7 +64,10 @@ fn logs_in(scenario: &Scenario, folder: &str) -> Vec { /// The one Command log in `commands/`, which must be named /// `.log`, and its stamp. fn the_one_command_log(scenario: &Scenario, folder: &str, prefix: &str) -> (PathBuf, String) { - let logs = logs_in(scenario, &format!("commands/{folder}")); + let logs: Vec<_> = logs_in(scenario, &format!("commands/{folder}")) + .into_iter() + .filter(|name| name.ends_with(".log")) + .collect(); assert_eq!(logs.len(), 1, "Command logs: {logs:?}"); let stamp = logs[0] .strip_prefix(prefix) @@ -130,13 +133,13 @@ fn a_run_keeps_a_command_log_of_everything_it_printed_and_its_session_logs_share result.stderr ); assert_eq!( - logs_in(&scenario, "sessions"), - [format!("7-{stamp}-implement.jsonl")] - ); - assert_eq!( - logs_in(&scenario, ""), - ["activity.log", "commands", "sessions"] + logs_in(&scenario, "commands/issue"), + [ + format!("7-{stamp}-implement.jsonl"), + format!("7-{stamp}.log") + ] ); + assert_eq!(logs_in(&scenario, ""), ["activity.log", "commands"]); assert_eq!( fs::read_dir(scenario.path(LOGS_DIR)) .unwrap() @@ -166,7 +169,7 @@ fn a_spec_runs_command_log_has_its_tickets_lines_and_its_tickets_keep_none() { ); } assert_eq!( - logs_in(&scenario, "sessions"), + scenario.log_files(&format!("{LOGS}/commands/issue"), "jsonl"), [ format!("20-{stamp}-spec-review.jsonl"), format!("21-{stamp}-implement.jsonl"), @@ -195,9 +198,37 @@ fn a_pickup_run_that_takes_an_issue_keeps_a_command_log_from_its_first_line() { let logging = log.find("logging this command to").expect(&log); assert!(passed_over < logging, "log: {log}"); assert_eq!( - logs_in(&scenario, "sessions"), - [format!("7-{stamp}-implement.jsonl")] + logs_in(&scenario, "commands/pickup"), + [ + format!("7-{stamp}-implement.jsonl"), + format!("7-{stamp}.log") + ] + ); +} + +#[test] +fn a_pickup_spec_keeps_its_tickets_session_logs_beside_its_command_log() { + let scenario = Scenario::new(); + scenario.spec_has_tickets(20, &[(21, &[]), (22, &[21])]); + scenario.issue_labelled(20, &["ready-for-agent"]); + scenario.agent_does_for(21, &agent_opens_pr(21, "issue-20")); + scenario.agent_does_for(22, &agent_opens_pr(22, "issue-20")); + + let result = run_in_zone(&scenario, &["pickup"], &[]); + + assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); + let (_, stamp) = the_one_command_log(&scenario, "pickup", "20-"); + assert_eq!( + logs_in(&scenario, "commands/pickup"), + [ + format!("20-{stamp}-spec-review.jsonl"), + format!("20-{stamp}.log"), + format!("21-{stamp}-implement.jsonl"), + format!("22-{stamp}-implement.jsonl"), + ] ); + assert!(!scenario.path(LOGS).join("sessions").exists()); + assert!(!scenario.path(LOGS).join("commands/issue").exists()); } #[test] @@ -225,7 +256,7 @@ printf 'Architecture review plan: %s\n' "$url" > "$FAKE_CLAUDE_FINAL_MESSAGE" "log: {log}" ); assert_eq!( - logs_in(&scenario, "sessions"), + scenario.log_files(&format!("{LOGS}/commands/architect"), "jsonl"), [ format!("8-{stamp}-implement.jsonl"), format!("architect-{stamp}-architecture-review.jsonl"), @@ -244,7 +275,7 @@ fn a_failed_run_ends_by_naming_its_session_log_then_its_command_log() { let (path, stamp) = the_one_command_log(&scenario, "issue", "7-"); let session = scenario .path(LOGS) - .join(format!("sessions/7-{stamp}-implement.jsonl")); + .join(format!("commands/issue/7-{stamp}-implement.jsonl")); let ending: Vec<_> = result.stderr.lines().rev().take(2).collect(); assert_eq!( ending, @@ -283,34 +314,31 @@ fn a_run_notification_names_the_command_log() { } #[test] -fn a_command_log_that_cannot_be_written_is_one_warning_and_changes_nothing_else() { +fn an_obstructed_commands_folder_warns_and_prevents_session_logging() { let scenario = Scenario::new(); scenario.agent_does(&agent_opens_pr(7, "main")); - // A file where the folder of Command logs would go. + // Command and Session logs now share this folder. fs::create_dir_all(scenario.path(LOGS)).unwrap(); fs::write(scenario.path(LOGS).join("commands"), "").unwrap(); let result = run_in_zone(&scenario, &[&scenario.issue_url(7)], &[]); - assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); - assert_eq!(result.stdout, format!("{PR_URL}\n")); - let warnings: Vec<_> = result - .stderr - .lines() - .filter(|line| line.contains("warning:")) - .collect(); - assert_eq!(warnings.len(), 1, "stderr: {}", result.stderr); + assert_eq!(result.code, Some(1), "stderr: {}", result.stderr); + assert_eq!(result.stdout, ""); + assert_eq!( + result + .stderr + .matches("warning: could not keep the Command log:") + .count(), + 1 + ); assert!( - warnings[0].starts_with("thirdshift: warning: could not keep the Command log: "), + result.stderr.contains("could not create"), "{}", - warnings[0] - ); - assert!(!result.stderr.contains("logging this command to")); - assert_eq!( - result.stderr.lines().last(), - Some(format!("thirdshift: PR {PR_URL} is ready for review").as_str()) + result.stderr ); - assert_eq!(logs_in(&scenario, "sessions").len(), 1); + assert!(scenario.claude_calls().is_empty()); + assert!(!scenario.path(LOGS).join("sessions").exists()); } #[test] @@ -377,8 +405,10 @@ gh fake checks "$(git rev-parse origin/main)" '[{"name": "test", "conclusion": " "log: {log}" ); assert!( - logs_in(&scenario, "sessions").contains(&format!("8-{stamp}-implement.jsonl")), + scenario + .log_files(&format!("{LOGS}/commands/issue"), "jsonl") + .contains(&format!("8-{stamp}-implement.jsonl")), "{:?}", - logs_in(&scenario, "sessions") + scenario.log_files(&format!("{LOGS}/commands/issue"), "jsonl") ); } diff --git a/tests/continuation.rs b/tests/continuation.rs index b436142b..0fdff7a9 100644 --- a/tests/continuation.rs +++ b/tests/continuation.rs @@ -101,7 +101,7 @@ fn assert_nothing_created(scenario: &Scenario, local_branch: &str) { assert_eq!(scenario.entries("tmp"), Vec::::new()); assert!( !scenario - .path("home/.thirdshift/logs/acme/widgets/sessions") + .path("home/.thirdshift/logs/acme/widgets/commands/issue") .exists(), "a session log was created" ); diff --git a/tests/failed_run.rs b/tests/failed_run.rs index 8d24e9b1..f753aabd 100644 --- a/tests/failed_run.rs +++ b/tests/failed_run.rs @@ -52,10 +52,10 @@ fn a_failing_session_pushes_a_failure_commit_with_the_uncommitted_work_saying_wh fn assert_failed(scenario: &Scenario, result: &RunResult, stdout: &str) { assert_eq!(result.code, Some(1), "stderr: {}", result.stderr); assert_eq!(result.stdout, stdout); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); assert_eq!(logs.len(), 1, "logs: {logs:?}"); let log = scenario - .path("home/.thirdshift/logs/acme/widgets/sessions") + .path("home/.thirdshift/logs/acme/widgets/commands/issue") .join(&logs[0]); assert!( result.stderr.contains(log.to_str().unwrap()), diff --git a/tests/fresh_run.rs b/tests/fresh_run.rs index b7168afc..ca45ab2d 100644 --- a/tests/fresh_run.rs +++ b/tests/fresh_run.rs @@ -63,9 +63,9 @@ fn a_clean_failed_acquisition_releases_its_claim_and_can_retry_without_starting_ assert_eq!(failed.stdout, ""); assert!(scenario.claude_calls().is_empty()); assert!( - !scenario - .path("home/.thirdshift/logs/acme/widgets/sessions") - .exists() + scenario + .log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl") + .is_empty() ); assert_eq!(scenario.issue_labels(7), ["ready-for-agent"]); assert_eq!(scenario.origin_log("issue-7"), None); @@ -295,7 +295,7 @@ fn logs_the_session_stream_under_the_home_directory() { scenario.run(&[&scenario.issue_url(7)]); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); assert_eq!(logs.len(), 1, "logs: {logs:?}"); let name = &logs[0]; assert!( @@ -304,7 +304,7 @@ fn logs_the_session_stream_under_the_home_directory() { ); let log = std::fs::read_to_string( scenario - .path("home/.thirdshift/logs/acme/widgets/sessions") + .path("home/.thirdshift/logs/acme/widgets/commands/issue") .join(name), ) .unwrap(); diff --git a/tests/grok_sessions.rs b/tests/grok_sessions.rs index 71a149a9..909fd8f9 100644 --- a/tests/grok_sessions.rs +++ b/tests/grok_sessions.rs @@ -133,9 +133,9 @@ echo '{{"type":"assistant","message":{{"content":[{{"type":"tool_use","name":"ru assert_eq!(call["GROK_FOLDER_TRUST"], "0"); } scenario.assert_every_grok_session_found_the_factory_skills(); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); let log = std::fs::read_to_string(scenario.path(&format!( - "home/.thirdshift/logs/acme/widgets/sessions/{}", + "home/.thirdshift/logs/acme/widgets/commands/issue/{}", logs[0] ))) .unwrap(); @@ -173,9 +173,9 @@ fn groks_error_is_in_the_failure_cause_even_when_the_cli_exits_zero() { let result = scenario.run(&["harness", "grok", &scenario.issue_url(7)]); assert_eq!(result.code, Some(1), "{}", result.stderr); assert!(result.stderr.contains(cause), "{}", result.stderr); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); let log = std::fs::read_to_string(scenario.path(&format!( - "home/.thirdshift/logs/acme/widgets/sessions/{}", + "home/.thirdshift/logs/acme/widgets/commands/issue/{}", logs[0] ))) .unwrap(); diff --git a/tests/harness_choice.rs b/tests/harness_choice.rs index 60cdca15..6ed686bc 100644 --- a/tests/harness_choice.rs +++ b/tests/harness_choice.rs @@ -285,7 +285,7 @@ fn the_command_log_the_activity_log_and_the_pr_body_name_the_harness_model_and_e let result = scenario.run(&[&scenario.issue_url(7), "model", "opus", "effort", "high"]); assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); - let commands = scenario.entries(&format!("{LOGS}/commands/issue")); + let commands = scenario.log_files(&format!("{LOGS}/commands/issue"), "log"); assert_eq!(commands.len(), 1, "{commands:?}"); let command_log = fs::read_to_string(scenario.path(&format!("{LOGS}/commands/issue/{}", commands[0]))) diff --git a/tests/merge_run.rs b/tests/merge_run.rs index 4ac5acab..ed34e865 100644 --- a/tests/merge_run.rs +++ b/tests/merge_run.rs @@ -639,7 +639,7 @@ fn a_review_repair_whose_background_work_was_killed_gets_a_resume() { "prompt: {}", calls[2]["prompt"] ); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); assert!( logs.iter() .any(|log| log.ends_with("-repair-1-resume.jsonl")), diff --git a/tests/mergeable.rs b/tests/mergeable.rs index 5950dfdd..ccec6304 100644 --- a/tests/mergeable.rs +++ b/tests/mergeable.rs @@ -144,7 +144,7 @@ fn logs_the_conflict_repair_with_the_runs_shared_timestamp() { let result = scenario.run(&[&scenario.issue_url(7)]); assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); assert_eq!(logs.len(), 2, "logs: {logs:?}"); let implement = logs .iter() diff --git a/tests/muse_sessions.rs b/tests/muse_sessions.rs index f30e6eb1..2ae627ef 100644 --- a/tests/muse_sessions.rs +++ b/tests/muse_sessions.rs @@ -188,7 +188,7 @@ fn missing_log_uses_stream_text_without_usage_and_a_missing_skill_warns_once_in_ assert_eq!(result.stderr.matches(warning).count(), 1); assert!(!result.stderr.contains("input tokens")); let dir = "home/.thirdshift/logs/acme/widgets/commands/issue"; - let name = scenario.entries(dir).pop().unwrap(); + let name = scenario.log_files(dir, "log").pop().unwrap(); let log = std::fs::read_to_string(scenario.path(&format!("{dir}/{name}"))).unwrap(); assert_eq!(log.matches(warning).count(), 1); } diff --git a/tests/opencode_sessions.rs b/tests/opencode_sessions.rs index 756d497b..e5db8990 100644 --- a/tests/opencode_sessions.rs +++ b/tests/opencode_sessions.rs @@ -386,7 +386,7 @@ fn a_missing_skill_warns_in_the_progress_and_command_log_without_retrying() { assert_eq!(result.stderr.matches(warning).count(), 1); assert_eq!(records(&scenario, "json").len(), 1); let dir = "home/.thirdshift/logs/acme/widgets/commands/issue"; - let name = scenario.entries(dir).pop().unwrap(); + let name = scenario.log_files(dir, "log").pop().unwrap(); let log = std::fs::read_to_string(scenario.path(&format!("{dir}/{name}"))).unwrap(); assert_eq!(log.matches(warning).count(), 1); } diff --git a/tests/pickup_run.rs b/tests/pickup_run.rs index af6f9504..5955198d 100644 --- a/tests/pickup_run.rs +++ b/tests/pickup_run.rs @@ -950,12 +950,15 @@ fn the_notification_of_a_run_left_ready_for_review_is_that_runs() { subject, "[thirdshift] acme/widgets#7 Sharpen the widgets: ready for review" ); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files( + "home/.thirdshift/logs/acme/widgets/commands/pickup", + "jsonl", + ); assert_eq!(logs.len(), 1, "logs: {logs:?}"); let log = scenario - .path("home/.thirdshift/logs/acme/widgets/sessions") + .path("home/.thirdshift/logs/acme/widgets/commands/pickup") .join(&logs[0]); - let commands = scenario.entries("home/.thirdshift/logs/acme/widgets/commands/pickup"); + let commands = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/pickup", "log"); assert_eq!(commands.len(), 1, "Command logs: {commands:?}"); let command_log = scenario .path("home/.thirdshift/logs/acme/widgets/commands/pickup") diff --git a/tests/progress_lines.rs b/tests/progress_lines.rs index 48303a54..144a4e97 100644 --- a/tests/progress_lines.rs +++ b/tests/progress_lines.rs @@ -187,10 +187,10 @@ fn still_logs_the_whole_stream() { let result = scenario.run(&[&scenario.issue_url(7)]); assert_eq!(result.code, Some(0), "{}", result.stderr); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); let log = std::fs::read( scenario - .path("home/.thirdshift/logs/acme/widgets/sessions") + .path("home/.thirdshift/logs/acme/widgets/commands/issue") .join(&logs[0]), ) .unwrap(); @@ -228,8 +228,8 @@ for log in "{logs}/commands/issue/"*.log; do stamp=${{name#7-}} stamp=${{stamp%.log}} done -mkdir -p "{logs}/sessions" -ln -s "{pipe}" "{logs}/sessions/7-$stamp-implement.jsonl" +mkdir -p "{logs}/commands/issue" +ln -s "{pipe}" "{logs}/commands/issue/7-$stamp-implement.jsonl" touch "{ready}" "#, logs = logs.display(), diff --git a/tests/run_notification.rs b/tests/run_notification.rs index 8b6ee9e4..924758a7 100644 --- a/tests/run_notification.rs +++ b/tests/run_notification.rs @@ -57,10 +57,10 @@ fn text(request: &Request) -> &str { /// The path of the one session log the Run wrote. fn the_one_log(scenario: &Scenario) -> String { - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/sessions"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl"); assert_eq!(logs.len(), 1, "logs: {logs:?}"); let log = scenario - .path("home/.thirdshift/logs/acme/widgets/sessions") + .path("home/.thirdshift/logs/acme/widgets/commands/issue") .join(&logs[0]); log.to_str().unwrap().to_string() } diff --git a/tests/security_run.rs b/tests/security_run.rs index 87e75995..4473b7a3 100644 --- a/tests/security_run.rs +++ b/tests/security_run.rs @@ -52,7 +52,7 @@ exit 1"#, } fn security_command_log(scenario: &Scenario) -> String { - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/commands/secure"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/secure", "log"); fs::read_to_string(scenario.path(&format!( "home/.thirdshift/logs/acme/widgets/commands/secure/{}", logs[0] @@ -131,6 +131,26 @@ printf '%s\n' '{{"type":"assistant","message":{{"model":"claude-opus-4-8","conte let result = scenario.run(&["secure", "model", "opus"]); assert_eq!(result.code, Some(0), "{}", result.stderr); let command_log = security_command_log(&scenario); + let session_logs = scenario.log_files( + "home/.thirdshift/logs/acme/widgets/commands/secure", + "jsonl", + ); + assert_eq!(session_logs.len(), 2, "{session_logs:?}"); + assert!( + session_logs + .iter() + .any(|name| name.ends_with("-security-audit.jsonl")) + ); + assert!( + session_logs + .iter() + .any(|name| name.ends_with("-security-reproduction-1.jsonl")) + ); + assert!( + !scenario + .path("home/.thirdshift/logs/acme/widgets/sessions") + .exists() + ); for line in [ "security-audit: Model: claude-opus-5-5", "security-audit: Model: claude-opus-4-8", @@ -827,7 +847,7 @@ fn a_failed_fix_keeps_its_claim_and_pauses_security_and_pickup_until_closed() { assert!(resend.requests().is_empty()); assert_eq!( scenario - .entries("home/.thirdshift/logs/acme/widgets/commands/secure") + .log_files("home/.thirdshift/logs/acme/widgets/commands/secure", "log") .len(), 1 ); @@ -1578,11 +1598,11 @@ fn an_unchanged_base_skips_after_a_completed_audit_and_resumes_when_origin_moves let before = fs::read_to_string(scenario.path("home/.thirdshift/logs/acme/widgets/activity.log")) .unwrap(); - let commands = scenario.entries("home/.thirdshift/logs/acme/widgets/commands/secure"); + let commands = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/secure", "log"); assert_two_quiet_skips_send_no_email(&scenario); assert_eq!(scenario.claude_calls().len(), 1); assert_eq!( - scenario.entries("home/.thirdshift/logs/acme/widgets/commands/secure"), + scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/secure", "log"), commands ); let after = @@ -1980,7 +2000,7 @@ fn another_pass_holds_the_security_run_and_its_skip_is_logged() { assert_eq!(scenario.claude_calls().len(), 1); assert_eq!( scenario - .entries("home/.thirdshift/logs/acme/widgets/commands/secure") + .log_files("home/.thirdshift/logs/acme/widgets/commands/secure", "log") .len(), 1 ); @@ -2052,7 +2072,7 @@ fn missing_final_line_or_invalid_reports_fail_without_recording() { .unwrap(); assert!(log.contains("Security run started")); assert!(log.contains("Security run ended: failed:"), "{log}"); - let logs = scenario.entries("home/.thirdshift/logs/acme/widgets/commands/secure"); + let logs = scenario.log_files("home/.thirdshift/logs/acme/widgets/commands/secure", "log"); let command_log = fs::read_to_string(scenario.path(&format!( "home/.thirdshift/logs/acme/widgets/commands/secure/{}", logs[0] diff --git a/tests/setup.rs b/tests/setup.rs index 60b4aa13..c9170f9b 100644 --- a/tests/setup.rs +++ b/tests/setup.rs @@ -268,7 +268,7 @@ fn a_run_with_the_written_user_config_behaves_as_with_none() { assert_eq!(scenario.gh_state()["prs"][0]["state"], "OPEN"); assert_eq!( scenario - .entries("home/.thirdshift/logs/acme/widgets/sessions") + .log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl") .len(), 1 ); diff --git a/tests/support/mod.rs b/tests/support/mod.rs index a4313a2f..9aca6a4d 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -1268,6 +1268,18 @@ test -f {root}/{COPY_REPLACED} names } + /// Log file names in a command folder, excluding other log formats and reports. + pub fn log_files(&self, relative: &str, extension: &str) -> Vec { + self.entries(relative) + .into_iter() + .filter(|name| { + Path::new(name) + .extension() + .is_some_and(|ext| ext == extension) + }) + .collect() + } + /// Point the launch clone's origin at `url`, another spelling of the /// GitHub URL, which git also redirects to the bare repo. pub fn set_origin_url(&self, url: &str) { diff --git a/tests/user_config.rs b/tests/user_config.rs index 9bd68875..60398be1 100644 --- a/tests/user_config.rs +++ b/tests/user_config.rs @@ -148,13 +148,13 @@ fn help_and_version_succeed_with_a_broken_config() { /// The agent commits nothing and exits 3, so the Run fails after one session. const AGENT_EXITS_3: &str = "exit 3\n"; -/// Assert the Run failed after logging its one session in `sessions/` under +/// Assert the Run failed after logging its one session in `commands/issue/` under /// `acme/widgets/` under `dir`, a directory under the scenario, and that the "session log:" line /// names that log. fn assert_logged_in(scenario: &Scenario, result: &RunResult, dir: &str) { assert_eq!(result.code, Some(1), "stderr: {}", result.stderr); - let dir = &format!("{dir}/acme/widgets/sessions"); - let logs = scenario.entries(dir); + let dir = &format!("{dir}/acme/widgets/commands/issue"); + let logs = scenario.log_files(dir, "jsonl"); assert_eq!(logs.len(), 1, "logs: {logs:?}"); let log = scenario.path(dir).join(&logs[0]); assert!( From c1b87b322221c7813de5b468cff1755f102eb17f Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Fri, 9 Oct 2026 11:24:49 -0400 Subject: [PATCH 2/3] Share command-folder names across log paths --- src/logs.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/logs.rs b/src/logs.rs index 415e9f6b..7fc63a18 100644 --- a/src/logs.rs +++ b/src/logs.rs @@ -251,13 +251,15 @@ impl Work<'_> { /// `thirdshift architect` in `architect/`. fn command_log(self, root: &Path, stamp: &str) -> PathBuf { let (folder, prefix) = match self { - Work::Run(issue) | Work::SpecRun(issue) => ("issue", format!("{}-", issue.number)), - Work::PickupRun(issue) => ("pickup", format!("{}-", issue.number)), - Work::ArchitectRun(_) => ("architect", String::new()), - Work::SecurityRun(_) => ("secure", String::new()), + Work::Run(issue) | Work::SpecRun(issue) => { + (CommandKind::Issue, format!("{}-", issue.number)) + } + Work::PickupRun(issue) => (CommandKind::Pickup, format!("{}-", issue.number)), + Work::ArchitectRun(_) => (CommandKind::Architect, String::new()), + Work::SecurityRun(_) => (CommandKind::Secure, String::new()), }; root.join("commands") - .join(folder) + .join(folder.folder()) .join(format!("{prefix}{stamp}.log")) } } From c429388256a4c81e80ad66242ce687b2f791fdf6 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Fri, 9 Oct 2026 11:31:40 -0400 Subject: [PATCH 3/3] Check transcript absence in shared log folders --- tests/continuation.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/continuation.rs b/tests/continuation.rs index 0fdff7a9..5406f7c8 100644 --- a/tests/continuation.rs +++ b/tests/continuation.rs @@ -93,16 +93,16 @@ fn the_open_prs_base_is_the_base_branch_whatever_is_checked_out() { ); } -/// Nothing was created: no session, worktree, temp directory or log, and the +/// No session, worktree or temp directory was created, and the /// local Issue branch is still there. fn assert_nothing_created(scenario: &Scenario, local_branch: &str) { assert!(scenario.claude_calls().is_empty(), "claude was run"); assert_eq!(scenario.entries("work"), vec!["widgets"]); assert_eq!(scenario.entries("tmp"), Vec::::new()); assert!( - !scenario - .path("home/.thirdshift/logs/acme/widgets/commands/issue") - .exists(), + scenario + .log_files("home/.thirdshift/logs/acme/widgets/commands/issue", "jsonl") + .is_empty(), "a session log was created" ); assert_ne!(scenario.launch_git(&["branch", "--list", local_branch]), "");