From 3be168c7d2624a62d2659c48a31c4de5555e708b Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:27:45 -0700 Subject: [PATCH 01/12] fix(skills): drop CWD from implicit skill source roots tsq skills refresh probed the current working directory's SKILLS/ folder as a source root ahead of the trusted exe-relative and embedded copies. Running the command inside an untrusted repo shipping SKILLS/tasque/SKILL.md let that content overwrite managed skill installs in user-global agent directories (~/.claude/skills, ~/.codex, ~/.copilot, ~/.opencode). Remove the CWD source root; explicit sources (TSQ_SKILLS_DIR, source_root_dir) and trusted exe-relative/embedded copies remain unaffected. Adds a regression test proving a malicious SKILLS/tasque/SKILL.md in CWD never reaches a managed target. --- src/skills/mod.rs | 3 --- tests/skills_refresh.rs | 60 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/skills/mod.rs b/src/skills/mod.rs index 598e796..4558874 100644 --- a/src/skills/mod.rs +++ b/src/skills/mod.rs @@ -430,9 +430,6 @@ fn resolve_managed_skill_source_directory( if let Ok(dir) = env::var("TSQ_SKILLS_DIR") { roots.push(normalize_directory(PathBuf::from(dir), &default_home)?); } - if let Ok(cwd) = env::current_dir() { - roots.push(cwd.join("SKILLS")); - } if let Ok(exe_path) = env::current_exe() && let Some(exe_dir) = exe_path.parent() { diff --git a/tests/skills_refresh.rs b/tests/skills_refresh.rs index db88113..f8faa9c 100644 --- a/tests/skills_refresh.rs +++ b/tests/skills_refresh.rs @@ -100,3 +100,63 @@ fn skills_refresh_json_wires_env_and_creates_no_repo_state() { "skills refresh must not create .tasque in cwd" ); } + +/// Regression test for the CWD-as-skill-source injection vector: a +/// `SKILLS/tasque/SKILL.md` sitting in the process's current working +/// directory must never be treated as a trusted refresh source, even when +/// `TSQ_SKILLS_DIR` is unset. Refresh must fall back to the trusted +/// exe-relative / embedded source instead of the untrusted CWD payload. +#[cfg(not(target_os = "windows"))] +#[test] +fn skills_refresh_ignores_untrusted_cwd_skills_source() { + let temp_root = Builder::new() + .prefix("tsq-skills-refresh-cwd-trust-test-") + .tempdir() + .expect("temp dir"); + let temp_path = temp_root.path(); + + // Attacker-controlled CWD carrying a malicious managed-looking skill. + let attacker_cwd = temp_path.join("attacker-cwd"); + let attacker_skill_dir = attacker_cwd.join("SKILLS").join("tasque"); + fs::create_dir_all(&attacker_skill_dir).expect("create attacker skill dir"); + fs::write( + attacker_skill_dir.join("SKILL.md"), + "\n# Malicious Tasque Skill\nINJECTED-BY-CWD-TEST\n", + ) + .expect("write attacker SKILL.md"); + + // A pre-existing managed install at the Claude target that refresh + // should update from a trusted source only. + let home_dir = temp_path.join("home"); + let codex_home = temp_path.join("codex-home"); + let claude_skill_dir = home_dir.join(".claude").join("skills").join("tasque"); + fs::create_dir_all(&claude_skill_dir).expect("create claude skill dir"); + fs::write( + claude_skill_dir.join("SKILL.md"), + "\n# Old Tasque Skill\n", + ) + .expect("write pre-existing managed SKILL.md"); + fs::create_dir_all(&codex_home).expect("create codex home dir"); + + let output = Command::new(tsq_bin()) + .args(["skills", "refresh", "--json"]) + .current_dir(&attacker_cwd) + .env("HOME", &home_dir) + .env("USERPROFILE", &home_dir) + .env("CODEX_HOME", &codex_home) + .env_remove("TSQ_SKILLS_DIR") + .output() + .expect("failed executing tsq binary"); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + + let refreshed = fs::read_to_string(claude_skill_dir.join("SKILL.md")) + .expect("expected SKILL.md to still exist after refresh"); + assert!( + !refreshed.contains("INJECTED-BY-CWD-TEST"), + "untrusted CWD SKILLS/ payload must never reach a managed target\nstdout:\n{}\nstderr:\n{}\nrefreshed SKILL.md:\n{}", + stdout, + stderr, + refreshed + ); +} From 8c5183f2591c7f0c0c8f55bcb4fce84e3c500067 Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:33:42 -0700 Subject: [PATCH 02/12] fix(render): escape control sequences in human output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task titles, descriptions, notes, and spec content are attacker-influenceable and synced across machines via events.jsonl. Human-readable output printed them raw, so ANSI/OSC escape sequences (cursor moves, window-title rewrites, OSC-52 clipboard writes) would execute in the terminal of anyone running `tsq show`/`tsq find` on a synced repo. Add sanitize_inline/sanitize_block in render.rs, escaping ESC and all other C0/C1 control chars plus DEL as \x{:02x} (existing \\ \t \r \n handling preserved). plain_cell now delegates to sanitize_inline. Apply at every human-print site for title, description, note text, spec content, and assignee across render.rs, tui_render.rs, and watch.rs (tree/table/inspector/ board views). --json output is untouched — machine consumers still get the original bytes. --- src/cli/render.rs | 137 +++++++++++++++++++++++---- src/cli/tui_render.rs | 18 ++-- src/cli/watch.rs | 6 +- tests/render_sanitize.rs | 194 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 326 insertions(+), 29 deletions(-) create mode 100644 tests/render_sanitize.rs diff --git a/src/cli/render.rs b/src/cli/render.rs index bd34464..a62b6b4 100644 --- a/src/cli/render.rs +++ b/src/cli/render.rs @@ -12,12 +12,46 @@ pub struct TreeRenderOptions { const MAX_NARROW_TREE_PREFIX_WIDTH: usize = 24; +/// Escape C0/C1 control characters and DEL for safe terminal display. +/// `\\`, `\t`, `\r`, `\n` keep their existing readable escapes; ESC and every +/// other control character are rendered as `\x{:02x}`. All other characters +/// (including newlines' surrounding text, Unicode, emoji) pass through +/// unchanged. +pub(crate) fn sanitize_inline(value: &str) -> String { + sanitize(value, false) +} + +/// Same escaping as `sanitize_inline`, but preserves real newlines for +/// multi-line human-output contexts (spec content, note bodies). +pub(crate) fn sanitize_block(value: &str) -> String { + sanitize(value, true) +} + +fn sanitize(value: &str, keep_newlines: bool) -> String { + let mut out = String::with_capacity(value.len()); + for ch in value.chars() { + match ch { + '\\' => out.push_str("\\\\"), + '\t' => out.push_str("\\t"), + '\r' => out.push_str("\\r"), + '\n' if keep_newlines => out.push('\n'), + '\n' => out.push_str("\\n"), + c if is_unsafe_control(c) => { + out.push_str(&format!("\\x{:02x}", c as u32)); + } + c => out.push(c), + } + } + out +} + +fn is_unsafe_control(ch: char) -> bool { + let code = ch as u32; + code < 0x20 || code == 0x7f || (0x80..=0x9f).contains(&code) +} + fn plain_cell(value: &str) -> String { - value - .replace('\\', "\\\\") - .replace('\t', "\\t") - .replace('\r', "\\r") - .replace('\n', "\\n") + sanitize_inline(value) } pub fn print_task_list(tasks: &[Task]) { @@ -32,12 +66,15 @@ pub fn print_task_list(tasks: &[Task]) { .map(|task| { vec![ task.id.clone(), - task.alias.clone(), + sanitize_inline(&task.alias), task.priority.to_string(), task_kind_to_string(task.kind).to_string(), status_to_string(task.status).to_string(), - task.assignee.clone().unwrap_or_else(|| "-".to_string()), - task.title.clone(), + task.assignee + .as_deref() + .map(sanitize_inline) + .unwrap_or_else(|| "-".to_string()), + sanitize_inline(&task.title), ] }) .collect(); @@ -102,7 +139,12 @@ pub fn print_task_list_plain(tasks: &[Task]) { } pub fn print_task(task: &Task) { - println!("{} {} {}", style::task_id(&task.id), task.alias, task.title); + println!( + "{} {} {}", + style::task_id(&task.id), + sanitize_inline(&task.alias), + sanitize_inline(&task.title) + ); println!( "{}={} {}={} {}={}", style::key("kind"), @@ -120,7 +162,7 @@ pub fn print_task(task: &Task) { ); } if let Some(assignee) = &task.assignee { - println!("{}={}", style::key("assignee"), assignee); + println!("{}={}", style::key("assignee"), sanitize_inline(assignee)); } if let Some(external_ref) = &task.external_ref { println!("{}={}", style::key("external_ref"), external_ref); @@ -138,7 +180,11 @@ pub fn print_task(task: &Task) { println!("{}={}", style::key("duplicate_of"), duplicate_of); } if let Some(description) = &task.description { - println!("{}={}", style::key("description"), description); + println!( + "{}={}", + style::key("description"), + sanitize_inline(description) + ); } println!("{}={}", style::key("notes"), task.notes.len()); if let (Some(spec_path), Some(spec_fingerprint)) = (&task.spec_path, &task.spec_fingerprint) { @@ -271,8 +317,9 @@ pub fn print_show_result_plain(data: &ShowResult) { pub fn print_spec_content(data: &SpecContentResult) { println!("--- spec: {} ---", data.spec_path); - print!("{}", data.content); - if !data.content.ends_with('\n') { + let sanitized = sanitize_block(&data.content); + print!("{}", sanitized); + if !sanitized.ends_with('\n') { println!(); } println!("--- end spec ---"); @@ -389,11 +436,14 @@ fn render_tree_node( meta_width, ); if max_title_width > 0 { - primary_parts.push(truncate_with_ellipsis(&node.task.title, max_title_width)); + primary_parts.push(sanitize_inline(&truncate_with_ellipsis( + &node.task.title, + max_title_width, + ))); } } Density::Medium | Density::Wide => { - primary_parts.push(node.task.title.clone()); + primary_parts.push(sanitize_inline(&node.task.title)); primary_parts.push(meta); if density == Density::Wide && let Some(flow) = &flow @@ -495,7 +545,7 @@ pub fn print_merge_result(result: &MergeResult) { "{}={} \"{}\" [{}]", style::key("target"), style::task_id(&result.target.id), - result.target.title, + sanitize_inline(&result.target.title), result.target.status ); if let Some(summary) = &result.plan_summary { @@ -544,7 +594,7 @@ fn format_meta_badge_text(task: &Task) -> String { let assignee = task .assignee .as_ref() - .map(|value| format!(" @{}", value)) + .map(|value| format!(" @{}", sanitize_inline(value))) .unwrap_or_default(); format!("[p{}{}]", task.priority, assignee) } @@ -786,7 +836,7 @@ pub fn print_task_note(task_id: &str, note: &TaskNote) { note.actor, style::muted(¬e.event_id) ); - println!("{}", note.text); + println!("{}", sanitize_block(¬e.text)); } pub fn print_task_notes(task_id: &str, notes: &[TaskNote]) { @@ -808,7 +858,7 @@ pub fn print_task_notes(task_id: &str, notes: &[TaskNote]) { note.actor, style::muted(¬e.event_id) ); - println!("{}", note.text); + println!("{}", sanitize_block(¬e.text)); } } @@ -877,7 +927,7 @@ fn print_dep_node(node: &DepTreeNode, prefix: &str, is_last: bool, is_root: bool style::tree_prefix(connector), format_status(node.task.status), style::task_id(&node.task.id), - node.task.title, + sanitize_inline(&node.task.title), dir_tag, type_tag ); @@ -947,6 +997,53 @@ mod tests { use super::*; use crate::types::{PlanningState, TaskKind}; + #[test] + fn sanitize_inline_escapes_esc() { + assert_eq!(sanitize_inline("a\u{1b}b"), "a\\x1bb"); + } + + #[test] + fn sanitize_inline_escapes_osc_sequence_including_bel() { + // ESC ] 0 ; x BEL — an OSC window-title sequence. + let input = "evil\u{1b}]0;pwned\u{7}title"; + let expected = "evil\\x1b]0;pwned\\x07title"; + assert_eq!(sanitize_inline(input), expected); + } + + #[test] + fn sanitize_inline_escapes_c1_csi() { + // U+009B is the single-byte CSI introducer in the C1 control range. + assert_eq!(sanitize_inline("a\u{9b}b"), "a\\x9bb"); + } + + #[test] + fn sanitize_inline_escapes_del() { + assert_eq!(sanitize_inline("a\u{7f}b"), "a\\x7fb"); + } + + #[test] + fn sanitize_inline_escapes_backslash_tab_cr_lf() { + assert_eq!(sanitize_inline("a\\b\tc\rd\ne"), "a\\\\b\\tc\\rd\\ne"); + } + + #[test] + fn sanitize_inline_passes_through_plain_ascii_and_emoji() { + let input = "Fix auth redirect 🎉 — done!"; + assert_eq!(sanitize_inline(input), input); + } + + #[test] + fn sanitize_block_preserves_real_newlines_but_escapes_control_chars() { + let input = "line one\nline\u{1b}two\nline three"; + let expected = "line one\nline\\x1btwo\nline three"; + assert_eq!(sanitize_block(input), expected); + } + + #[test] + fn sanitize_block_still_escapes_backslash_tab_cr() { + assert_eq!(sanitize_block("a\\b\tc\rd"), "a\\\\b\\tc\\rd"); + } + #[test] fn narrow_tree_lines_fit_terminal_width_for_deep_hierarchies() { let width = 80; diff --git a/src/cli/tui_render.rs b/src/cli/tui_render.rs index 6b71200..7a7fcfb 100644 --- a/src/cli/tui_render.rs +++ b/src/cli/tui_render.rs @@ -1,4 +1,4 @@ -use crate::cli::render::truncate_with_ellipsis; +use crate::cli::render::{sanitize_inline, truncate_with_ellipsis}; use crate::cli::style; use crate::cli::terminal::resolve_width; use crate::output::{err_envelope, ok_envelope}; @@ -249,7 +249,7 @@ fn render_board_view(data: &TuiFrameData, width: usize) -> Vec { } fn render_board_card(task: &Task) -> String { - let title = truncate_with_ellipsis(&task.title, 18); + let title = sanitize_inline(&truncate_with_ellipsis(&task.title, 18)); format!( "{} {} {} {}", style::task_id(&task.id), @@ -283,7 +283,10 @@ fn render_inspector(data: &TuiFrameData, width: usize) -> Vec { lines.push(format!("id={}", style::task_id(&task.id))); lines.push(format!( "title={}", - truncate_with_ellipsis(&task.title, width.saturating_sub(8).max(12)) + sanitize_inline(&truncate_with_ellipsis( + &task.title, + width.saturating_sub(8).max(12) + )) )); lines.push(format!( "status={} kind={} priority={} planning={}", @@ -294,7 +297,10 @@ fn render_inspector(data: &TuiFrameData, width: usize) -> Vec { )); lines.push(format!( "assignee={} parent={} labels={}", - task.assignee.as_deref().unwrap_or("unassigned"), + task.assignee + .as_deref() + .map(sanitize_inline) + .unwrap_or_else(|| "unassigned".to_string()), task.parent_id.as_deref().unwrap_or("-"), labels )); @@ -406,9 +412,9 @@ fn render_table_row(task: &Task, selected: bool, title_width: usize) -> String { marker, task.id, type_pill(task.kind), - truncate_with_ellipsis(&task.title, title_width), + sanitize_inline(&truncate_with_ellipsis(&task.title, title_width)), status_pill(task.status), - truncate_with_ellipsis(assignee, 12), + sanitize_inline(&truncate_with_ellipsis(assignee, 12)), priority_pill(task.priority), spec_pill(task), ) diff --git a/src/cli/watch.rs b/src/cli/watch.rs index 936a719..b789837 100644 --- a/src/cli/watch.rs +++ b/src/cli/watch.rs @@ -2,7 +2,7 @@ use crate::app::service::TasqueService; use crate::app::service_types::ListFilter; use crate::cli::render::{ TreeRenderOptions, format_meta_badge, format_status, format_status_text, render_task_tree, - status_to_string, truncate_with_ellipsis, + sanitize_inline, status_to_string, truncate_with_ellipsis, }; use crate::cli::style; use crate::cli::terminal::{Density, resolve_density, resolve_width}; @@ -467,7 +467,7 @@ fn render_flat_tasks(tasks: &[Task], width: usize) -> Vec { "{} {} {}", status, style::task_id(&task.id), - truncate_with_ellipsis(&task.title, title_width) + sanitize_inline(&truncate_with_ellipsis(&task.title, title_width)) )); lines.push(format!(" {}", meta)); } else { @@ -475,7 +475,7 @@ fn render_flat_tasks(tasks: &[Task], width: usize) -> Vec { "{} {} {} {}", status, style::task_id(&task.id), - task.title, + sanitize_inline(&task.title), meta )); } diff --git a/tests/render_sanitize.rs b/tests/render_sanitize.rs new file mode 100644 index 0000000..61a7934 --- /dev/null +++ b/tests/render_sanitize.rs @@ -0,0 +1,194 @@ +mod common; + +use common::{create_task_with_args, init_repo, run_cli, run_json}; +use serde_json::Value; + +// ESC ] 0 ; pwned BEL — an OSC window-title rewrite sequence, the canonical +// terminal-injection payload. `\u{1b}` is ESC (0x1b), `\u{7}` is BEL (0x07). +const EVIL_TITLE: &str = "evil\u{1b}]0;pwned\u{7}title"; +const ESC_BYTE: u8 = 0x1b; + +fn assert_no_raw_esc(output: &str) { + // Piped stdout from these commands must never contain a literal, + // unescaped ESC byte — style.rs disables ANSI color when stdout is not + // a TTY (see `style::use_color`), so any ESC byte here must have come + // from user-controlled task data, not from styling. + assert!( + !output.as_bytes().contains(&ESC_BYTE), + "human output contained a raw ESC byte: {:?}", + output + ); + assert!( + output.contains("\\x1b"), + "expected the escaped form \\x1b to be present: {:?}", + output + ); +} + +#[test] +fn human_show_output_escapes_control_sequences_in_title() { + let repo = common::make_repo(); + init_repo(repo.path()); + let id = create_task_with_args(repo.path(), EVIL_TITLE, &["--force"]); + + let result = run_cli(repo.path(), ["show", &id]); + + assert_eq!(result.code, 0, "stderr:\n{}", result.stderr); + assert_no_raw_esc(&result.stdout); +} + +#[test] +fn json_show_output_preserves_raw_control_sequences_in_title() { + let repo = common::make_repo(); + init_repo(repo.path()); + let id = create_task_with_args(repo.path(), EVIL_TITLE, &["--force"]); + + let result = run_json(repo.path(), ["show", &id]); + + assert_eq!(result.cli.code, 0, "stderr:\n{}", result.cli.stderr); + let title = result + .envelope + .get("data") + .and_then(|data| data.get("task")) + .and_then(|task| task.get("title")) + .and_then(Value::as_str) + .expect("expected data.task.title"); + assert_eq!( + title, EVIL_TITLE, + "--json must preserve the original bytes for machine consumers" + ); +} + +#[test] +fn human_find_output_escapes_control_sequences_in_title() { + let repo = common::make_repo(); + init_repo(repo.path()); + create_task_with_args(repo.path(), EVIL_TITLE, &["--force"]); + + let result = run_cli(repo.path(), ["find", "open"]); + + assert_eq!(result.code, 0, "stderr:\n{}", result.stderr); + assert_no_raw_esc(&result.stdout); +} + +#[test] +fn human_show_output_escapes_control_sequences_in_description() { + let repo = common::make_repo(); + init_repo(repo.path()); + let evil_description = format!("body{}", EVIL_TITLE); + let id = create_task_with_args( + repo.path(), + "Task with evil description", + &["--force", "--description", &evil_description], + ); + + let result = run_cli(repo.path(), ["show", &id]); + + assert_eq!(result.code, 0, "stderr:\n{}", result.stderr); + assert_no_raw_esc(&result.stdout); +} + +#[test] +fn json_show_output_preserves_raw_control_sequences_in_description() { + let repo = common::make_repo(); + init_repo(repo.path()); + let evil_description = format!("body{}", EVIL_TITLE); + let id = create_task_with_args( + repo.path(), + "Task with evil description", + &["--force", "--description", &evil_description], + ); + + let result = run_json(repo.path(), ["show", &id]); + + assert_eq!(result.cli.code, 0, "stderr:\n{}", result.cli.stderr); + let description = result + .envelope + .get("data") + .and_then(|data| data.get("task")) + .and_then(|task| task.get("description")) + .and_then(Value::as_str) + .expect("expected data.task.description"); + assert_eq!(description, evil_description); +} + +#[test] +fn human_note_output_escapes_control_sequences() { + let repo = common::make_repo(); + init_repo(repo.path()); + let id = create_task_with_args(repo.path(), "Task for note test", &["--force"]); + let evil_note = format!("note{}", EVIL_TITLE); + + let result = run_cli(repo.path(), ["note", &id, &evil_note]); + + assert_eq!(result.code, 0, "stderr:\n{}", result.stderr); + assert_no_raw_esc(&result.stdout); + + let list_result = run_cli(repo.path(), ["notes", &id]); + assert_eq!(list_result.code, 0, "stderr:\n{}", list_result.stderr); + assert_no_raw_esc(&list_result.stdout); +} + +#[test] +fn json_note_output_preserves_raw_control_sequences() { + let repo = common::make_repo(); + init_repo(repo.path()); + let id = create_task_with_args(repo.path(), "Task for note json test", &["--force"]); + let evil_note = format!("note{}", EVIL_TITLE); + + let result = run_json(repo.path(), ["note", &id, &evil_note]); + + assert_eq!(result.cli.code, 0, "stderr:\n{}", result.cli.stderr); + let text = result + .envelope + .get("data") + .and_then(|data| data.get("note")) + .and_then(|note| note.get("text")) + .and_then(Value::as_str) + .expect("expected data.note.text"); + assert_eq!(text, evil_note); +} + +#[test] +fn human_spec_show_output_escapes_control_sequences() { + let repo = common::make_repo(); + init_repo(repo.path()); + let id = create_task_with_args(repo.path(), "Task for spec test", &["--force"]); + let evil_spec = format!("# Context\nbody{}\nmore text", EVIL_TITLE); + + let attach = run_cli(repo.path(), ["spec", &id, "--text", &evil_spec]); + assert_eq!(attach.code, 0, "stderr:\n{}", attach.stderr); + + let show = run_cli(repo.path(), ["spec", &id, "--show"]); + assert_eq!(show.code, 0, "stderr:\n{}", show.stderr); + assert_no_raw_esc(&show.stdout); + // Real newlines from the spec body must still render as actual line + // breaks, not be collapsed into a single escaped line. + assert!( + show.stdout.contains("# Context\n"), + "expected real newlines to be preserved in spec content:\n{}", + show.stdout + ); +} + +#[test] +fn json_spec_content_preserves_raw_control_sequences() { + let repo = common::make_repo(); + init_repo(repo.path()); + let id = create_task_with_args(repo.path(), "Task for spec json test", &["--force"]); + let evil_spec = format!("# Context\nbody{}\nmore text", EVIL_TITLE); + + let attach = run_cli(repo.path(), ["spec", &id, "--text", &evil_spec]); + assert_eq!(attach.code, 0, "stderr:\n{}", attach.stderr); + + let show = run_json(repo.path(), ["spec", &id, "--show"]); + assert_eq!(show.cli.code, 0, "stderr:\n{}", show.cli.stderr); + let content = show + .envelope + .get("data") + .and_then(|data| data.get("spec")) + .and_then(|spec| spec.get("content")) + .and_then(Value::as_str) + .expect("expected data.spec.content"); + assert_eq!(content, evil_spec); +} From d716cc3c3cf83d1994943e2a72d186a0ea4c0944 Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:29:02 -0700 Subject: [PATCH 03/12] fix(sync): configure merge driver when materializing sync worktree The tasque-events merge driver definition lives only in local git config, written during initial setup_sync_branch/migration. Git config does not travel with clones, so on a freshly cloned machine the cold ensure_worktree path in resolve_effective_root and the explicit tsq sync path never configured it, letting git fall back to a default textual 3-way merge on events.jsonl and corrupting the reader. Configure the driver after ensure_worktree succeeds on the cold resolve path, and re-ensure it in sync_worktree before commit/push so clones that materialized their worktree on a pre-fix binary self-heal. --- src/app/sync.rs | 7 +++- tests/sync_branch.rs | 94 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/app/sync.rs b/src/app/sync.rs index 10f3b64..bf854eb 100644 --- a/src/app/sync.rs +++ b/src/app/sync.rs @@ -57,7 +57,11 @@ pub fn resolve_effective_root(repo_root: &str) -> Result { )); } - let worktree = with_setup_lock(repo_root, || git::ensure_worktree(repo_path, &branch))?; + let worktree = with_setup_lock(repo_root, || { + let wt = git::ensure_worktree(repo_path, &branch)?; + git::setup_merge_driver_config(repo_path)?; + Ok(wt) + })?; Ok(worktree.to_string_lossy().to_string()) } @@ -196,6 +200,7 @@ pub fn sync_worktree(repo_root: &str, push: bool) -> Result String { + let output = git_output(repo, args); + String::from_utf8_lossy(&output.stdout).trim().to_string() +} + +fn create_bare_origin(base: &std::path::Path) -> std::path::PathBuf { + let remote = base.join("origin.git"); + let remote_arg = remote.to_string_lossy().to_string(); + git(base, &["init", "--bare", remote_arg.as_str()]); + git(&remote, &["symbolic-ref", "HEAD", "refs/heads/main"]); + remote +} + #[test] fn sync_branch_requires_git_repo() { let repo = make_repo(); @@ -29,3 +42,82 @@ fn sync_branch_requires_git_repo() { .and_then(Value::as_str); assert_eq!(code, Some("GIT_NOT_AVAILABLE")); } + +const EXPECTED_MERGE_DRIVER: &str = "tsq merge-driver %O %A %B"; + +/// Fresh clones only receive the sync branch and its committed +/// `.gitattributes` (`merge=tasque-events`) via git; the merge driver +/// *definition* lives in local git config and does not travel with a clone. +/// Materializing the sync worktree on a clone (via any data command) must +/// configure the driver so `git merge`/`git pull` on a diverged sync branch +/// invokes `tsq merge-driver` instead of falling back to a default textual +/// 3-way merge on JSONL. +#[test] +fn clone_materializes_worktree_and_configures_merge_driver() { + let repo = make_repo(); + let base = repo.path(); + let source = base.join("source"); + fs::create_dir(&source).expect("source dir"); + + init_git_repo_with_identity(&source, Some("main")); + + let init = run_cli(&source, ["init"]); + assert_eq!(init.code, 0, "stderr: {}", init.stderr); + let create = run_cli(&source, ["create", "Driver clone task"]); + assert_eq!(create.code, 0, "stderr: {}", create.stderr); + + git(&source, &["add", ".tasque/config.json", ".gitattributes"]); + git(&source, &["commit", "-m", "seed main config"]); + + let remote = create_bare_origin(base); + let remote_arg = remote.to_string_lossy().to_string(); + git(&source, &["remote", "add", "origin", remote_arg.as_str()]); + git(&source, &["push", "origin", "HEAD:main"]); + git(&source, &["push", "origin", "tsq-sync"]); + + let clone = base.join("clone"); + let clone_arg = clone.to_string_lossy().to_string(); + git(base, &["clone", remote_arg.as_str(), clone_arg.as_str()]); + + // Sanity: a fresh clone has no local merge driver config at all. + let pre_check = git_output(&clone, &["config", "--get", "merge.tasque-events.driver"]); + assert!( + !pre_check.status.success(), + "expected no merge driver config before the worktree materializes" + ); + + // First data command in the clone materializes the sync worktree + // (cold `ensure_worktree` path in `resolve_effective_root`). + let list = run_cli(&clone, ["find", "open", "--json"]); + assert_eq!(list.code, 0, "stderr: {}", list.stderr); + assert!( + list.stdout.contains("Driver clone task"), + "expected cloned repo to read remote sync task:\n{}", + list.stdout + ); + + let driver = git_out(&clone, &["config", "--get", "merge.tasque-events.driver"]); + assert_eq!( + driver, EXPECTED_MERGE_DRIVER, + "expected merge driver to be configured after worktree materializes" + ); + + // `tsq sync` heals a clone whose worktree was materialized on a + // pre-fix binary: manually unset the driver, then verify `tsq sync` + // restores it. + git(&clone, &["config", "--unset", "merge.tasque-events.driver"]); + let missing = git_output(&clone, &["config", "--get", "merge.tasque-events.driver"]); + assert!( + !missing.status.success(), + "expected merge driver config to be unset before sync" + ); + + let sync_result = run_cli(&clone, ["sync", "--no-push"]); + assert_eq!(sync_result.code, 0, "stderr: {}", sync_result.stderr); + + let restored = git_out(&clone, &["config", "--get", "merge.tasque-events.driver"]); + assert_eq!( + restored, EXPECTED_MERGE_DRIVER, + "expected `tsq sync` to re-ensure the merge driver config" + ); +} From 720c1004f2b291843c2d26a2e0926546348fe0b7 Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:38:01 -0700 Subject: [PATCH 04/12] fix(sync): make implicit migration push best-effort and clear events after local commit Implicit sync-branch migration (triggered by any data command in a legacy repo) pushed to origin with hard-fail propagation, so read-only commands like `tsq find open` failed outright when offline or when push was rejected. And clear_repo_events ran only after a successful push, leaving stale main-tree events behind on push failure with no automatic retry. Clear main-tree events right after the local worktree commit (data is durable locally at that point), and add PushMode: the implicit resolve path uses BestEffort (push failure becomes a stderr warning suggesting 'tsq sync'), while explicit `tsq migrate` keeps Required hard-fail semantics. `tsq sync` push behavior is unchanged. --- src/app/service.rs | 7 ++- src/app/sync.rs | 42 +++++++++++++-- tests/sync_branch.rs | 119 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 6 deletions(-) diff --git a/src/app/service.rs b/src/app/service.rs index 4468cc0..b7e5457 100644 --- a/src/app/service.rs +++ b/src/app/service.rs @@ -321,7 +321,12 @@ impl TasqueService { } pub fn migrate(&self, branch: &str) -> Result { - crate::app::sync::migrate_to_sync_branch(&self.ctx.repo_root, branch, &self.ctx.actor) + crate::app::sync::migrate_to_sync_branch( + &self.ctx.repo_root, + branch, + &self.ctx.actor, + crate::app::sync::PushMode::Required, + ) } pub fn sync(&self, push: bool) -> Result { diff --git a/src/app/sync.rs b/src/app/sync.rs index bf854eb..36f48e2 100644 --- a/src/app/sync.rs +++ b/src/app/sync.rs @@ -35,7 +35,12 @@ pub fn resolve_effective_root(repo_root: &str) -> Result { None => { let repo_path = Path::new(repo_root); if git::is_git_repo(repo_path) && !git::is_sync_worktree_path(repo_path) { - let migrated = migrate_to_sync_branch(repo_root, DEFAULT_SYNC_BRANCH, "tsq")?; + let migrated = migrate_to_sync_branch( + repo_root, + DEFAULT_SYNC_BRANCH, + "tsq", + PushMode::BestEffort, + )?; return Ok(migrated.worktree_path); } return Ok(repo_root.to_string()); @@ -130,6 +135,15 @@ fn setup_sync_branch_locked(repo_root: &str, branch: &str) -> Result Result { let existing = read_events(repo_root)?; @@ -169,10 +184,22 @@ pub fn migrate_to_sync_branch( let wt_path = Path::new(&setup.worktree_path); let _ = git::commit_worktree(wt_path, "chore: migrate tasque events to sync branch")?; + clear_repo_events(repo_root)?; if let Some(remote) = git::current_upstream_remote(Path::new(repo_root))? { - git::push_current_set_upstream(wt_path, &remote, branch)?; + match push { + PushMode::Required => { + git::push_current_set_upstream(wt_path, &remote, branch)?; + } + PushMode::BestEffort => { + if let Err(error) = git::push_current_set_upstream(wt_path, &remote, branch) { + eprintln!( + "tsq: warning: migrated events to sync branch but push to '{remote}' failed: {}; run 'tsq sync' to push later", + error.message + ); + } + } + } } - clear_repo_events(repo_root)?; Ok(MigrateResult { events_migrated: to_append.len(), @@ -692,8 +719,13 @@ mod tests { ]; append_events(repo, &events).expect("append_events"); - let result = migrate_to_sync_branch(&repo.to_string_lossy(), DEFAULT_SYNC_BRANCH, "test") - .expect("migrate"); + let result = migrate_to_sync_branch( + &repo.to_string_lossy(), + DEFAULT_SYNC_BRANCH, + "test", + PushMode::Required, + ) + .expect("migrate"); assert_eq!(result.events_migrated, 0); let migrated = read_events(&result.worktree_path).expect("read_events"); diff --git a/tests/sync_branch.rs b/tests/sync_branch.rs index 0fd4b22..1573644 100644 --- a/tests/sync_branch.rs +++ b/tests/sync_branch.rs @@ -121,3 +121,122 @@ fn clone_materializes_worktree_and_configures_merge_driver() { "expected `tsq sync` to re-ensure the merge driver config" ); } + +/// Build a legacy repo: main-tree `.tasque` data (no `sync_branch` in +/// config), then git-init it and point `origin` at a nonexistent path so +/// any push fails. +fn make_legacy_repo_with_unreachable_origin( + base: &std::path::Path, + title: &str, +) -> std::path::PathBuf { + let root = base.join("repo"); + fs::create_dir(&root).expect("repo dir"); + + let init = run_cli(&root, ["init"]); + assert_eq!(init.code, 0, "stderr: {}", init.stderr); + let create = run_cli(&root, ["create", title]); + assert_eq!(create.code, 0, "stderr: {}", create.stderr); + + let config = fs::read_to_string(root.join(".tasque").join("config.json")).expect("config"); + assert!( + !config.contains("\"sync_branch\": \""), + "expected legacy config without sync_branch:\n{}", + config + ); + + init_git_repo_with_identity(&root, Some("main")); + let missing_remote = base.join("missing-origin.git"); + let missing_remote_arg = missing_remote.to_string_lossy().to_string(); + git( + &root, + &["remote", "add", "origin", missing_remote_arg.as_str()], + ); + + root +} + +/// Implicit migration (any data command in a legacy repo) must not hard-fail +/// a read path when the push to origin fails: push is best-effort with a +/// stderr warning, and the main-tree events are cleared once the local +/// worktree commit succeeds. +#[test] +fn implicit_migration_survives_unreachable_origin_with_warning() { + let repo = make_repo(); + let base = repo.path(); + let root = make_legacy_repo_with_unreachable_origin(base, "Offline implicit task"); + + let list = run_cli(&root, ["find", "open", "--json"]); + assert_eq!( + list.code, 0, + "expected read command to succeed despite failed push\nstdout:\n{}\nstderr:\n{}", + list.stdout, list.stderr + ); + let envelope: Value = serde_json::from_str(list.stdout.trim()).expect("json envelope"); + assert_eq!(envelope.get("ok").and_then(Value::as_bool), Some(true)); + assert!( + list.stdout.contains("Offline implicit task"), + "expected migrated task in output:\n{}", + list.stdout + ); + assert!( + list.stderr + .contains("tsq: warning: migrated events to sync branch but push to 'origin' failed"), + "expected best-effort push warning on stderr:\n{}", + list.stderr + ); + + let root_events = + fs::read_to_string(root.join(".tasque").join("events.jsonl")).expect("root events"); + assert!( + root_events.is_empty(), + "expected main-tree events cleared after local migration" + ); + let sync_events = fs::read_to_string( + root.join(".git") + .join("tsq-sync") + .join(".tasque") + .join("events.jsonl"), + ) + .expect("sync worktree events"); + assert!( + !sync_events.is_empty(), + "expected events present in sync worktree" + ); +} + +/// Explicit `tsq migrate` keeps push failures fatal, but the events must +/// already be migrated and the main tree cleared (local commit lands before +/// the push), so a later `tsq sync` can complete the push. +#[test] +fn explicit_migrate_fails_on_unreachable_origin_after_local_migration() { + let repo = make_repo(); + let base = repo.path(); + let root = make_legacy_repo_with_unreachable_origin(base, "Offline explicit task"); + + let migrate = run_cli(&root, ["migrate", "--json"]); + assert_ne!( + migrate.code, 0, + "expected explicit migrate to fail when push fails\nstdout:\n{}\nstderr:\n{}", + migrate.stdout, migrate.stderr + ); + let envelope: Value = serde_json::from_str(migrate.stdout.trim()).expect("json envelope"); + assert_eq!(envelope.get("ok").and_then(Value::as_bool), Some(false)); + + let root_events = + fs::read_to_string(root.join(".tasque").join("events.jsonl")).expect("root events"); + assert!( + root_events.is_empty(), + "expected main-tree events cleared even when push fails" + ); + let sync_events = fs::read_to_string( + root.join(".git") + .join("tsq-sync") + .join(".tasque") + .join("events.jsonl"), + ) + .expect("sync worktree events"); + assert!( + !sync_events.is_empty(), + "expected events migrated locally despite failed push" + ); +} From 2a29593e29ee489aca9d93c10e43791e51269548 Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:32:18 -0700 Subject: [PATCH 05/12] fix(tui): fetch tasks via watch --once and dep trees via deps verb --- tui-opentui/src/data.ts | 54 +++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/tui-opentui/src/data.ts b/tui-opentui/src/data.ts index c751aa5..30d1c25 100644 --- a/tui-opentui/src/data.ts +++ b/tui-opentui/src/data.ts @@ -33,7 +33,7 @@ export interface DependencyNode { children: DependencyNode[]; } -const DEFAULT_STATUS = "open,in_progress"; +const DEFAULT_STATUS = "open,in_progress,blocked,deferred,closed,canceled"; export function readConfigFromEnv(): TuiConfig { const intervalRaw = process.env.TSQ_TUI_INTERVAL ?? "2"; @@ -57,7 +57,7 @@ export function readConfigFromEnv(): TuiConfig { } export function fetchTasks(config: TuiConfig): DataSnapshot { - const args = ["--json", "list"]; + const args = ["--json", "watch", "--once", "--status", config.statusCsv]; if (config.assignee) { args.push("--assignee", config.assignee); } @@ -79,31 +79,36 @@ export function fetchTasks(config: TuiConfig): DataSnapshot { }; } - const stdout = bytesToString(subprocess.stdout); + const parsed = parseTasksEnvelope(bytesToString(subprocess.stdout)); + return { + fetchedAt, + tasks: parsed.tasks, + warning: parsed.warning, + }; +} +export function parseTasksEnvelope(stdout: string): { + tasks: TasqueTask[]; + warning?: string; +} { let payload: ListEnvelope; try { payload = JSON.parse(stdout) as ListEnvelope; } catch { return { - fetchedAt, tasks: [], - warning: "Unable to parse JSON output from tsq list", + warning: "Unable to parse JSON output from tsq watch --once", }; } if (!payload.ok) { return { - fetchedAt, tasks: [], - warning: payload.error?.message ?? "tsq list returned an error", + warning: payload.error?.message ?? "tsq watch returned an error", }; } - return { - fetchedAt, - tasks: payload.data?.tasks ?? [], - }; + return { tasks: payload.data?.tasks ?? [] }; } function bytesToString(buffer: Uint8Array): string { @@ -126,17 +131,7 @@ export function fetchDependencyTree( taskId: string, ): { root?: DependencyNode; warning?: string } { const subprocess = Bun.spawnSync( - [ - tsqBin, - "--json", - "dep", - "tree", - taskId, - "--direction", - "both", - "--depth", - "4", - ], + [tsqBin, "--json", "deps", taskId, "--direction", "both", "--depth", "4"], { stdin: "ignore", stdout: "pipe", @@ -147,19 +142,26 @@ export function fetchDependencyTree( if (subprocess.exitCode !== 0) { const stderr = bytesToString(subprocess.stderr).trim(); return { - warning: stderr || `Failed to run ${tsqBin} dep tree ${taskId}`, + warning: stderr || `Failed to run ${tsqBin} deps ${taskId}`, }; } + return parseDependencyEnvelope(bytesToString(subprocess.stdout)); +} + +export function parseDependencyEnvelope(stdout: string): { + root?: DependencyNode; + warning?: string; +} { let payload: DepEnvelope; try { - payload = JSON.parse(bytesToString(subprocess.stdout)) as DepEnvelope; + payload = JSON.parse(stdout) as DepEnvelope; } catch { - return { warning: "Unable to parse JSON output from tsq dep tree" }; + return { warning: "Unable to parse JSON output from tsq deps" }; } if (!payload.ok) { - return { warning: payload.error?.message ?? "tsq dep tree returned an error" }; + return { warning: payload.error?.message ?? "tsq deps returned an error" }; } const root = normalizeDependencyNode(payload.data?.root); From 881eb28301a0a1d85249eef8464c5c2451f9fb0a Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:32:18 -0700 Subject: [PATCH 06/12] fix(tui): load specs via tsq spec --show and list all epics --- tui-opentui/src/model.ts | 48 ++++++++++----------- tui-opentui/src/tui-app.tsx | 37 +++++------------ tui-opentui/src/tui-helpers.ts | 76 +++++++++++++++++++++++++++------- tui-opentui/src/tui-views.tsx | 70 ++++++++----------------------- 4 files changed, 111 insertions(+), 120 deletions(-) diff --git a/tui-opentui/src/model.ts b/tui-opentui/src/model.ts index 0bae35a..3807b5c 100644 --- a/tui-opentui/src/model.ts +++ b/tui-opentui/src/model.ts @@ -156,34 +156,32 @@ export function boardColumns( return lanes; } -export function buildEpicProgress(tasks: TasqueTask[]): EpicProgress | null { +export function buildEpicProgressList(tasks: TasqueTask[]): EpicProgress[] { const epics = tasks.filter((task) => task.kind === "epic"); - const epic = epics.at(0); - if (!epic) { - return null; - } - const children = tasks.filter((task) => task.parent_id === epic.id); - - let done = 0; - let open = 0; - let inProgress = 0; - for (const child of children) { - if (child.status === "closed" || child.status === "canceled") { - done += 1; - } else if (child.status === "in_progress" || child.status === "blocked") { - inProgress += 1; - } else { - open += 1; + return epics.map((epic) => { + const children = tasks.filter((task) => task.parent_id === epic.id); + + let done = 0; + let open = 0; + let inProgress = 0; + for (const child of children) { + if (child.status === "closed" || child.status === "canceled") { + done += 1; + } else if (child.status === "in_progress" || child.status === "blocked") { + inProgress += 1; + } else { + open += 1; + } } - } - return { - epic, - children, - done, - open, - inProgress, - }; + return { + epic, + children, + done, + open, + inProgress, + }; + }); } export function normalizeTab(value: string | undefined): TabKey { diff --git a/tui-opentui/src/tui-app.tsx b/tui-opentui/src/tui-app.tsx index 8ee868a..3108aad 100644 --- a/tui-opentui/src/tui-app.tsx +++ b/tui-opentui/src/tui-app.tsx @@ -11,7 +11,7 @@ import { type TabKey, type TasqueTask, boardColumns, - buildEpicProgress, + buildEpicProgressList, computeSummary, sortTasks, specState, @@ -81,18 +81,15 @@ export function App() { [activeFilter?.statuses, allTasks], ); const summary = useMemo(() => computeSummary(filteredTasks), [filteredTasks]); - const epicProgress = useMemo(() => buildEpicProgress(filteredTasks), [filteredTasks]); + const epicProgressList = useMemo(() => buildEpicProgressList(filteredTasks), [filteredTasks]); const board = useMemo(() => boardColumns(filteredTasks), [filteredTasks]); const visibleTasks = useMemo(() => { if (tab === "epics") { - if (!epicProgress) { - return [] as TasqueTask[]; - } - return epicProgress.children.length > 0 ? epicProgress.children : [epicProgress.epic]; + return epicProgressList.map((progress) => progress.epic); } return filteredTasks; - }, [tab, filteredTasks, epicProgress]); + }, [tab, filteredTasks, epicProgressList]); const treeLines = useMemo(() => buildTreeLines(filteredTasks), [filteredTasks]); const selectedIndex = selectedByTab[tab]; @@ -161,28 +158,15 @@ export function App() { if (specState(task) !== "attached" || !task.spec_path) { return; } - const specPath = task.spec_path; + const result = readSpecLines(config.tsqBin, task.id); setSpecDialog({ taskId: task.id, taskTitle: task.title, - specPath, - lines: ["Loading spec..."], + specPath: task.spec_path, + lines: result.lines, + warning: result.warning, offset: 0, - loading: true, - }); - void readSpecLines(specPath).then((result) => { - setSpecDialog((current) => { - if (!current || current.taskId !== task.id || current.specPath !== specPath) { - return current; - } - return { - ...current, - lines: result.lines, - warning: result.warning, - offset: 0, - loading: false, - }; - }); + loading: false, }); }; @@ -412,9 +396,8 @@ export function App() { {tab === "epics" ? ( ) : null} diff --git a/tui-opentui/src/tui-helpers.ts b/tui-opentui/src/tui-helpers.ts index bb7c17b..b927891 100644 --- a/tui-opentui/src/tui-helpers.ts +++ b/tui-opentui/src/tui-helpers.ts @@ -122,30 +122,76 @@ function buildAncestorPrefix(siblingTrail: boolean[]): string { .join(""); } -export async function readSpecLines( - specPath: string, -): Promise<{ lines: string[]; warning?: string }> { +export function readSpecLines( + tsqBin: string, + taskId: string, +): { lines: string[]; warning?: string } { + const subprocess = Bun.spawnSync([tsqBin, "--json", "spec", taskId, "--show"], { + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + + if (subprocess.exitCode !== 0) { + const stderr = new TextDecoder().decode(subprocess.stderr).trim(); + return { + lines: [], + warning: stderr || `Failed to run ${tsqBin} spec ${taskId} --show`, + }; + } + + return parseSpecEnvelope(new TextDecoder().decode(subprocess.stdout)); +} + +export function parseSpecEnvelope(stdout: string): { + lines: string[]; + warning?: string; +} { + let payload: unknown; try { - const text = await Bun.file(specPath).text(); - if (text.length === 0) { - return { lines: ["(empty spec)"] }; - } + payload = JSON.parse(stdout); + } catch { return { - lines: text.replaceAll("\r\n", "\n").split("\n"), + lines: [], + warning: "Unable to parse JSON output from tsq spec --show", }; - } catch (error) { + } + + if (!payload || typeof payload !== "object") { + return { lines: [], warning: "Unexpected payload from tsq spec --show" }; + } + const envelope = payload as Record; + + if (envelope.ok !== true) { + const error = + envelope.error && typeof envelope.error === "object" + ? (envelope.error as Record) + : undefined; + const message = + typeof error?.message === "string" ? error.message : undefined; return { lines: [], - warning: `Failed to open spec: ${errorMessage(error)}`, + warning: message ?? "tsq spec --show returned an error", }; } -} -function errorMessage(error: unknown): string { - if (error instanceof Error && error.message) { - return error.message; + const data = + envelope.data && typeof envelope.data === "object" + ? (envelope.data as Record) + : undefined; + const spec = + data?.spec && typeof data.spec === "object" + ? (data.spec as Record) + : undefined; + const content = typeof spec?.content === "string" ? spec.content : undefined; + if (content === undefined) { + return { lines: [], warning: "Spec payload missing content" }; + } + + if (content.length === 0) { + return { lines: ["(empty spec)"] }; } - return "unknown error"; + return { lines: content.replaceAll("\r\n", "\n").split("\n") }; } export function buildFilterPresets(statusCsv: string): FilterPreset[] { diff --git a/tui-opentui/src/tui-views.tsx b/tui-opentui/src/tui-views.tsx index 4b96061..f512a3f 100644 --- a/tui-opentui/src/tui-views.tsx +++ b/tui-opentui/src/tui-views.tsx @@ -14,7 +14,6 @@ import { } from "./model"; import { THEME, - buildTableLayout, buildTreePrefix, flattenDependencyTree, formatUpdatedAt, @@ -24,10 +23,9 @@ import { renderMeter, specLabel, statusIcon, - tableHeader, treeDisplayId, } from "./tui-helpers"; -import type { SelectedByLane, SpecDialogState, TableLayout, TreeLine } from "./tui-types"; +import type { SelectedByLane, SpecDialogState, TreeLine } from "./tui-types"; export function TabChip({ tab, @@ -48,36 +46,34 @@ export function TabChip({ } export function EpicsView({ - tasks, + progressList, selectedTaskId, - epicProgress, width, }: { - tasks: TasqueTask[]; + progressList: EpicProgress[]; selectedTaskId?: string; - epicProgress: EpicProgress | null; width: number; }) { - const layout = useMemo(() => buildTableLayout(width), [width]); + const titleWidth = Math.max(16, width - 48); return ( - {epicProgress ? ( - + {progressList.map(({ epic, children, done, inProgress, open }) => ( + - {epicProgress.epic.id} - {titleWithEllipsis(epicProgress.epic.title, 48)} - {renderMeter(epicProgress.done, epicProgress.children.length, 12)} - {epicProgress.done}/{epicProgress.children.length} + {statusIcon(epic.status)} + {epic.id} + {titleWithEllipsis(epic.title, titleWidth)} + {renderMeter(done, children.length, 12)} + {done}/{children.length} +
+ done {done} | in_progress {inProgress} | open {open} - ) : null} - - {tableHeader(layout)} - - {tasks.map((task) => ( - ))} - {tasks.length === 0 ? ( + {progressList.length === 0 ? ( No epic tasks found @@ -86,38 +82,6 @@ export function EpicsView({ ); } -function TaskRow({ - task, - selected, - layout, -}: { - task: TasqueTask; - selected: boolean; - layout: TableLayout; -}) { - const spec = specState(task); - return ( - - - {statusIcon(task.status)} - {pad(task.id, layout.idWidth)} - {pad(kindLabel(task.kind), layout.typeWidth)} - - {pad(titleWithEllipsis(task.title, layout.titleWidth), layout.titleWidth)}{" "} - - - {pad(`P${task.priority}`, layout.priorityWidth)} - {layout.showSpec ? ( - <> - - - - ) : null} - - - ); -} - export function BoardView({ tasks, lane, From e7f3bc28a527be4cafbae21e4a34d9ab5a75b5db Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:34:31 -0700 Subject: [PATCH 07/12] test(tui): add bun test suite with real-binary CLI contract test --- .github/workflows/ci.yml | 9 ++ tui-opentui/package.json | 3 +- tui-opentui/tests/contract.test.ts | 86 ++++++++++++++++ tui-opentui/tests/data.test.ts | 151 +++++++++++++++++++++++++++++ tui-opentui/tests/model.test.ts | 126 ++++++++++++++++++++++++ 5 files changed, 374 insertions(+), 1 deletion(-) create mode 100644 tui-opentui/tests/contract.test.ts create mode 100644 tui-opentui/tests/data.test.ts create mode 100644 tui-opentui/tests/model.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5b4ed4a..1859cd5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,6 +42,15 @@ jobs: - run: bun run typecheck working-directory: tui-opentui + - uses: dtolnay/rust-toolchain@stable + + - run: cargo build --quiet --bin tsq + + - run: bun test + working-directory: tui-opentui + env: + TSQ_CONTRACT_BIN: ${{ github.workspace }}/target/debug/tsq + - uses: actions/setup-node@v4 with: node-version: "24" diff --git a/tui-opentui/package.json b/tui-opentui/package.json index f8027d3..e9fc242 100644 --- a/tui-opentui/package.json +++ b/tui-opentui/package.json @@ -6,7 +6,8 @@ "scripts": { "dev": "bun run --watch src/index.tsx", "start": "bun run src/index.tsx", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "test": "bun test" }, "devDependencies": { "@types/bun": "latest", diff --git a/tui-opentui/tests/contract.test.ts b/tui-opentui/tests/contract.test.ts new file mode 100644 index 0000000..a269386 --- /dev/null +++ b/tui-opentui/tests/contract.test.ts @@ -0,0 +1,86 @@ +import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { type TuiConfig, fetchDependencyTree, fetchTasks } from "../src/data"; +import { readSpecLines } from "../src/tui-helpers"; + +const bin = process.env.TSQ_CONTRACT_BIN; + +if (!bin) { + console.log( + "contract.test.ts: TSQ_CONTRACT_BIN not set; skipping real-binary contract tests", + ); +} + +const describeContract = bin ? describe : describe.skip; + +describeContract("tsq CLI contract", () => { + let dir = ""; + let previousCwd = ""; + + const run = (args: string[]): string => { + const subprocess = Bun.spawnSync([bin as string, ...args], { + cwd: dir, + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + if (subprocess.exitCode !== 0) { + const stderr = new TextDecoder().decode(subprocess.stderr); + throw new Error(`tsq ${args.join(" ")} failed: ${stderr}`); + } + return new TextDecoder().decode(subprocess.stdout); + }; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "tsq-contract-")); + previousCwd = process.cwd(); + run(["init", "--no-wizard"]); + run(["create", "epic demo", "--kind", "epic", "--force"]); // tsq-1 + run(["create", "child", "--parent", "tsq-1", "--force"]); // tsq-1.1 + run(["create", "blocker", "--force"]); // tsq-2 + run(["block", "tsq-1.1", "by", "tsq-2"]); + run(["spec", "tsq-1.1", "--text", "# hello contract"]); + // fetchTasks and friends spawn tsq without an explicit cwd, so run the + // suite from inside the initialized repo, mirroring the TUI launcher. + process.chdir(dir); + }); + + afterAll(() => { + if (previousCwd) { + process.chdir(previousCwd); + } + if (dir) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("fetchTasks returns tasks via watch --once", () => { + const config: TuiConfig = { + intervalSeconds: 2, + statusCsv: "open,in_progress,blocked,deferred,closed,canceled", + initialTab: "tasks", + tsqBin: bin as string, + }; + const snapshot = fetchTasks(config); + expect(snapshot.warning).toBeUndefined(); + const ids = snapshot.tasks.map((task) => task.id); + expect(ids).toContain("tsq-1"); + expect(ids).toContain("tsq-1.1"); + expect(ids).toContain("tsq-2"); + }); + + it("fetchDependencyTree returns the root via the deps verb", () => { + const result = fetchDependencyTree(bin as string, "tsq-1.1"); + expect(result.warning).toBeUndefined(); + expect(result.root?.id).toBe("tsq-1.1"); + expect(result.root?.children.length).toBeGreaterThan(0); + }); + + it("readSpecLines returns spec content via spec --show", () => { + const result = readSpecLines(bin as string, "tsq-1.1"); + expect(result.warning).toBeUndefined(); + expect(result.lines[0]).toBe("# hello contract"); + }); +}); diff --git a/tui-opentui/tests/data.test.ts b/tui-opentui/tests/data.test.ts new file mode 100644 index 0000000..7985742 --- /dev/null +++ b/tui-opentui/tests/data.test.ts @@ -0,0 +1,151 @@ +import { describe, expect, it } from "bun:test"; +import { parseDependencyEnvelope, parseTasksEnvelope } from "../src/data"; +import { parseSpecEnvelope } from "../src/tui-helpers"; + +const sampleTask = { + id: "tsq-1", + kind: "task", + title: "demo", + status: "open", + priority: 2, + labels: [], + created_at: "2026-01-01T00:00:00.000Z", + updated_at: "2026-01-01T00:00:00.000Z", +}; + +describe("parseTasksEnvelope", () => { + it("returns tasks from a valid watch envelope", () => { + const stdout = JSON.stringify({ + ok: true, + data: { frame_ts: "2026-01-01T00:00:00.000Z", tasks: [sampleTask] }, + }); + const result = parseTasksEnvelope(stdout); + expect(result.warning).toBeUndefined(); + expect(result.tasks).toHaveLength(1); + expect(result.tasks[0]?.id).toBe("tsq-1"); + }); + + it("surfaces the error message from an ok:false envelope", () => { + const stdout = JSON.stringify({ + ok: false, + error: { code: "VALIDATION_ERROR", message: "bad filter" }, + }); + const result = parseTasksEnvelope(stdout); + expect(result.tasks).toEqual([]); + expect(result.warning).toBe("bad filter"); + }); + + it("warns on malformed JSON", () => { + const result = parseTasksEnvelope("not json"); + expect(result.tasks).toEqual([]); + expect(result.warning).toBe( + "Unable to parse JSON output from tsq watch --once", + ); + }); + + it("returns an empty task list when data is missing", () => { + const result = parseTasksEnvelope(JSON.stringify({ ok: true })); + expect(result.tasks).toEqual([]); + expect(result.warning).toBeUndefined(); + }); +}); + +describe("parseDependencyEnvelope", () => { + it("returns the root node from a valid deps envelope", () => { + const stdout = JSON.stringify({ + ok: true, + data: { + root: { + id: "tsq-1", + task: sampleTask, + children: [ + { + id: "tsq-2", + dep_type: "blocks", + direction: "outgoing", + children: [], + }, + ], + }, + }, + }); + const result = parseDependencyEnvelope(stdout); + expect(result.warning).toBeUndefined(); + expect(result.root?.id).toBe("tsq-1"); + expect(result.root?.task?.title).toBe("demo"); + expect(result.root?.children).toHaveLength(1); + expect(result.root?.children[0]?.depType).toBe("blocks"); + }); + + it("surfaces the error message from an ok:false envelope", () => { + const stdout = JSON.stringify({ + ok: false, + error: { message: "task not found" }, + }); + expect(parseDependencyEnvelope(stdout).warning).toBe("task not found"); + }); + + it("warns on malformed JSON", () => { + expect(parseDependencyEnvelope("{oops").warning).toBe( + "Unable to parse JSON output from tsq deps", + ); + }); + + it("warns when the root node is missing", () => { + const result = parseDependencyEnvelope( + JSON.stringify({ ok: true, data: {} }), + ); + expect(result.root).toBeUndefined(); + expect(result.warning).toBe("Dependency tree payload missing root node"); + }); +}); + +describe("parseSpecEnvelope", () => { + it("splits data.spec.content into lines", () => { + const stdout = JSON.stringify({ + ok: true, + data: { + spec: { + path: ".tasque/specs/tsq-1/spec.md", + fingerprint: "abc123", + content: "# hello\r\nworld", + }, + }, + }); + const result = parseSpecEnvelope(stdout); + expect(result.warning).toBeUndefined(); + expect(result.lines).toEqual(["# hello", "world"]); + }); + + it("returns a placeholder for empty content", () => { + const stdout = JSON.stringify({ + ok: true, + data: { spec: { path: "p", fingerprint: "f", content: "" } }, + }); + expect(parseSpecEnvelope(stdout).lines).toEqual(["(empty spec)"]); + }); + + it("surfaces the error message from an ok:false envelope", () => { + const stdout = JSON.stringify({ + ok: false, + error: { message: "no spec attached" }, + }); + const result = parseSpecEnvelope(stdout); + expect(result.lines).toEqual([]); + expect(result.warning).toBe("no spec attached"); + }); + + it("warns on malformed JSON", () => { + expect(parseSpecEnvelope("").warning).toBe( + "Unable to parse JSON output from tsq spec --show", + ); + }); + + it("warns when data.spec.content is missing", () => { + const result = parseSpecEnvelope( + JSON.stringify({ ok: true, data: { spec: { path: "p" } } }), + ); + expect(result.lines).toEqual([]); + expect(result.warning).toBe("Spec payload missing content"); + }); +}); diff --git a/tui-opentui/tests/model.test.ts b/tui-opentui/tests/model.test.ts new file mode 100644 index 0000000..a248176 --- /dev/null +++ b/tui-opentui/tests/model.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it } from "bun:test"; +import { + type TasqueTask, + buildEpicProgressList, + computeSummary, + sortTasks, +} from "../src/model"; + +function makeTask(overrides: Partial & { id: string }): TasqueTask { + return { + kind: "task", + title: overrides.id, + status: "open", + priority: 2, + labels: [], + created_at: "2026-01-01T00:00:00.000Z", + updated_at: "2026-01-01T00:00:00.000Z", + ...overrides, + }; +} + +describe("buildEpicProgressList", () => { + it("returns an empty list when there are no epics", () => { + const tasks = [makeTask({ id: "tsq-1" }), makeTask({ id: "tsq-2" })]; + expect(buildEpicProgressList(tasks)).toEqual([]); + }); + + it("counts closed and canceled children as done for a single epic", () => { + const tasks = [ + makeTask({ id: "tsq-1", kind: "epic" }), + makeTask({ id: "tsq-1.1", parent_id: "tsq-1", status: "closed" }), + makeTask({ id: "tsq-1.2", parent_id: "tsq-1", status: "canceled" }), + makeTask({ id: "tsq-1.3", parent_id: "tsq-1", status: "in_progress" }), + makeTask({ id: "tsq-1.4", parent_id: "tsq-1", status: "blocked" }), + makeTask({ id: "tsq-1.5", parent_id: "tsq-1", status: "open" }), + makeTask({ id: "tsq-1.6", parent_id: "tsq-1", status: "deferred" }), + ]; + + const progress = buildEpicProgressList(tasks); + expect(progress).toHaveLength(1); + expect(progress[0]?.epic.id).toBe("tsq-1"); + expect(progress[0]?.children).toHaveLength(6); + expect(progress[0]?.done).toBe(2); + expect(progress[0]?.inProgress).toBe(2); + expect(progress[0]?.open).toBe(2); + }); + + it("returns one entry per epic with its own children", () => { + const tasks = [ + makeTask({ id: "tsq-1", kind: "epic" }), + makeTask({ id: "tsq-2", kind: "epic" }), + makeTask({ id: "tsq-3", kind: "epic" }), + makeTask({ id: "tsq-1.1", parent_id: "tsq-1", status: "closed" }), + makeTask({ id: "tsq-2.1", parent_id: "tsq-2", status: "open" }), + makeTask({ id: "tsq-2.2", parent_id: "tsq-2", status: "closed" }), + ]; + + const progress = buildEpicProgressList(tasks); + expect(progress.map((entry) => entry.epic.id)).toEqual([ + "tsq-1", + "tsq-2", + "tsq-3", + ]); + expect(progress[0]?.done).toBe(1); + expect(progress[1]?.children).toHaveLength(2); + expect(progress[1]?.done).toBe(1); + expect(progress[1]?.open).toBe(1); + expect(progress[2]?.children).toHaveLength(0); + expect(progress[2]?.done).toBe(0); + }); +}); + +describe("sortTasks", () => { + it("orders by status, then priority, then created_at", () => { + const tasks = [ + makeTask({ id: "tsq-1", status: "open", priority: 1 }), + makeTask({ id: "tsq-2", status: "in_progress", priority: 3 }), + makeTask({ + id: "tsq-3", + status: "open", + priority: 1, + created_at: "2025-01-01T00:00:00.000Z", + }), + makeTask({ id: "tsq-4", status: "closed" }), + ]; + + const sorted = sortTasks(tasks).map((task) => task.id); + expect(sorted).toEqual(["tsq-2", "tsq-3", "tsq-1", "tsq-4"]); + }); + + it("does not mutate the input array", () => { + const tasks = [ + makeTask({ id: "tsq-1", status: "closed" }), + makeTask({ id: "tsq-2", status: "in_progress" }), + ]; + sortTasks(tasks); + expect(tasks[0]?.id).toBe("tsq-1"); + }); +}); + +describe("computeSummary", () => { + it("counts open, in_progress, and blocked tasks", () => { + const tasks = [ + makeTask({ id: "tsq-1", status: "open" }), + makeTask({ id: "tsq-2", status: "in_progress" }), + makeTask({ id: "tsq-3", status: "blocked" }), + makeTask({ id: "tsq-4", status: "closed" }), + ]; + + expect(computeSummary(tasks)).toEqual({ + total: 4, + open: 1, + inProgress: 1, + blocked: 1, + }); + }); + + it("returns zeroes for an empty list", () => { + expect(computeSummary([])).toEqual({ + total: 0, + open: 0, + inProgress: 0, + blocked: 0, + }); + }); +}); From 5fda5b9fa934535c5d423aae3f566e9db67a463e Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:35:07 -0700 Subject: [PATCH 08/12] docs(tui): rewrite README for CLI data source and real keybindings --- tui-opentui/README.md | 70 +++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/tui-opentui/README.md b/tui-opentui/README.md index 7ee1a0a..9616b73 100644 --- a/tui-opentui/README.md +++ b/tui-opentui/README.md @@ -1,38 +1,70 @@ # Tasque OpenTUI (Read-Only) -OpenTUI React app for Tasque with tabbed views: -- `Tasks` -- `Epics` -- `Board` +OpenTUI React app for Tasque with four tabs: +- `Tasks`: task tree with planning/spec/updated metadata +- `Epics`: one progress row per epic (done/in-progress/open counts) +- `Board`: open / in-progress / done lanes +- `Deps`: dependency tree of the selected task The app is read-only. It does not mutate Tasque data. -## Data sources (fallback order) -1. `tsq-rs list --json` -2. `tsq list --json` -3. `.tasque/state.json` -4. `.tasque/events.jsonl` +## Data source + +There is a single data source: the TUI spawns the `tsq` CLI with `--json` and +parses the standard envelope. There is no fallback to `.tasque/state.json` or +`.tasque/events.jsonl`. + +Commands used: +- `tsq --json watch --once --status [--assignee ]` — task list (every refresh) +- `tsq --json deps --direction both --depth 4` — dependency tree (Deps tab) +- `tsq --json spec --show` — spec content (spec dialog) + +## Environment variables + +- `TSQ_TUI_BIN`: path to the `tsq` binary (default: `tsq` on `PATH`) +- `TSQ_TUI_INTERVAL`: refresh interval in seconds, clamped to 1-60 (default: `2`) +- `TSQ_TUI_STATUS`: status CSV passed to `watch --once` and used for the first + filter preset (default: `open,in_progress,blocked,deferred,closed,canceled`; + tabs filter client-side, so narrowing this hides tasks from Board/Epics too) +- `TSQ_TUI_ASSIGNEE`: filter tasks by assignee +- `TSQ_TUI_VIEW`: initial tab; one of `tasks`, `epics`, `board`, `deps` (default: `tasks`) ## Spec state -Every row/card/details pane includes spec state: -- `attached` -- `missing` -- `invalid` -When metadata exists, spec state is validated against file existence and fingerprint. +Every row/card includes spec state derived from task metadata: +- `attached`: `spec_path` and `spec_fingerprint` both present +- `missing`: neither present +- `invalid`: only one present ## Run + ```bash cd tui-opentui bun install bun run start ``` +## Test + +```bash +bun test # unit tests; contract test skips +TSQ_CONTRACT_BIN=/abs/path/to/tsq bun test # also runs real-binary contract test +``` + ## Keyboard -- `q` or `Esc`: quit (uses `renderer.destroy()`) -- `Ctrl+C`: quit (uses `renderer.destroy()`) -- `Tab` / `Left` / `Right`: switch tabs + +- `q` or `Esc`: quit +- `Ctrl+C`: quit +- `Tab`: next tab +- `1` / `2` / `3` / `4`: jump to Tasks / Epics / Board / Deps - `Up` / `Down` or `j` / `k`: move selection -- `h` / `l`: switch board lane (Board tab) +- `h` / `l` or `Left` / `Right`: switch board lane (Board tab only) +- `f`: cycle filter preset +- `Enter`: open spec dialog for the selected task (when a spec is attached) - `r`: refresh now -- `p`: pause/resume auto-refresh + +Spec dialog: +- `Esc` / `Enter` / `q`: close +- `j` / `k` or `Up` / `Down`: scroll +- `PgUp` / `PgDn`: page +- `Home` / `End`: jump to start/end From 15f207628e714dcbef79f02554ddd39ef6a2cd0f Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:47:42 -0700 Subject: [PATCH 09/12] perf(tui): move tsq subprocess calls off the render thread --- tui-opentui/src/data.ts | 123 ++++++++++++++++++++++------- tui-opentui/src/tui-app.tsx | 86 +++++++++++++++----- tui-opentui/src/tui-helpers.ts | 25 +++--- tui-opentui/tests/contract.test.ts | 12 +-- 4 files changed, 180 insertions(+), 66 deletions(-) diff --git a/tui-opentui/src/data.ts b/tui-opentui/src/data.ts index 30d1c25..71add23 100644 --- a/tui-opentui/src/data.ts +++ b/tui-opentui/src/data.ts @@ -56,30 +56,64 @@ export function readConfigFromEnv(): TuiConfig { }; } -export function fetchTasks(config: TuiConfig): DataSnapshot { - const args = ["--json", "watch", "--once", "--status", config.statusCsv]; - if (config.assignee) { - args.push("--assignee", config.assignee); - } +export interface TsqSpawnResult { + exitCode: number; + stdout: string; + stderr: string; +} - const subprocess = Bun.spawnSync([config.tsqBin, ...args], { +export async function runTsq(argv: string[]): Promise { + const subprocess = Bun.spawn(argv, { stdin: "ignore", stdout: "pipe", stderr: "pipe", }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(subprocess.stdout).text(), + new Response(subprocess.stderr).text(), + subprocess.exited, + ]); + + return { exitCode, stdout, stderr }; +} + +export function spawnWarning(bin: string, error: unknown): string { + const message = + error instanceof Error && error.message ? error.message : "unknown error"; + return `Failed to run ${bin}: ${message}`; +} + +export async function fetchTasks(config: TuiConfig): Promise { + const args = ["--json", "watch", "--once", "--status", config.statusCsv]; + if (config.assignee) { + args.push("--assignee", config.assignee); + } + + let result: TsqSpawnResult; + try { + result = await runTsq([config.tsqBin, ...args]); + } catch (error) { + return { + fetchedAt: new Date().toISOString(), + tasks: [], + warning: spawnWarning(config.tsqBin, error), + }; + } + const fetchedAt = new Date().toISOString(); - if (subprocess.exitCode !== 0) { - const stderr = bytesToString(subprocess.stderr).trim(); + if (result.exitCode !== 0) { return { fetchedAt, tasks: [], - warning: stderr || `Failed to run ${config.tsqBin} ${args.join(" ")}`, + warning: + result.stderr.trim() || + `Failed to run ${config.tsqBin} ${args.join(" ")}`, }; } - const parsed = parseTasksEnvelope(bytesToString(subprocess.stdout)); + const parsed = parseTasksEnvelope(result.stdout); return { fetchedAt, tasks: parsed.tasks, @@ -111,10 +145,6 @@ export function parseTasksEnvelope(stdout: string): { return { tasks: payload.data?.tasks ?? [] }; } -function bytesToString(buffer: Uint8Array): string { - return new TextDecoder().decode(buffer); -} - interface DepEnvelope { ok: boolean; data?: { @@ -126,27 +156,62 @@ interface DepEnvelope { }; } -export function fetchDependencyTree( +export async function fetchDependencyTree( tsqBin: string, taskId: string, -): { root?: DependencyNode; warning?: string } { - const subprocess = Bun.spawnSync( - [tsqBin, "--json", "deps", taskId, "--direction", "both", "--depth", "4"], - { - stdin: "ignore", - stdout: "pipe", - stderr: "pipe", - }, - ); - - if (subprocess.exitCode !== 0) { - const stderr = bytesToString(subprocess.stderr).trim(); +): Promise<{ root?: DependencyNode; warning?: string }> { + let result: TsqSpawnResult; + try { + result = await runTsq([ + tsqBin, + "--json", + "deps", + taskId, + "--direction", + "both", + "--depth", + "4", + ]); + } catch (error) { + return { warning: spawnWarning(tsqBin, error) }; + } + + if (result.exitCode !== 0) { return { - warning: stderr || `Failed to run ${tsqBin} deps ${taskId}`, + warning: result.stderr.trim() || `Failed to run ${tsqBin} deps ${taskId}`, }; } - return parseDependencyEnvelope(bytesToString(subprocess.stdout)); + return parseDependencyEnvelope(result.stdout); +} + +export function createInFlightGuard( + fetcher: () => Promise, +): () => Promise { + let inFlight = false; + return async () => { + if (inFlight) { + return undefined; + } + inFlight = true; + try { + return await fetcher(); + } finally { + inFlight = false; + } + }; +} + +export function createLatestGuard(): ( + fetcher: () => Promise, +) => Promise { + let sequence = 0; + return async (fetcher: () => Promise) => { + sequence += 1; + const ticket = sequence; + const result = await fetcher(); + return ticket === sequence ? result : undefined; + }; } export function parseDependencyEnvelope(stdout: string): { diff --git a/tui-opentui/src/tui-app.tsx b/tui-opentui/src/tui-app.tsx index 3108aad..a417c61 100644 --- a/tui-opentui/src/tui-app.tsx +++ b/tui-opentui/src/tui-app.tsx @@ -1,7 +1,10 @@ import { useKeyboard, useRenderer, useTerminalDimensions } from "@opentui/react"; import { useEffect, useMemo, useState } from "react"; import { + type DataSnapshot, type DependencyNode, + createInFlightGuard, + createLatestGuard, fetchDependencyTree, fetchTasks, readConfigFromEnv, @@ -38,8 +41,11 @@ export function App() { const dimensions = useTerminalDimensions(); const [tab, setTab] = useState(config.initialTab); - const [snapshot, setSnapshot] = useState(() => fetchTasks(config)); - const [warning, setWarning] = useState(snapshot.warning); + const [snapshot, setSnapshot] = useState({ + fetchedAt: "-", + tasks: [], + }); + const [warning, setWarning] = useState(); const [selectedByTab, setSelectedByTab] = useState({ tasks: 0, epics: 0, @@ -63,15 +69,27 @@ export function App() { }, [filterPresets.length]); useEffect(() => { - const refresh = () => { - const next = fetchTasks(config); + let cancelled = false; + const guardedFetch = createInFlightGuard(() => fetchTasks(config)); + + const refresh = async () => { + const next = await guardedFetch(); + if (cancelled || !next) { + // Skipped poll (previous fetch still in flight) or effect torn down. + return; + } setSnapshot(next); setWarning(next.warning); }; - refresh(); - const timer = setInterval(refresh, config.intervalSeconds * 1000); - return () => clearInterval(timer); + void refresh(); + const timer = setInterval(() => { + void refresh(); + }, config.intervalSeconds * 1000); + return () => { + cancelled = true; + clearInterval(timer); + }; }, [config]); const allTasks = useMemo(() => sortTasks(snapshot.tasks), [snapshot.tasks]); @@ -122,19 +140,35 @@ export function App() { const tableRowBudget = Math.max(4, dimensions.height - 18); const specDialogBodyRows = Math.max(6, dimensions.height - 21); + const latestDependencyFetch = useMemo(() => createLatestGuard(), []); + useEffect(() => { if (tab !== "deps") { return; } - if (!selectedTask?.id) { + const taskId = selectedTask?.id; + if (!taskId) { setDependencyRoot(undefined); setDependencyWarning(undefined); return; } - const dependency = fetchDependencyTree(config.tsqBin, selectedTask.id); - setDependencyRoot(dependency.root); - setDependencyWarning(dependency.warning); - }, [config.tsqBin, selectedTask?.id, tab]); + // Debounce rapid selection changes; the latest-guard discards responses + // that resolve after a newer fetch has been issued. + const timer = setTimeout(() => { + void latestDependencyFetch(() => + fetchDependencyTree(config.tsqBin, taskId), + ).then((dependency) => { + if (!dependency) { + return; + } + setDependencyRoot(dependency.root); + setDependencyWarning(dependency.warning); + }); + }, 150); + return () => { + clearTimeout(timer); + }; + }, [config.tsqBin, latestDependencyFetch, selectedTask?.id, tab]); useEffect(() => { if (rowCount === 0 || tab === "board") { @@ -158,15 +192,28 @@ export function App() { if (specState(task) !== "attached" || !task.spec_path) { return; } - const result = readSpecLines(config.tsqBin, task.id); setSpecDialog({ taskId: task.id, taskTitle: task.title, specPath: task.spec_path, - lines: result.lines, - warning: result.warning, + lines: ["Loading spec..."], offset: 0, - loading: false, + loading: true, + }); + void readSpecLines(config.tsqBin, task.id).then((result) => { + setSpecDialog((current) => { + // Stale check: apply only if the dialog is still open for this task. + if (!current || current.taskId !== task.id) { + return current; + } + return { + ...current, + lines: result.lines, + warning: result.warning, + offset: 0, + loading: false, + }; + }); }); }; @@ -221,9 +268,10 @@ export function App() { } if (key.name === "r") { - const next = fetchTasks(config); - setSnapshot(next); - setWarning(next.warning); + void fetchTasks(config).then((next) => { + setSnapshot(next); + setWarning(next.warning); + }); return; } diff --git a/tui-opentui/src/tui-helpers.ts b/tui-opentui/src/tui-helpers.ts index b927891..855559c 100644 --- a/tui-opentui/src/tui-helpers.ts +++ b/tui-opentui/src/tui-helpers.ts @@ -1,4 +1,4 @@ -import type { DependencyNode } from "./data"; +import { type DependencyNode, type TsqSpawnResult, runTsq, spawnWarning } from "./data"; import { TAB_ORDER, TASK_STATUS_ORDER, @@ -122,25 +122,26 @@ function buildAncestorPrefix(siblingTrail: boolean[]): string { .join(""); } -export function readSpecLines( +export async function readSpecLines( tsqBin: string, taskId: string, -): { lines: string[]; warning?: string } { - const subprocess = Bun.spawnSync([tsqBin, "--json", "spec", taskId, "--show"], { - stdin: "ignore", - stdout: "pipe", - stderr: "pipe", - }); +): Promise<{ lines: string[]; warning?: string }> { + let result: TsqSpawnResult; + try { + result = await runTsq([tsqBin, "--json", "spec", taskId, "--show"]); + } catch (error) { + return { lines: [], warning: spawnWarning(tsqBin, error) }; + } - if (subprocess.exitCode !== 0) { - const stderr = new TextDecoder().decode(subprocess.stderr).trim(); + if (result.exitCode !== 0) { return { lines: [], - warning: stderr || `Failed to run ${tsqBin} spec ${taskId} --show`, + warning: + result.stderr.trim() || `Failed to run ${tsqBin} spec ${taskId} --show`, }; } - return parseSpecEnvelope(new TextDecoder().decode(subprocess.stdout)); + return parseSpecEnvelope(result.stdout); } export function parseSpecEnvelope(stdout: string): { diff --git a/tui-opentui/tests/contract.test.ts b/tui-opentui/tests/contract.test.ts index a269386..0000ed1 100644 --- a/tui-opentui/tests/contract.test.ts +++ b/tui-opentui/tests/contract.test.ts @@ -56,14 +56,14 @@ describeContract("tsq CLI contract", () => { } }); - it("fetchTasks returns tasks via watch --once", () => { + it("fetchTasks returns tasks via watch --once", async () => { const config: TuiConfig = { intervalSeconds: 2, statusCsv: "open,in_progress,blocked,deferred,closed,canceled", initialTab: "tasks", tsqBin: bin as string, }; - const snapshot = fetchTasks(config); + const snapshot = await fetchTasks(config); expect(snapshot.warning).toBeUndefined(); const ids = snapshot.tasks.map((task) => task.id); expect(ids).toContain("tsq-1"); @@ -71,15 +71,15 @@ describeContract("tsq CLI contract", () => { expect(ids).toContain("tsq-2"); }); - it("fetchDependencyTree returns the root via the deps verb", () => { - const result = fetchDependencyTree(bin as string, "tsq-1.1"); + it("fetchDependencyTree returns the root via the deps verb", async () => { + const result = await fetchDependencyTree(bin as string, "tsq-1.1"); expect(result.warning).toBeUndefined(); expect(result.root?.id).toBe("tsq-1.1"); expect(result.root?.children.length).toBeGreaterThan(0); }); - it("readSpecLines returns spec content via spec --show", () => { - const result = readSpecLines(bin as string, "tsq-1.1"); + it("readSpecLines returns spec content via spec --show", async () => { + const result = await readSpecLines(bin as string, "tsq-1.1"); expect(result.warning).toBeUndefined(); expect(result.lines[0]).toBe("# hello contract"); }); From 2971c11776e3eb7351645df295d3663037f09479 Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 07:47:42 -0700 Subject: [PATCH 10/12] test(tui): cover in-flight and stale-response guards --- tui-opentui/tests/async-guards.test.ts | 79 ++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 tui-opentui/tests/async-guards.test.ts diff --git a/tui-opentui/tests/async-guards.test.ts b/tui-opentui/tests/async-guards.test.ts new file mode 100644 index 0000000..8fb31cf --- /dev/null +++ b/tui-opentui/tests/async-guards.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "bun:test"; +import { createInFlightGuard, createLatestGuard } from "../src/data"; + +function deferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve!: (value: T) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +} + +describe("createInFlightGuard", () => { + it("skips a call while a fetch is in flight instead of queueing it", async () => { + let calls = 0; + const gate = deferred(); + const guarded = createInFlightGuard(() => { + calls += 1; + return gate.promise; + }); + + const first = guarded(); + const second = guarded(); + + expect(await second).toBeUndefined(); + expect(calls).toBe(1); + + gate.resolve("done"); + expect(await first).toBe("done"); + }); + + it("allows the next call after the previous one resolves", async () => { + let calls = 0; + const guarded = createInFlightGuard(async () => { + calls += 1; + return calls; + }); + + expect(await guarded()).toBe(1); + expect(await guarded()).toBe(2); + expect(calls).toBe(2); + }); + + it("releases the guard when the fetcher rejects", async () => { + let calls = 0; + const guarded = createInFlightGuard(async () => { + calls += 1; + if (calls === 1) { + throw new Error("boom"); + } + return "recovered"; + }); + + await expect(guarded()).rejects.toThrow("boom"); + expect(await guarded()).toBe("recovered"); + }); +}); + +describe("createLatestGuard", () => { + it("discards an older response that resolves after a newer request", async () => { + const guard = createLatestGuard(); + const slow = deferred(); + const fast = deferred(); + + const first = guard(() => slow.promise); + const second = guard(() => fast.promise); + + fast.resolve("new"); + expect(await second).toBe("new"); + + slow.resolve("old"); + expect(await first).toBeUndefined(); + }); + + it("passes through sequential responses", async () => { + const guard = createLatestGuard(); + expect(await guard(async () => "a")).toBe("a"); + expect(await guard(async () => "b")).toBe("b"); + }); +}); From 84f69b40266fb7cee5207f27a7e9a0629d5cd50c Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 12:27:20 -0700 Subject: [PATCH 11/12] fix: address CodeRabbit PR feedback --- .github/workflows/ci.yml | 18 ++++++--- src/app/sync.rs | 8 +++- src/cli/render.rs | 42 +++++++++++++++----- src/cli/tui_render.rs | 12 +++--- src/cli/watch.rs | 2 +- tests/render_sanitize.rs | 6 +++ tests/skills_refresh.rs | 13 ++++++ tests/sync_branch.rs | 72 +++++++++++++++------------------- tui-opentui/src/data.ts | 61 ++++++++++++++++++++-------- tui-opentui/src/model.ts | 12 +++++- tui-opentui/src/tui-app.tsx | 25 ++++++++---- tui-opentui/src/tui-helpers.ts | 35 +++++++++++++++-- tui-opentui/tests/data.test.ts | 57 ++++++++++++++++++++++++++- 13 files changed, 268 insertions(+), 95 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1859cd5..ff6dd02 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,12 +18,16 @@ jobs: name: Rust Quality runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@4be7066ada62dd38de10e7b70166bc74ed198c30 # stable with: components: rustfmt, clippy + - uses: Swatinem/rust-cache@42dc69e1aa15d09112580998cf2ef0119e2e91ae # v2 + - run: cargo fmt --check - run: cargo clippy --all-targets --all-features -- -D warnings - run: cargo test --quiet @@ -32,9 +36,11 @@ jobs: name: JS and npm checks runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + with: + persist-credentials: false - - uses: oven-sh/setup-bun@v2 + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 - run: bun install --frozen-lockfile working-directory: tui-opentui @@ -42,7 +48,7 @@ jobs: - run: bun run typecheck working-directory: tui-opentui - - uses: dtolnay/rust-toolchain@stable + - uses: Swatinem/rust-cache@42dc69e1aa15d09112580998cf2ef0119e2e91ae # v2 - run: cargo build --quiet --bin tsq @@ -51,7 +57,7 @@ jobs: env: TSQ_CONTRACT_BIN: ${{ github.workspace }}/target/debug/tsq - - uses: actions/setup-node@v4 + - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: "24" diff --git a/src/app/sync.rs b/src/app/sync.rs index 36f48e2..c991318 100644 --- a/src/app/sync.rs +++ b/src/app/sync.rs @@ -192,9 +192,13 @@ pub fn migrate_to_sync_branch( } PushMode::BestEffort => { if let Err(error) = git::push_current_set_upstream(wt_path, &remote, branch) { + // `remote` is git-derived and `error.message` may carry + // untrusted data; sanitize both before writing to stderr to + // avoid emitting raw terminal control sequences. eprintln!( - "tsq: warning: migrated events to sync branch but push to '{remote}' failed: {}; run 'tsq sync' to push later", - error.message + "tsq: warning: migrated events to sync branch but push to '{}' failed: {}; run 'tsq sync' to push later", + crate::cli::render::sanitize_inline(&remote), + crate::cli::render::sanitize_inline(&error.message) ); } } diff --git a/src/cli/render.rs b/src/cli/render.rs index a62b6b4..e0ab91b 100644 --- a/src/cli/render.rs +++ b/src/cli/render.rs @@ -12,11 +12,12 @@ pub struct TreeRenderOptions { const MAX_NARROW_TREE_PREFIX_WIDTH: usize = 24; -/// Escape C0/C1 control characters and DEL for safe terminal display. +/// Escape C0/C1 control characters, DEL, and bidi controls for safe terminal display. /// `\\`, `\t`, `\r`, `\n` keep their existing readable escapes; ESC and every -/// other control character are rendered as `\x{:02x}`. All other characters -/// (including newlines' surrounding text, Unicode, emoji) pass through -/// unchanged. +/// other byte-sized control character are rendered as `\x{:02x}`. Bidi controls +/// render as `\u{...}` so they cannot reorder nearby terminal text. All other +/// characters (including newlines' surrounding text, Unicode, emoji) pass +/// through unchanged. pub(crate) fn sanitize_inline(value: &str) -> String { sanitize(value, false) } @@ -36,6 +37,9 @@ fn sanitize(value: &str, keep_newlines: bool) -> String { '\r' => out.push_str("\\r"), '\n' if keep_newlines => out.push('\n'), '\n' => out.push_str("\\n"), + c if is_bidi_control(c) => { + out.push_str(&format!("\\u{{{:x}}}", c as u32)); + } c if is_unsafe_control(c) => { out.push_str(&format!("\\x{:02x}", c as u32)); } @@ -47,7 +51,11 @@ fn sanitize(value: &str, keep_newlines: bool) -> String { fn is_unsafe_control(ch: char) -> bool { let code = ch as u32; - code < 0x20 || code == 0x7f || (0x80..=0x9f).contains(&code) + code < 0x20 || code == 0x7f || (0x80..=0x9f).contains(&code) || is_bidi_control(ch) +} + +fn is_bidi_control(ch: char) -> bool { + matches!(ch as u32, 0x202a..=0x202e | 0x2066..=0x2069) } fn plain_cell(value: &str) -> String { @@ -165,10 +173,18 @@ pub fn print_task(task: &Task) { println!("{}={}", style::key("assignee"), sanitize_inline(assignee)); } if let Some(external_ref) = &task.external_ref { - println!("{}={}", style::key("external_ref"), external_ref); + println!( + "{}={}", + style::key("external_ref"), + sanitize_inline(external_ref) + ); } if let Some(discovered_from) = &task.discovered_from { - println!("{}={}", style::key("discovered_from"), discovered_from); + println!( + "{}={}", + style::key("discovered_from"), + sanitize_inline(discovered_from) + ); } if let Some(parent) = &task.parent_id { println!("{}={}", style::key("parent"), parent); @@ -436,10 +452,10 @@ fn render_tree_node( meta_width, ); if max_title_width > 0 { - primary_parts.push(sanitize_inline(&truncate_with_ellipsis( - &node.task.title, + primary_parts.push(truncate_with_ellipsis( + &sanitize_inline(&node.task.title), max_title_width, - ))); + )); } } Density::Medium | Density::Wide => { @@ -1016,6 +1032,12 @@ mod tests { assert_eq!(sanitize_inline("a\u{9b}b"), "a\\x9bb"); } + #[test] + fn sanitize_inline_escapes_bidi_controls() { + assert_eq!(sanitize_inline("safe\u{202e}txt"), "safe\\u{202e}txt"); + assert_eq!(sanitize_inline("safe\u{2066}txt"), "safe\\u{2066}txt"); + } + #[test] fn sanitize_inline_escapes_del() { assert_eq!(sanitize_inline("a\u{7f}b"), "a\\x7fb"); diff --git a/src/cli/tui_render.rs b/src/cli/tui_render.rs index 7a7fcfb..d7a90fe 100644 --- a/src/cli/tui_render.rs +++ b/src/cli/tui_render.rs @@ -249,7 +249,7 @@ fn render_board_view(data: &TuiFrameData, width: usize) -> Vec { } fn render_board_card(task: &Task) -> String { - let title = sanitize_inline(&truncate_with_ellipsis(&task.title, 18)); + let title = truncate_with_ellipsis(&sanitize_inline(&task.title), 18); format!( "{} {} {} {}", style::task_id(&task.id), @@ -283,10 +283,10 @@ fn render_inspector(data: &TuiFrameData, width: usize) -> Vec { lines.push(format!("id={}", style::task_id(&task.id))); lines.push(format!( "title={}", - sanitize_inline(&truncate_with_ellipsis( - &task.title, + truncate_with_ellipsis( + &sanitize_inline(&task.title), width.saturating_sub(8).max(12) - )) + ) )); lines.push(format!( "status={} kind={} priority={} planning={}", @@ -412,9 +412,9 @@ fn render_table_row(task: &Task, selected: bool, title_width: usize) -> String { marker, task.id, type_pill(task.kind), - sanitize_inline(&truncate_with_ellipsis(&task.title, title_width)), + truncate_with_ellipsis(&sanitize_inline(&task.title), title_width), status_pill(task.status), - sanitize_inline(&truncate_with_ellipsis(assignee, 12)), + truncate_with_ellipsis(&sanitize_inline(assignee), 12), priority_pill(task.priority), spec_pill(task), ) diff --git a/src/cli/watch.rs b/src/cli/watch.rs index b789837..df139df 100644 --- a/src/cli/watch.rs +++ b/src/cli/watch.rs @@ -467,7 +467,7 @@ fn render_flat_tasks(tasks: &[Task], width: usize) -> Vec { "{} {} {}", status, style::task_id(&task.id), - sanitize_inline(&truncate_with_ellipsis(&task.title, title_width)) + truncate_with_ellipsis(&sanitize_inline(&task.title), title_width) )); lines.push(format!(" {}", meta)); } else { diff --git a/tests/render_sanitize.rs b/tests/render_sanitize.rs index 61a7934..910207d 100644 --- a/tests/render_sanitize.rs +++ b/tests/render_sanitize.rs @@ -7,6 +7,7 @@ use serde_json::Value; // terminal-injection payload. `\u{1b}` is ESC (0x1b), `\u{7}` is BEL (0x07). const EVIL_TITLE: &str = "evil\u{1b}]0;pwned\u{7}title"; const ESC_BYTE: u8 = 0x1b; +const BEL_BYTE: u8 = 0x07; fn assert_no_raw_esc(output: &str) { // Piped stdout from these commands must never contain a literal, @@ -18,6 +19,11 @@ fn assert_no_raw_esc(output: &str) { "human output contained a raw ESC byte: {:?}", output ); + assert!( + !output.as_bytes().contains(&BEL_BYTE), + "human output contained a raw BEL byte: {:?}", + output + ); assert!( output.contains("\\x1b"), "expected the escaped form \\x1b to be present: {:?}", diff --git a/tests/skills_refresh.rs b/tests/skills_refresh.rs index f8faa9c..387e259 100644 --- a/tests/skills_refresh.rs +++ b/tests/skills_refresh.rs @@ -150,8 +150,21 @@ fn skills_refresh_ignores_untrusted_cwd_skills_source() { let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); + let old_placeholder = "\n# Old Tasque Skill\n"; + assert!( + output.status.success(), + "expected refresh to succeed\nstdout:\n{}\nstderr:\n{}", + stdout, + stderr + ); + let refreshed = fs::read_to_string(claude_skill_dir.join("SKILL.md")) .expect("expected SKILL.md to still exist after refresh"); + assert_ne!( + refreshed, old_placeholder, + "managed SKILL.md should be refreshed from the trusted source, not left as the pre-existing placeholder\nstdout:\n{}\nstderr:\n{}", + stdout, stderr + ); assert!( !refreshed.contains("INJECTED-BY-CWD-TEST"), "untrusted CWD SKILLS/ payload must never reach a managed target\nstdout:\n{}\nstderr:\n{}\nrefreshed SKILL.md:\n{}", diff --git a/tests/sync_branch.rs b/tests/sync_branch.rs index 1573644..484757b 100644 --- a/tests/sync_branch.rs +++ b/tests/sync_branch.rs @@ -9,6 +9,29 @@ fn git_out(repo: &std::path::Path, args: &[&str]) -> String { String::from_utf8_lossy(&output.stdout).trim().to_string() } +/// Assert that events were migrated out of the main tree into the sync +/// worktree: root `events.jsonl` cleared, sync worktree `events.jsonl` +/// populated. Shared by the implicit- and explicit-migration tests. +fn assert_events_migrated_to_sync_worktree(root: &std::path::Path) { + let root_events = + fs::read_to_string(root.join(".tasque").join("events.jsonl")).expect("root events"); + assert!( + root_events.is_empty(), + "expected main-tree events cleared after local migration" + ); + let sync_events = fs::read_to_string( + root.join(".git") + .join("tsq-sync") + .join(".tasque") + .join("events.jsonl"), + ) + .expect("sync worktree events"); + assert!( + !sync_events.is_empty(), + "expected events present in sync worktree" + ); +} + fn create_bare_origin(base: &std::path::Path) -> std::path::PathBuf { let remote = base.join("origin.git"); let remote_arg = remote.to_string_lossy().to_string(); @@ -137,11 +160,12 @@ fn make_legacy_repo_with_unreachable_origin( let create = run_cli(&root, ["create", title]); assert_eq!(create.code, 0, "stderr: {}", create.stderr); - let config = fs::read_to_string(root.join(".tasque").join("config.json")).expect("config"); + let config_text = fs::read_to_string(root.join(".tasque").join("config.json")).expect("config"); + let config: Value = serde_json::from_str(&config_text).expect("config must be valid JSON"); assert!( - !config.contains("\"sync_branch\": \""), + config.get("sync_branch").is_none(), "expected legacy config without sync_branch:\n{}", - config + config_text ); init_git_repo_with_identity(&root, Some("main")); @@ -185,23 +209,7 @@ fn implicit_migration_survives_unreachable_origin_with_warning() { list.stderr ); - let root_events = - fs::read_to_string(root.join(".tasque").join("events.jsonl")).expect("root events"); - assert!( - root_events.is_empty(), - "expected main-tree events cleared after local migration" - ); - let sync_events = fs::read_to_string( - root.join(".git") - .join("tsq-sync") - .join(".tasque") - .join("events.jsonl"), - ) - .expect("sync worktree events"); - assert!( - !sync_events.is_empty(), - "expected events present in sync worktree" - ); + assert_events_migrated_to_sync_worktree(&root); } /// Explicit `tsq migrate` keeps push failures fatal, but the events must @@ -214,29 +222,13 @@ fn explicit_migrate_fails_on_unreachable_origin_after_local_migration() { let root = make_legacy_repo_with_unreachable_origin(base, "Offline explicit task"); let migrate = run_cli(&root, ["migrate", "--json"]); - assert_ne!( - migrate.code, 0, - "expected explicit migrate to fail when push fails\nstdout:\n{}\nstderr:\n{}", + assert_eq!( + migrate.code, 2, + "expected explicit migrate to fail with storage/IO error when push fails\nstdout:\n{}\nstderr:\n{}", migrate.stdout, migrate.stderr ); let envelope: Value = serde_json::from_str(migrate.stdout.trim()).expect("json envelope"); assert_eq!(envelope.get("ok").and_then(Value::as_bool), Some(false)); - let root_events = - fs::read_to_string(root.join(".tasque").join("events.jsonl")).expect("root events"); - assert!( - root_events.is_empty(), - "expected main-tree events cleared even when push fails" - ); - let sync_events = fs::read_to_string( - root.join(".git") - .join("tsq-sync") - .join(".tasque") - .join("events.jsonl"), - ) - .expect("sync worktree events"); - assert!( - !sync_events.is_empty(), - "expected events migrated locally despite failed push" - ); + assert_events_migrated_to_sync_worktree(&root); } diff --git a/tui-opentui/src/data.ts b/tui-opentui/src/data.ts index 71add23..a95ed7c 100644 --- a/tui-opentui/src/data.ts +++ b/tui-opentui/src/data.ts @@ -62,20 +62,24 @@ export interface TsqSpawnResult { stderr: string; } -export async function runTsq(argv: string[]): Promise { +export async function runTsq( + argv: string[], + options: { timeoutMs?: number } = {}, +): Promise { const subprocess = Bun.spawn(argv, { stdin: "ignore", stdout: "pipe", stderr: "pipe", + timeout: options.timeoutMs ?? 10_000, }); const [stdout, stderr, exitCode] = await Promise.all([ - new Response(subprocess.stdout).text(), - new Response(subprocess.stderr).text(), + new Response(subprocess.stdout).text().catch(() => ""), + new Response(subprocess.stderr).text().catch(() => ""), subprocess.exited, ]); - return { exitCode, stdout, stderr }; + return { exitCode: exitCode ?? -1, stdout, stderr }; } export function spawnWarning(bin: string, error: unknown): string { @@ -125,9 +129,9 @@ export function parseTasksEnvelope(stdout: string): { tasks: TasqueTask[]; warning?: string; } { - let payload: ListEnvelope; + let payload: unknown; try { - payload = JSON.parse(stdout) as ListEnvelope; + payload = JSON.parse(stdout); } catch { return { tasks: [], @@ -135,14 +139,27 @@ export function parseTasksEnvelope(stdout: string): { }; } - if (!payload.ok) { + if (!payload || typeof payload !== "object") { return { tasks: [], - warning: payload.error?.message ?? "tsq watch returned an error", + warning: "Unexpected payload from tsq watch --once", }; } + const envelope = payload as ListEnvelope; - return { tasks: payload.data?.tasks ?? [] }; + if (!envelope.ok) { + return { + tasks: [], + warning: envelope.error?.message ?? "tsq watch returned an error", + }; + } + + const tasks = envelope.data?.tasks; + if (tasks !== undefined && !Array.isArray(tasks)) { + return { tasks: [], warning: "Task payload missing tasks array" }; + } + + return { tasks: tasks ?? [] }; } interface DepEnvelope { @@ -209,8 +226,15 @@ export function createLatestGuard(): ( return async (fetcher: () => Promise) => { sequence += 1; const ticket = sequence; - const result = await fetcher(); - return ticket === sequence ? result : undefined; + try { + const result = await fetcher(); + return ticket === sequence ? result : undefined; + } catch (error) { + if (ticket !== sequence) { + return undefined; + } + throw error; + } }; } @@ -218,18 +242,23 @@ export function parseDependencyEnvelope(stdout: string): { root?: DependencyNode; warning?: string; } { - let payload: DepEnvelope; + let payload: unknown; try { - payload = JSON.parse(stdout) as DepEnvelope; + payload = JSON.parse(stdout); } catch { return { warning: "Unable to parse JSON output from tsq deps" }; } - if (!payload.ok) { - return { warning: payload.error?.message ?? "tsq deps returned an error" }; + if (!payload || typeof payload !== "object") { + return { warning: "Unexpected payload from tsq deps" }; + } + const envelope = payload as DepEnvelope; + + if (!envelope.ok) { + return { warning: envelope.error?.message ?? "tsq deps returned an error" }; } - const root = normalizeDependencyNode(payload.data?.root); + const root = normalizeDependencyNode(envelope.data?.root); if (!root) { return { warning: "Dependency tree payload missing root node" }; } diff --git a/tui-opentui/src/model.ts b/tui-opentui/src/model.ts index 3807b5c..ac32c16 100644 --- a/tui-opentui/src/model.ts +++ b/tui-opentui/src/model.ts @@ -158,8 +158,18 @@ export function boardColumns( export function buildEpicProgressList(tasks: TasqueTask[]): EpicProgress[] { const epics = tasks.filter((task) => task.kind === "epic"); + const childrenByParent = new Map(); + for (const task of tasks) { + if (!task.parent_id) { + continue; + } + const children = childrenByParent.get(task.parent_id) ?? []; + children.push(task); + childrenByParent.set(task.parent_id, children); + } + return epics.map((epic) => { - const children = tasks.filter((task) => task.parent_id === epic.id); + const children = childrenByParent.get(epic.id) ?? []; let done = 0; let open = 0; diff --git a/tui-opentui/src/tui-app.tsx b/tui-opentui/src/tui-app.tsx index a417c61..af63244 100644 --- a/tui-opentui/src/tui-app.tsx +++ b/tui-opentui/src/tui-app.tsx @@ -1,5 +1,5 @@ import { useKeyboard, useRenderer, useTerminalDimensions } from "@opentui/react"; -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { type DataSnapshot, type DependencyNode, @@ -63,6 +63,7 @@ export function App() { const [filterIndex, setFilterIndex] = useState(0); const [dependencyRoot, setDependencyRoot] = useState(); const [dependencyWarning, setDependencyWarning] = useState(); + const refreshTasksRef = useRef<() => void>(() => {}); useEffect(() => { setFilterIndex((current) => Math.min(current, filterPresets.length - 1)); @@ -82,6 +83,10 @@ export function App() { setWarning(next.warning); }; + refreshTasksRef.current = () => { + void refresh(); + }; + void refresh(); const timer = setInterval(() => { void refresh(); @@ -152,13 +157,16 @@ export function App() { setDependencyWarning(undefined); return; } + setDependencyRoot(undefined); + setDependencyWarning(undefined); + let cancelled = false; // Debounce rapid selection changes; the latest-guard discards responses // that resolve after a newer fetch has been issued. const timer = setTimeout(() => { void latestDependencyFetch(() => fetchDependencyTree(config.tsqBin, taskId), ).then((dependency) => { - if (!dependency) { + if (cancelled || !dependency) { return; } setDependencyRoot(dependency.root); @@ -166,6 +174,7 @@ export function App() { }); }, 150); return () => { + cancelled = true; clearTimeout(timer); }; }, [config.tsqBin, latestDependencyFetch, selectedTask?.id, tab]); @@ -203,7 +212,7 @@ export function App() { void readSpecLines(config.tsqBin, task.id).then((result) => { setSpecDialog((current) => { // Stale check: apply only if the dialog is still open for this task. - if (!current || current.taskId !== task.id) { + if (!current || current.taskId !== task.id || current.specPath !== task.spec_path) { return current; } return { @@ -268,10 +277,7 @@ export function App() { } if (key.name === "r") { - void fetchTasks(config).then((next) => { - setSnapshot(next); - setWarning(next.warning); - }); + refreshTasksRef.current(); return; } @@ -354,7 +360,10 @@ export function App() { }; }); }); - const itemBudget = tab === "tasks" ? Math.max(2, Math.floor(tableRowBudget / 2)) : tableRowBudget; + const itemBudget = + tab === "tasks" || tab === "epics" + ? Math.max(2, Math.floor(tableRowBudget / 2)) + : tableRowBudget; const [start, end] = visibleRange(selectedIndex, rowCount, itemBudget); return ( diff --git a/tui-opentui/src/tui-helpers.ts b/tui-opentui/src/tui-helpers.ts index 855559c..3418f80 100644 --- a/tui-opentui/src/tui-helpers.ts +++ b/tui-opentui/src/tui-helpers.ts @@ -134,6 +134,12 @@ export async function readSpecLines( } if (result.exitCode !== 0) { + if (result.stdout.trim()) { + const parsed = parseSpecEnvelopeResult(result.stdout); + if (parsed.kind === "envelope-error") { + return { lines: parsed.lines, warning: parsed.warning }; + } + } return { lines: [], warning: @@ -144,22 +150,38 @@ export async function readSpecLines( return parseSpecEnvelope(result.stdout); } +type SpecEnvelopeResult = { + kind: "ok" | "envelope-error" | "parse-error"; + lines: string[]; + warning?: string; +}; + export function parseSpecEnvelope(stdout: string): { lines: string[]; warning?: string; } { + const parsed = parseSpecEnvelopeResult(stdout); + return { lines: parsed.lines, warning: parsed.warning }; +} + +function parseSpecEnvelopeResult(stdout: string): SpecEnvelopeResult { let payload: unknown; try { payload = JSON.parse(stdout); } catch { return { + kind: "parse-error", lines: [], warning: "Unable to parse JSON output from tsq spec --show", }; } if (!payload || typeof payload !== "object") { - return { lines: [], warning: "Unexpected payload from tsq spec --show" }; + return { + kind: "parse-error", + lines: [], + warning: "Unexpected payload from tsq spec --show", + }; } const envelope = payload as Record; @@ -171,6 +193,7 @@ export function parseSpecEnvelope(stdout: string): { const message = typeof error?.message === "string" ? error.message : undefined; return { + kind: "envelope-error", lines: [], warning: message ?? "tsq spec --show returned an error", }; @@ -186,13 +209,17 @@ export function parseSpecEnvelope(stdout: string): { : undefined; const content = typeof spec?.content === "string" ? spec.content : undefined; if (content === undefined) { - return { lines: [], warning: "Spec payload missing content" }; + return { + kind: "envelope-error", + lines: [], + warning: "Spec payload missing content", + }; } if (content.length === 0) { - return { lines: ["(empty spec)"] }; + return { kind: "ok", lines: ["(empty spec)"] }; } - return { lines: content.replaceAll("\r\n", "\n").split("\n") }; + return { kind: "ok", lines: content.replace(/\r\n?/g, "\n").split("\n") }; } export function buildFilterPresets(statusCsv: string): FilterPreset[] { diff --git a/tui-opentui/tests/data.test.ts b/tui-opentui/tests/data.test.ts index 7985742..3caa24a 100644 --- a/tui-opentui/tests/data.test.ts +++ b/tui-opentui/tests/data.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { parseDependencyEnvelope, parseTasksEnvelope } from "../src/data"; +import { parseDependencyEnvelope, parseTasksEnvelope, runTsq } from "../src/data"; import { parseSpecEnvelope } from "../src/tui-helpers"; const sampleTask = { @@ -48,6 +48,26 @@ describe("parseTasksEnvelope", () => { expect(result.tasks).toEqual([]); expect(result.warning).toBeUndefined(); }); + + it("warns on a null payload", () => { + const result = parseTasksEnvelope("null"); + expect(result.tasks).toEqual([]); + expect(result.warning).toBe("Unexpected payload from tsq watch --once"); + }); + + it("warns on a non-object payload", () => { + const result = parseTasksEnvelope(JSON.stringify("oops")); + expect(result.tasks).toEqual([]); + expect(result.warning).toBe("Unexpected payload from tsq watch --once"); + }); + + it("warns on a non-array tasks field", () => { + const result = parseTasksEnvelope( + JSON.stringify({ ok: true, data: { tasks: "not-a-list" } }), + ); + expect(result.tasks).toEqual([]); + expect(result.warning).toBe("Task payload missing tasks array"); + }); }); describe("parseDependencyEnvelope", () => { @@ -98,6 +118,18 @@ describe("parseDependencyEnvelope", () => { expect(result.root).toBeUndefined(); expect(result.warning).toBe("Dependency tree payload missing root node"); }); + + it("warns on a null payload", () => { + const result = parseDependencyEnvelope("null"); + expect(result.root).toBeUndefined(); + expect(result.warning).toBe("Unexpected payload from tsq deps"); + }); + + it("warns on a non-object payload", () => { + const result = parseDependencyEnvelope(JSON.stringify(42)); + expect(result.root).toBeUndefined(); + expect(result.warning).toBe("Unexpected payload from tsq deps"); + }); }); describe("parseSpecEnvelope", () => { @@ -149,3 +181,26 @@ describe("parseSpecEnvelope", () => { expect(result.warning).toBe("Spec payload missing content"); }); }); + +describe("runTsq", () => { + it("returns stdout, stderr, and exitCode for a normal process", async () => { + const result = await runTsq([ + process.execPath, + "-e", + "process.stdout.write('hi'); process.stderr.write('bye'); process.exit(3)", + ]); + + expect(result).toEqual({ exitCode: 3, stdout: "hi", stderr: "bye" }); + }); + + it("returns a non-zero exit when a process times out", async () => { + const start = Date.now(); + const result = await runTsq( + [process.execPath, "-e", "setInterval(() => {}, 1000)"], + { timeoutMs: 300 }, + ); + + expect(result.exitCode).not.toBe(0); + expect(Date.now() - start).toBeLessThan(5000); + }); +}); From 1d731e85ee248a9ba65eae4305ad0ec1c11ad178 Mon Sep 17 00:00:00 2001 From: Aditya Sharma Date: Thu, 2 Jul 2026 13:08:06 -0700 Subject: [PATCH 12/12] fix: address follow-up PR comments --- .github/workflows/ci.yml | 6 ++---- src/app/sync.rs | 2 +- src/cli/render.rs | 7 +++++-- tests/sync_branch.rs | 30 ++++++++++++++++++++++++++---- tui-opentui/src/data.ts | 9 +++++++++ tui-opentui/src/tui-helpers.ts | 2 +- tui-opentui/tests/data.test.ts | 14 ++++++++++++++ 7 files changed, 58 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ff6dd02..70ee161 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,9 +22,7 @@ jobs: with: persist-credentials: false - - uses: dtolnay/rust-toolchain@4be7066ada62dd38de10e7b70166bc74ed198c30 # stable - with: - components: rustfmt, clippy + - run: rustup component add rustfmt clippy - uses: Swatinem/rust-cache@42dc69e1aa15d09112580998cf2ef0119e2e91ae # v2 @@ -57,7 +55,7 @@ jobs: env: TSQ_CONTRACT_BIN: ${{ github.workspace }}/target/debug/tsq - - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: "24" diff --git a/src/app/sync.rs b/src/app/sync.rs index c991318..aefd461 100644 --- a/src/app/sync.rs +++ b/src/app/sync.rs @@ -184,7 +184,6 @@ pub fn migrate_to_sync_branch( let wt_path = Path::new(&setup.worktree_path); let _ = git::commit_worktree(wt_path, "chore: migrate tasque events to sync branch")?; - clear_repo_events(repo_root)?; if let Some(remote) = git::current_upstream_remote(Path::new(repo_root))? { match push { PushMode::Required => { @@ -204,6 +203,7 @@ pub fn migrate_to_sync_branch( } } } + clear_repo_events(repo_root)?; Ok(MigrateResult { events_migrated: to_append.len(), diff --git a/src/cli/render.rs b/src/cli/render.rs index e0ab91b..7e53d97 100644 --- a/src/cli/render.rs +++ b/src/cli/render.rs @@ -51,11 +51,11 @@ fn sanitize(value: &str, keep_newlines: bool) -> String { fn is_unsafe_control(ch: char) -> bool { let code = ch as u32; - code < 0x20 || code == 0x7f || (0x80..=0x9f).contains(&code) || is_bidi_control(ch) + code < 0x20 || code == 0x7f || (0x80..=0x9f).contains(&code) } fn is_bidi_control(ch: char) -> bool { - matches!(ch as u32, 0x202a..=0x202e | 0x2066..=0x2069) + matches!(ch as u32, 0x061c | 0x200e | 0x200f | 0x202a..=0x202e | 0x2066..=0x2069) } fn plain_cell(value: &str) -> String { @@ -1034,6 +1034,9 @@ mod tests { #[test] fn sanitize_inline_escapes_bidi_controls() { + assert_eq!(sanitize_inline("safe\u{061c}txt"), "safe\\u{61c}txt"); + assert_eq!(sanitize_inline("safe\u{200e}txt"), "safe\\u{200e}txt"); + assert_eq!(sanitize_inline("safe\u{200f}txt"), "safe\\u{200f}txt"); assert_eq!(sanitize_inline("safe\u{202e}txt"), "safe\\u{202e}txt"); assert_eq!(sanitize_inline("safe\u{2066}txt"), "safe\\u{2066}txt"); } diff --git a/tests/sync_branch.rs b/tests/sync_branch.rs index 484757b..1f28983 100644 --- a/tests/sync_branch.rs +++ b/tests/sync_branch.rs @@ -6,6 +6,12 @@ use std::fs; fn git_out(repo: &std::path::Path, args: &[&str]) -> String { let output = git_output(repo, args); + assert!( + output.status.success(), + "git {:?} failed: {}", + args, + String::from_utf8_lossy(&output.stderr) + ); String::from_utf8_lossy(&output.stdout).trim().to_string() } @@ -212,9 +218,9 @@ fn implicit_migration_survives_unreachable_origin_with_warning() { assert_events_migrated_to_sync_worktree(&root); } -/// Explicit `tsq migrate` keeps push failures fatal, but the events must -/// already be migrated and the main tree cleared (local commit lands before -/// the push), so a later `tsq sync` can complete the push. +/// Explicit `tsq migrate` keeps push failures fatal and preserves the original +/// main-tree events when the required push fails. The sync worktree commit has +/// landed locally, so a later successful migrate can dedupe and clear safely. #[test] fn explicit_migrate_fails_on_unreachable_origin_after_local_migration() { let repo = make_repo(); @@ -230,5 +236,21 @@ fn explicit_migrate_fails_on_unreachable_origin_after_local_migration() { let envelope: Value = serde_json::from_str(migrate.stdout.trim()).expect("json envelope"); assert_eq!(envelope.get("ok").and_then(Value::as_bool), Some(false)); - assert_events_migrated_to_sync_worktree(&root); + let root_events = + fs::read_to_string(root.join(".tasque").join("events.jsonl")).expect("root events"); + assert!( + !root_events.is_empty(), + "expected main-tree events preserved after required push failure" + ); + let sync_events = fs::read_to_string( + root.join(".git") + .join("tsq-sync") + .join(".tasque") + .join("events.jsonl"), + ) + .expect("sync worktree events"); + assert!( + !sync_events.is_empty(), + "expected events present in sync worktree" + ); } diff --git a/tui-opentui/src/data.ts b/tui-opentui/src/data.ts index a95ed7c..eee8eed 100644 --- a/tui-opentui/src/data.ts +++ b/tui-opentui/src/data.ts @@ -71,6 +71,7 @@ export async function runTsq( stdout: "pipe", stderr: "pipe", timeout: options.timeoutMs ?? 10_000, + killSignal: "SIGKILL", }); const [stdout, stderr, exitCode] = await Promise.all([ @@ -147,6 +148,10 @@ export function parseTasksEnvelope(stdout: string): { } const envelope = payload as ListEnvelope; + if (typeof envelope.ok !== "boolean") { + return { tasks: [], warning: "Unexpected payload from tsq watch --once" }; + } + if (!envelope.ok) { return { tasks: [], @@ -254,6 +259,10 @@ export function parseDependencyEnvelope(stdout: string): { } const envelope = payload as DepEnvelope; + if (typeof envelope.ok !== "boolean") { + return { warning: "Unexpected payload from tsq deps" }; + } + if (!envelope.ok) { return { warning: envelope.error?.message ?? "tsq deps returned an error" }; } diff --git a/tui-opentui/src/tui-helpers.ts b/tui-opentui/src/tui-helpers.ts index 3418f80..b335764 100644 --- a/tui-opentui/src/tui-helpers.ts +++ b/tui-opentui/src/tui-helpers.ts @@ -136,7 +136,7 @@ export async function readSpecLines( if (result.exitCode !== 0) { if (result.stdout.trim()) { const parsed = parseSpecEnvelopeResult(result.stdout); - if (parsed.kind === "envelope-error") { + if (parsed.kind === "envelope-error" || parsed.kind === "parse-error") { return { lines: parsed.lines, warning: parsed.warning }; } } diff --git a/tui-opentui/tests/data.test.ts b/tui-opentui/tests/data.test.ts index 3caa24a..f3db590 100644 --- a/tui-opentui/tests/data.test.ts +++ b/tui-opentui/tests/data.test.ts @@ -61,6 +61,14 @@ describe("parseTasksEnvelope", () => { expect(result.warning).toBe("Unexpected payload from tsq watch --once"); }); + it("warns on a non-boolean ok field", () => { + const result = parseTasksEnvelope( + JSON.stringify({ ok: "false", data: { tasks: [sampleTask] } }), + ); + expect(result.tasks).toEqual([]); + expect(result.warning).toBe("Unexpected payload from tsq watch --once"); + }); + it("warns on a non-array tasks field", () => { const result = parseTasksEnvelope( JSON.stringify({ ok: true, data: { tasks: "not-a-list" } }), @@ -130,6 +138,12 @@ describe("parseDependencyEnvelope", () => { expect(result.root).toBeUndefined(); expect(result.warning).toBe("Unexpected payload from tsq deps"); }); + + it("warns on a non-boolean ok field", () => { + const result = parseDependencyEnvelope(JSON.stringify({ ok: "true" })); + expect(result.root).toBeUndefined(); + expect(result.warning).toBe("Unexpected payload from tsq deps"); + }); }); describe("parseSpecEnvelope", () => {