diff --git a/console/README.md b/console/README.md index 36732d80..70d61fbd 100644 --- a/console/README.md +++ b/console/README.md @@ -221,7 +221,11 @@ folding still works through live updates. Keyboard row navigation skips folded rows, and the same controls work in every Theme and on phones. Without JavaScript, groups start open and their native disclosures still work. The folders on the left can filter to one section, including Done and Resolved. -Rows show state, subject, title, gist and age. A Spec with recorded Tickets also +Rows show state, subject, title, gist and age. Compact Done headers also identify +their Command kind on desktop and phone, with or without JavaScript, in +every Theme and across the Fleet. Stored `Run` kinds display as `Ticket run`; +Custom processes show `Process` and their retained process name. Done outcomes +remain visible on phones alongside the kind. A Spec with recorded Tickets also shows `2 / 5 Tickets complete`, including on phones and on the visible row of a Pass that dispatched it. This counts recorded merged Tickets, including Tickets closed before the run, and updates with the Command record. Fleet uses the owning diff --git a/console/tests/inbox.test.cjs b/console/tests/inbox.test.cjs index ca6c9866..353bafe4 100644 --- a/console/tests/inbox.test.cjs +++ b/console/tests/inbox.test.cjs @@ -66,6 +66,72 @@ async function pageFor(t, options) { return page; } +test('Done headers keep Command kinds and outcomes readable on desktop and phones', async t => { + const directory = join(home, '.thirdshift/logs/acme/widgets/commands/issue'); + const paths = [1, 2, 3, 4, 5].map(number => join(directory, `${number}-fixture.command.json`)); + const originals = await Promise.all(paths.map(path => readFile(path, 'utf8'))); + const base = JSON.parse(originals[0]); + const process = 'nightly-maintenance-of-every-service-in-the-development-environment'; + const cases = [ + ['Run', null, 'Ticket run'], + ['Spec run', null, 'Spec run'], + ['Triage run', null, 'Triage run'], + ['Review run', null, 'Review run'], + ['Process', process, `Process ${process}`], + ]; + const fixtures = cases.map(([kind, process_name], index) => ({ ...base, kind, process_name, + name: `done-kind-${index}`, tickets: index === 1 ? base.tickets : [], cause: null, outcome: 'merged', + issue: { number: 42, url: 'https://github.com/acme/widgets/issues/42', title: 'Identify completed work' }, + })); + try { + for (const viewport of [{ width: 1440, height: 900 }, { width: 390, height: 844 }]) { + for (const javaScriptEnabled of [true, false]) { + await Promise.all(paths.map((path, index) => replaceRecord(path, JSON.stringify(fixtures[index])))); + const page = await pageFor(t, { viewport, javaScriptEnabled }); + await page.goto(`${origin}/?section=done`); + const heads = paths.map((_, index) => page.locator(`#inbox .command[data-command*="${index + 1}-fixture.command.json"] > .row-head`)); + for (const family of ['focus', 'plain', 'press', 'signal', 'garden', 'factory']) { + for (const mode of ['light', 'dark']) { + const theme = `${family}-${mode}`; + await page.evaluate(theme => { document.documentElement.dataset.theme = theme; }, theme); + await page.evaluate(() => document.fonts.ready); + for (const [index, [, , label]] of cases.entries()) { + assert.equal(await heads[index].locator('.command-kind').textContent(), label); + assert.equal(await heads[index].locator('.subject a').getAttribute('href'), fixtures[index].issue.url); + const fields = ['.command-kind', '.state', '.subject-issue', '.issue-title', '.gist', '.age']; + if (index === 1) fields.push('.ticket-progress'); + for (const field of fields) { + const layout = await heads[index].locator(field).evaluate(node => { + const box = node.getBoundingClientRect(); + const range = document.createRange(); + range.selectNodeContents(node); + const rects = [...range.getClientRects()]; + return { box: box.toJSON(), rects: rects.map(rect => rect.toJSON()), visible: box.width > 0 && box.height > 0 && rects.length > 0 && rects.every(rect => + rect.left >= box.left - 1 && rect.right <= box.right + 1 && + rect.left >= 0 && rect.right <= innerWidth) }; + }); + assert.ok(layout.visible, `${field} must be readable for ${label}, ${theme}, ${viewport.width}px, JS=${javaScriptEnabled}: ${JSON.stringify(layout)}`); + } + assert.equal(await heads[index].locator('.gist').textContent(), 'merged'); + } + assert.equal(await page.evaluate(() => document.documentElement.scrollWidth > innerWidth), false); + } + } + assert.equal(await page.locator('body').evaluate(node => node.classList.contains('peek-open')), false); + if (javaScriptEnabled) { + await replaceRecord(paths[0], JSON.stringify({ ...fixtures[0], kind: 'Fix run' })); + await heads[0].locator('.command-kind').getByText('Fix run', { exact: true }).waitFor(); + } + await page.reload(); + assert.equal(await heads[0].locator('.command-kind').textContent(), javaScriptEnabled ? 'Fix run' : 'Ticket run'); + await page.close(); + } + } + } finally { + await Promise.all(paths.map((path, index) => replaceRecord(path, originals[index]))); + } +}); + test('Needs you Review and Triage headers prefer PR links and keep issue and Command fallbacks', async t => { const directory = join(home, '.thirdshift/logs/acme/widgets/commands/issue'); const paths = [1, 2, 3, 4, 5].map(number => join(directory, `${number}-fixture.command.json`)); @@ -1767,8 +1833,8 @@ test('Ready rows link PR subjects, show titles and CI across widths, and update const path = join(dir, `${number}-fixture.command.json`); paths.push(path); const url = `https://github.com/acme/widgets/pull/${number}`; - await replaceRecord(path, JSON.stringify({ ...base, kind: 'Run', outcome: 'ready for review', - cause: null, tickets: [], pull_request: url, + await replaceRecord(path, JSON.stringify({ ...base, kind: number === 232 ? 'Spec run' : 'Run', outcome: 'ready for review', + cause: null, tickets: number === 232 ? base.tickets : [], pull_request: url, issue: number === 229 ? null : { ...base.issue, number, url: `https://github.com/acme/widgets/issues/${number}`, title: 'A distinct issue title that must not replace the PR title' } })); // Undefined represents an old cache; null represents an unread PR. if (ci !== null) subjects[url] = { state: 'Open', merged_at: null, closed_at: null, ci, mergeable, @@ -1780,7 +1846,7 @@ test('Ready rows link PR subjects, show titles and CI across widths, and update for (const javaScriptEnabled of [true, false]) { const page = await pageFor(t, { javaScriptEnabled }); await page.goto(`${origin}/?section=ready-for-review`); - for (const width of [1440, 768, 390]) { + for (const width of [1440, 1000, 900, 768, 390]) { await page.setViewportSize({ width, height: 900 }); for (const theme of ['focus-light', 'focus-dark', 'signal-light', 'signal-dark']) { await page.evaluate(theme => { document.documentElement.dataset.theme = theme; }, theme); @@ -1793,6 +1859,7 @@ test('Ready rows link PR subjects, show titles and CI across widths, and update assert.equal(await row.locator('script').count(), 0); assert.equal(await row.locator('.row-head > .issue-title').isVisible(), true); assert.equal(await row.locator('.merge-conflict').count(), mergeable === 'conflicting' ? 1 : 0); + assert.equal(await row.locator('.command-kind').isVisible(), false, 'the extra label belongs to Done'); const badges = [[row.locator('.ci'), label, tag]]; if (mergeable === 'conflicting') badges.push([row.locator('.merge-conflict'), 'Merge conflict', 'urgent']); for (const [badge, label, tag] of badges) { @@ -1812,6 +1879,15 @@ test('Ready rows link PR subjects, show titles and CI across widths, and update assert.equal(seen.clipped, false, `${width}: ${label} must be readable`); assert.equal(seen.fits, true, `${width}: ${label} must fit`); } + const overlap = await row.locator('.row-head').evaluate(head => { + const checks = head.querySelector('.pr-checks').getBoundingClientRect(); + return [...head.querySelectorAll('.command-kind, .ticket-progress')].some(node => { + const field = node.getBoundingClientRect(); + return field.left < checks.right && field.right > checks.left && + field.top < checks.bottom && field.bottom > checks.top; + }); + }); + assert.equal(overlap, false, `${width}: PR checks must not overlap Command kind or Ticket progress`); } } assert.equal(await page.evaluate(() => document.documentElement.scrollWidth <= innerWidth), true); diff --git a/site/how-it-works/index.html b/site/how-it-works/index.html index fc6734dd..cd42b484 100644 --- a/site/how-it-works/index.html +++ b/site/how-it-works/index.html @@ -195,6 +195,7 @@

How thirdshift works

A later Command on the same pull request, as on the same issue, takes up an earlier row waiting in Ready for review or Needs you. This includes a Fix run on a pull request with no linked issue: the earlier row says “taken up by a later Command started at <time>”. Ready for review rows with CI red or Merge conflict offer Start Fix run when the owning machine currently permits run and knows the repository's Launch directory. Confirm the pull request, repository and machine to start there using its User config and CLI defaults; Cancel or Escape starts nothing. Other-account Commands stay read-only. Copy fix command remains on the row and in Command details: thirdshift fix <PR URL>. Copy it and run it from a clone of the repository.

Every Console tab title names its host; the Inbox also keeps its Needs you count through live updates.

Click the serving machine's name in the Console header to return to Overview (/?section=overview) from another page or section, clearing any machine filter.

+

Done rows identify their Command kind, such as Ticket run, and Custom processes by their process name. The kind and outcome remain visible on desktop and phone, with or without JavaScript, in every Theme and across the Fleet.

A Spec's Inbox row shows its last recorded Ticket progress, such as 2 / 5 Tickets complete, on desktop and phone. A Pass that dispatched the Spec carries that count on its visible row. All Tickets complete can still leave Spec review or Delivery running.

The Console and status show the serving OS account's Commands by default. thirdshift console other-accounts readable, or a comma-separated list of local account names, adds readable Other-account Commands with their owner shown. They appear in Needs you and its tab count, but have no Stop, Requeue, Dismiss or Undo. Those actions apply only to the serving account's Commands. Grant read access with filesystem permissions or ACLs; thirdshift never grants access. Account visibility.

Ready for review subjects link directly to the recorded pull request and show its title, including Review runs without an issue. Titles come from the batched GitHub refresh and update on open pages. Until a title is known, the PR link stays usable with “(title unavailable)”. Without a PR, rows use their issue subject and title, or the Command subject when neither exists. Activating the link opens GitHub without opening Details.

diff --git a/site/index.html b/site/index.html index c3d79e49..c66a8e6d 100644 --- a/site/index.html +++ b/site/index.html @@ -502,6 +502,7 @@

Seven workflows

Ready for review subjects link directly to the recorded pull request and show its title, including Review runs without an issue. The batched GitHub refresh updates titles on open pages; unavailable titles keep a usable PR link with “(title unavailable)”. Without a PR, rows keep the issue subject and title, or the Command subject when neither exists. Following the subject link opens GitHub without opening Details.

In Needs you and Overview, Review and Triage row headers link directly to the recorded pull request, or to the issue when no PR is recorded, on desktop and phone and across the Fleet. Existing-issue Triage retains its explicit or Pass-selected issue even when it fails. Older records and Report-based Triage without a target keep the repository and Command label.

Every Inbox group starts open. Click its heading, or focus it and press Enter or Space, to fold its rows. Headings and counts stay visible, and counts and active runs still include folded rows. Each browser remembers its choices through live updates and reloads when storage is available. Keyboard row navigation skips folded rows, in every Theme and on phones.

+

Done rows show their Command kind, such as Ticket run or Triage run; Custom processes show their process name. Done keeps the kind and outcome visible on desktop and phone, with or without JavaScript, in every Theme and across the Fleet.

On a desktop, Inbox issue and pull request numbers stay visible when long repository or Process subjects shrink. Hover a subject for its full text.

Command details opens with the Command's issue subject linked to the issue and its full title, wrapping long titles above Stop and Dismiss. An unknown title says “(title unavailable)”; a Command without an issue shows its Command subject alone. The recorded issue title is also in thirdshift show <command> --json.

An ended, unsuccessful Command shows its full failure reason immediately below that heading, before the controls, with URLs as links. When its own cause is absent, it uses the first cause from failed dispatched work. Successful or live Commands, and those with no cause, show no reason. The cause also stays in the facts list.

diff --git a/src/console/assets/console.css b/src/console/assets/console.css index b529f210..d859426b 100644 --- a/src/console/assets/console.css +++ b/src/console/assets/console.css @@ -269,6 +269,9 @@ body:is(.mailbox, .archive-page).peek-open { max-width: 1600px; grid-template-co #inbox > .group > summary .count { margin-left: 8px; color: var(--dim); } #inbox > .group > .empty, #inbox > .group > p { margin: 0; padding: 10px; } .row-head > .gist { display: none; } +.one-line [data-group="done"] .row-head > .gist { display: block; } +.command-kind { display: none; color: var(--text); overflow-wrap: anywhere; } +.one-line [data-group="done"] .command-kind { display: block; } .one-line .command { margin: 0; border-radius: 0; border-bottom: 1px solid var(--rule); } .one-line .command:last-child { border-bottom: 0; } @media (scripting: enabled) { @@ -284,11 +287,16 @@ body:is(.mailbox, .archive-page).peek-open { max-width: 1600px; grid-template-co .one-line .row-head > .heartbeat { position: absolute; left: 2px; top: 50%; margin-top: -3px; } .one-line .row-head > .state { grid-column: 1; justify-self: start; max-width: 100%; } .one-line .row-head > .subject { grid-column: 2; } + .one-line .row-head > .subject > a { display: flex; min-width: 0; } + .one-line .subject-repository { min-width: 0; overflow: hidden; text-overflow: ellipsis; } + .one-line .subject-issue { flex: none; } + .one-line .row-head > .command-kind { grid-column: 2; grid-row: 2; white-space: normal; } .one-line .row-head > .issue-title { grid-column: 3; color: var(--text); } .one-line .row-head > .issue-title:empty { display: none; } .one-line .row-head > .gist { display: block; grid-column: 4; } .one-line .row-head:not(:has(> .ticket-progress)) > .issue-title:empty + .gist { grid-column: 3 / 5; } .one-line .row-head > .ticket-progress { grid-column: 2; grid-row: 2; color: var(--text); white-space: normal; } + .one-line [data-group="done"] .row-head > .ticket-progress { grid-row: 3; } .one-line .row-head:has(> .ticket-progress) > .gist { grid-column: 3 / -1; grid-row: 2; white-space: normal; } /* Keep the pull request indicators together as the row narrows. */ .one-line .row-head > .pr-checks { grid-column: 5; grid-row: 1; align-self: center; overflow: visible; } @@ -296,7 +304,7 @@ body:is(.mailbox, .archive-page).peek-open { max-width: 1600px; grid-template-co .one-line .row-head > .new::before { content: "New"; font-size: 12px; color: var(--head); font-weight: var(--head-weight); } .one-line .row-head > .age { grid-column: 7; margin: 0; font-size: 12px; } .one-line .row-head .finish { display: none; } - .one-line .row-head > .row-controls { grid-column: 8; white-space: normal; overflow: visible; } + .one-line .row-head > .row-controls { grid-column: 8; grid-row: 1; white-space: normal; overflow: visible; } .one-line .row-head > .row-controls button { padding: 0 8px; font-size: 12px; } .one-line .row-head [data-copy-status] { font-size: 12px; } .one-line .row-head [data-copy-status]:empty { display: none; } @@ -316,9 +324,6 @@ body:is(.mailbox, .archive-page).peek-open { max-width: 1600px; grid-template-co @media (scripting: enabled) and (min-width: 761px) { .one-line .row-head > .subject:has(> a) { display: flex; align-items: baseline; gap: .6em; } .one-line .subject > .account-tag { flex: none; } - .one-line .row-head > .subject > a { display: flex; min-width: 0; } - .one-line .subject-repository { min-width: 0; overflow: hidden; text-overflow: ellipsis; } - .one-line .subject-issue { flex: none; } } @media (scripting: enabled) and (min-width: 761px) and (max-width: 1100px) { .one-line .row-head > .pr-checks { grid-column: 1 / -1; grid-row: 2; justify-self: start; } @@ -336,11 +341,15 @@ body:is(.mailbox, .archive-page).peek-open { max-width: 1600px; grid-template-co .one-line .row-head > .age { grid-column: 3; grid-row: 1; } .one-line .row-head > .row-controls { grid-column: 3; grid-row: 2; justify-self: end; } .one-line .row-head > .issue-title { grid-column: 1 / 3; grid-row: 2; } + .one-line .row-head > .command-kind { grid-column: 1 / -1; grid-row: 3; } .one-line .row-head > .gist { display: none; } .one-line .row-head:not(:has(> .ticket-progress)) > .issue-title:empty + .gist { display: block; grid-column: 1 / 3; grid-row: 2; } .one-line .command[data-slow="true"] > .row-head:not(:has(> .ticket-progress)) > .gist { display: block; grid-column: 1 / -1; grid-row: 3; white-space: normal; } + .one-line [data-group="done"] .row-head > .gist { display: block; grid-column: 1 / -1; grid-row: 4; white-space: normal; } .one-line .row-head > .ticket-progress { grid-column: 1 / -1; grid-row: 3; } + .one-line [data-group="done"] .row-head > .ticket-progress { grid-row: 4; } .one-line .row-head:has(> .ticket-progress) > .gist { display: block; grid-column: 1 / -1; grid-row: 4; white-space: normal; } + .one-line [data-group="done"] .row-head:has(> .ticket-progress) > .gist { grid-row: 5; } .one-line .row-head:has(> .ticket-progress) > .pr-checks { grid-row: 5; } } } diff --git a/src/console/row.html b/src/console/row.html index a804d304..04f3e93d 100644 --- a/src/console/row.html +++ b/src/console/row.html @@ -6,6 +6,7 @@ {{ row.title }}{{ self.gist()|readable }}{% match row.timing.finish %}{% when Some with (finish) %} · {% when None %}{% endmatch %}{{ row.timing.age }} {% match row.ticket_progress %}{% when Some with (progress) %}{{ progress.complete }} / {{ progress.total }} Tickets complete{% when None %}{% endmatch %} {% match row.ci %}{% when Some with (ci) %}{{ ci.label() }}{% if row.merge_conflict %}Merge conflict{% endif %}{% when None %}{% endmatch %} + {{ row.command_kind }} {% if can_copy || (can_act && (row.can_dismiss || row.remote_stop || row.remote_undo)) %}
{% if can_copy %} {% endif %} {% if can_act && !row.console.is_empty() && row.remote_stop %}
{% endif %} {% if can_act && row.can_dismiss %}
{% if !row.console.is_empty() %}{% endif %}
{% endif %} diff --git a/src/logs/status/inbox.rs b/src/logs/status/inbox.rs index 38ff4c9c..b46c6802 100644 --- a/src/logs/status/inbox.rs +++ b/src/logs/status/inbox.rs @@ -55,6 +55,7 @@ pub(crate) struct Row { pub target_pr: Option, pub base_branch: Option, pub title: String, + pub command_kind: String, pub state: String, pub now_line: String, pub timing: Timing, @@ -479,6 +480,10 @@ fn present(value: &serde_json::Value, now: DateTime) -> Result { .filter(|pr| pr.issue.repo_slug().eq_ignore_ascii_case(&facts.repository)), base_branch: facts.base_branch.filter(|_| live_target), title, + command_kind: process.map_or_else( + || crate::logs::display_name(&facts.kind).to_owned(), + |name| format!("Process {name}"), + ), state, now_line: super::stage_details( facts.outcome.as_deref(), @@ -739,6 +744,7 @@ fn entry_row(entry: Observation<'_>, now: DateTime, root: &std::path::Pat target_pr: None, base_branch: None, title, + command_kind: crate::logs::display_name(&command.kind).to_owned(), state: command.state.into(), now_line: command .outcome diff --git a/tests/console/inbox.rs b/tests/console/inbox.rs index 4387ad2a..30b14061 100644 --- a/tests/console/inbox.rs +++ b/tests/console/inbox.rs @@ -3,6 +3,89 @@ use serde_json::Value; use std::path::Path; use std::{fs, process::Command}; +#[test] +fn done_headers_identify_command_kinds_locally_and_across_fleet() { + use serde_json::json; + use std::time::{Duration, Instant}; + + let owner = tempfile::tempdir().unwrap(); + let caller = tempfile::tempdir().unwrap(); + configure(owner.path()); + configure(caller.path()); + let cases = [ + ("Run", None, "Ticket run"), + ("Spec run", None, "Spec run"), + ("Triage run", None, "Triage run"), + ("Review run", None, "Review run"), + ("Pickup run", None, "Pickup run"), + ( + "Process", + Some("nightly "), + "Process nightly <checks>", + ), + ("Process", None, "Process legacy-process"), + ]; + for (index, (kind, process, _)) in cases.iter().enumerate() { + let number = index as u64 + 1; + let mut fixture = record(owner.path(), number, Some("merged"), 0); + fixture["kind"] = json!(kind); + fixture["name"] = json!("legacy-process"); + fixture["process_name"] = json!(process); + fixture["issue"]["number"] = json!(42); + fixture["issue"]["url"] = json!("https://github.com/acme/widgets/issues/42"); + fixture["tickets"] = json!([]); + if *kind == "Pickup run" { + let mut child = fixture.clone(); + child["kind"] = json!("Run"); + fixture["dispatched"] = json!([child]); + } + super::write_record(owner.path(), number, &fixture); + } + let binary = Path::new(env!("CARGO_BIN_EXE_thirdshift")); + let local = Console::start(binary, owner.path()); + let token = super::tokens::create(owner.path(), "caller", &["--scopes", "read"]); + assert!( + super::fleet::add(caller.path(), "owner", &local.address, &token) + .status + .success() + ); + let fleet = Console::start(binary, caller.path()); + for console in [&local, &fleet] { + let _events = console.get("/events"); + let deadline = Instant::now() + Duration::from_secs(30); + let html = loop { + let html = console.get("/?section=done").text().unwrap(); + if html.contains("legacy-process") { + break html; + } + assert!(Instant::now() < deadline, "Fleet rows never arrived"); + std::thread::sleep(Duration::from_millis(25)); + }; + for (index, (_, _, label)) in cases.iter().enumerate() { + let selector = format!("{}-fixture.command.json", index + 1); + let head = section(&html, "done") + .split("
  • ") + .nth(1) + .unwrap() + .split("
    • ") + .next() + .unwrap(); + assert!( + head.contains(&format!("{label}")), + "{label} missing: {head}" + ); + assert!( + head.contains("href=\"https://github.com/acme/widgets/issues/42\""), + "{head}" + ); + assert!(head.contains(">merged"), "{head}"); + } + } +} + /// A Triage URL that fails the origin check is still recorded. The compact /// header must not lend the launch repository's name to that other issue. #[test]