Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions console/tests/inbox.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ test('Copy fix command works on ready Inbox rows and in Command details', async
await row.waitFor();
const copy = row.getByRole('button', { name: 'Copy fix command', exact: true });
assert.equal(await copy.count(), offered ? 1 : 0, `${width}:${outcome}:${ci}:${mergeable}`);
// This Console knows no Launch directory: retain the copy fallback alone.
assert.equal(await row.getByRole('button', { name: 'Start Fix run', exact: true }).count(), 0);
if (offered) {
assert.equal(await copy.getAttribute('type'), 'button');
assert.equal(await copy.getAttribute('data-copy-cli'), cli);
Expand Down
26 changes: 25 additions & 1 deletion console/tests/start-fixture.cjs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
const { spawn, execFileSync } = require('node:child_process');
const { once } = require('node:events');
const { mkdtemp, mkdir, writeFile, rm } = require('node:fs/promises');
const { mkdtemp, mkdir, readFile, writeFile, rm } = require('node:fs/promises');
const { tmpdir } = require('node:os');
const { join, resolve } = require('node:path');
const { createInterface } = require('node:readline');
const { chromium } = require('./playwright.cjs');
const { replaceRecord } = require('./records.cjs');

async function startFixture(extraEnv = {}) {
const home = await mkdtemp(join(tmpdir(), 'thirdshift-start-page-'));
Expand Down Expand Up @@ -65,6 +66,29 @@ else:
finally { lines.close(); }
browser = await chromium.launch({ headless: true });
return { home, repo, browser, origin, close,
async readyPullRequest(number = 42, changes = {}, github = {}) {
const logs = join(home, '.thirdshift/logs');
const directory = join(logs, 'acme/widgets/commands/review');
await mkdir(directory, { recursive: true });
const pull_request = changes.pull_request ?? `https://github.com/acme/widgets/pull/${number}`;
const now = new Date().toISOString();
const path = join(directory, `${number}-fixture.command.json`);
await replaceRecord(path, JSON.stringify({
version: 1, kind: 'Review run', repository: 'acme/widgets', hostname: 'fixture',
issue: null, process_id: 0, process_start: 'gone', harness: 'claude', model: 'old-model', effort: 'high',
started_at: now, ended_at: now, stages: [], outcome: 'ready for review', pull_request,
...changes
}));
const cache = join(logs, 'github-inbox.json');
const kept = JSON.parse(await readFile(cache, 'utf8').catch(error => {
if (error.code !== 'ENOENT') throw error;
return '{"version":1,"subjects":{}}';
}));
kept.subjects[pull_request] = { state: 'Open', ci: 'red', mergeable: 'mergeable', title: 'Repair these widgets', ...github };
await replaceRecord(cache, JSON.stringify(kept));
await writeFile(join(logs, 'github-inbox.changed'), `${number}:${JSON.stringify(github)}`);
return path;
},
cli(args, input) { return execFileSync(binary, args, { env, input, encoding: 'utf8' }); },
scopes(value) { execFileSync(binary, ['console', 'scopes', '--scopes', value], { env }); },
async page(options) {
Expand Down
211 changes: 210 additions & 1 deletion console/tests/start.test.cjs
Original file line number Diff line number Diff line change
@@ -1,13 +1,221 @@
const { test, before, after } = require('node:test');
const assert = require('node:assert/strict');
const { mkdir, writeFile } = require('node:fs/promises');
const { mkdir, readFile, writeFile, rm } = require('node:fs/promises');
const { join } = require('node:path');
const { startFixture } = require('./start-fixture.cjs');
const { replaceRecord } = require('./records.cjs');
const { hostname } = require('node:os');

let fixture;
before(async () => { fixture = await startFixture(); });
after(async () => { await fixture?.close(); });

test('Start Fix run confirms the pull request and defaults, cancels, and submits once', async t => {
await fixture.readyPullRequest();
for (const viewport of [{ width: 1440, height: 900 }, { width: 390, height: 844 }]) {
const page = await fixture.page({ viewport });
t.after(() => page.context().close());
const requests = [];
let respond;
let received;
const receiving = new Promise(resolve => { received = resolve; });
await page.route('**/api/v1/runs', async route => {
requests.push({ body: route.request().postDataJSON(), csrf: route.request().headers()['x-csrf-token'] });
received();
await new Promise(resolve => { respond = resolve; });
await route.fulfill({ json: { version: 1, name: 'acme/widgets fix #42' } });
});
const start = page.getByRole('button', { name: 'Start Fix run', exact: true });
await start.click({ timeout: 5000 });
const confirm = page.getByRole('dialog', { name: 'Confirm start', exact: true });
await confirm.waitFor();
const request = { command: 'fix', repository: 'acme/widgets', pull_request: 'https://github.com/acme/widgets/pull/42' };
assert.deepEqual(JSON.parse(await confirm.locator('pre').textContent()), request);
assert.match(await confirm.textContent(), /https:\/\/github.com\/acme\/widgets\/pull\/42/);
assert.ok((await confirm.textContent()).includes(`machine ${hostname()}`));
assert.match(await confirm.textContent(), /User config and the CLI defaults/);
assert.deepEqual(requests, []);
await confirm.getByRole('button', { name: 'Cancel', exact: true }).click();
await confirm.waitFor({ state: 'hidden' });
await start.click();
await page.keyboard.press('Escape');
await confirm.waitFor({ state: 'hidden' });
assert.deepEqual(requests, []);
await start.click();
await confirm.getByRole('button', { name: 'Start now', exact: true }).click();
try {
await page.locator('[data-start-result]').filter({ hasText: 'Starting…' }).waitFor();
await receiving;
assert.equal(await start.isDisabled(), true);
assert.equal(requests.length, 1);
assert.deepEqual(requests[0].body, request);
assert.ok(requests[0].csrf);
} finally { respond?.(); }
await page.locator('[data-start-result]').filter({ hasText: 'Started Command acme/widgets fix #42.' }).waitFor();
await start.click();
await confirm.waitFor();
await page.keyboard.press('Escape');
await confirm.waitFor({ state: 'hidden' });
assert.equal(await page.evaluate(() => document.documentElement.scrollWidth <= innerWidth), true);
await page.context().close();
}
});

test('Start Fix run requires an eligible row, current run permission and a known Launch directory', async t => {
const configPath = join(fixture.home, '.thirdshift/config.toml');
const config = await readFile(configPath, 'utf8');
const cronPath = join(fixture.home, 'crontab');
const cron = await readFile(cronPath, 'utf8');
t.after(async () => {
await replaceRecord(configPath, config);
await writeFile(cronPath, cron);
fixture.scopes('read,security,act,run,admin');
await fixture.readyPullRequest();
});
const page = await fixture.page();
t.after(() => page.context().close());
for (const [outcome, ci, mergeable, eligible] of [
['ready for review', 'red', 'mergeable', true],
['ready for review', 'green', 'conflicting', true],
['ready for review', 'pending', 'conflicting', true],
['ready for review', 'green', 'mergeable', false],
['ready for review', 'unknown', 'unknown', false],
['failed', 'red', 'conflicting', false],
['merged', 'red', 'conflicting', false],
]) {
await fixture.readyPullRequest(42, { outcome }, { ci, mergeable });
await page.goto(`${fixture.origin}/?section=${outcome === 'failed' ? 'needs-you' : outcome === 'merged' ? 'done' : 'ready-for-review'}`);
assert.equal(await page.getByRole('button', { name: 'Start Fix run', exact: true }).count(), Number(eligible), `${outcome}:${ci}:${mergeable}`);
}
await fixture.readyPullRequest();
for (const scopes of ['read', 'read,act', 'read,run']) {
fixture.scopes(scopes);
await page.goto(fixture.origin);
assert.equal(await page.getByRole('button', { name: 'Start Fix run', exact: true }).count(), Number(scopes.includes('run')));
assert.equal(await page.getByRole('button', { name: 'Copy fix command', exact: true }).count(), 1);
}
await replaceRecord(configPath, config.replace(/^launch_directories = .*\n/, ''));
await writeFile(cronPath, '');
await page.goto(fixture.origin);
assert.equal(await page.getByRole('button', { name: 'Start Fix run', exact: true }).count(), 0);
assert.equal(await page.getByRole('button', { name: 'Copy fix command', exact: true }).count(), 1);
});

test('Start Fix run recognizes GitHub repository identity across URL casing', async t => {
const pull_request = 'https://github.com/ACME/Widgets/pull/42';
await fixture.readyPullRequest(42, { pull_request });
t.after(() => fixture.readyPullRequest());
const page = await fixture.page();
t.after(() => page.context().close());
await page.getByRole('button', { name: 'Start Fix run', exact: true }).click();
const confirm = page.getByRole('dialog', { name: 'Confirm start', exact: true });
await confirm.waitFor();
assert.deepEqual(JSON.parse(await confirm.locator('pre').textContent()), {
command: 'fix', repository: 'ACME/Widgets', pull_request
});
await page.keyboard.press('Escape');
await confirm.waitFor({ state: 'hidden' });
});

test('Start Fix run works after live row and permission changes and preserves uncertain-response feedback', async t => {
await fixture.readyPullRequest(42, {}, { ci: 'green' });
fixture.scopes('read');
t.after(async () => { fixture.scopes('read,security,act,run,admin'); await fixture.readyPullRequest(); });
const page = await fixture.page();
t.after(() => page.context().close());
const start = page.locator('.command[data-command$="42-fixture.command.json"]').getByRole('button', { name: 'Start Fix run', exact: true });
assert.equal(await start.count(), 0);
fixture.scopes('read,run');
await fixture.readyPullRequest(42, {}, { ci: 'green', mergeable: 'conflicting' });
await start.waitFor();
await start.click();
const confirm = page.getByRole('dialog', { name: 'Confirm start', exact: true });
await confirm.waitFor();
await page.keyboard.press('Escape');
await confirm.waitFor({ state: 'hidden' });
let requests = 0, release, received;
const receiving = new Promise(resolve => { received = resolve; });
await page.route('**/api/v1/runs', async route => {
requests++;
received();
await new Promise(resolve => { release = resolve; });
await route.abort('failed');
});
await start.click();
await confirm.getByRole('button', { name: 'Start now', exact: true }).click();
await receiving;
try {
await fixture.readyPullRequest(42, { cause: 'Updated while start is pending' });
await page.getByText('Updated while start is pending', { exact: true }).waitFor({ state: 'attached' });
assert.equal(await start.isDisabled(), true);
assert.equal(requests, 1);
} finally { release(); }
const result = page.locator('[data-start-result]');
await result.filter({ hasText: 'Check Inbox and logs before retrying.' }).waitFor();
await fixture.readyPullRequest(42, { cause: 'Updated after uncertain response' });
await page.getByText('Updated after uncertain response', { exact: true }).waitFor({ state: 'attached' });
assert.match(await result.textContent(), /Check Inbox and logs before retrying/);
assert.equal(requests, 1);
const inserted = await fixture.readyPullRequest(43);
t.after(() => rm(inserted, { force: true }));
const newStart = page.locator('.command[data-command$="43-fixture.command.json"]').getByRole('button', { name: 'Start Fix run', exact: true });
await newStart.click();
await confirm.waitFor();
assert.equal(JSON.parse(await confirm.locator('pre').textContent()).pull_request, 'https://github.com/acme/widgets/pull/43');
await page.keyboard.press('Escape');
await confirm.waitFor({ state: 'hidden' });
fixture.scopes('read');
await start.waitFor({ state: 'detached' });
});

test('Start Fix run forwards only to the owning machine with its current run permission', async t => {
const owner = await startFixture();
t.after(() => owner.close());
const caller = await startFixture({ THIRDSHIFT_FLEET_POLL_MS: '100' });
t.after(() => caller.close());
await owner.readyPullRequest();
const token = owner.cli(['console', 'token', 'create', 'fix-caller', '--scopes', 'read,run']).trim();
caller.cli(['console', 'fleet', 'add', 'owner', owner.origin], `${token}\n`);
caller.scopes('read');
for (const viewport of [{ width: 1440, height: 900 }, { width: 390, height: 844 }]) {
const page = await caller.page({ viewport });
t.after(() => page.context().close());
const row = page.locator('.command[data-machine="owner"]');
const start = row.getByRole('button', { name: 'Start Fix run', exact: true });
await start.click();
const confirm = page.getByRole('dialog', { name: 'Confirm start', exact: true });
await confirm.waitFor();
assert.match(await confirm.textContent(), /machine owner/);
const requests = [];
page.on('request', request => {
if (request.method() === 'POST' && /\/runs(?:\?|$)/.test(request.url())) {
requests.push({ path: new URL(request.url()).pathname + new URL(request.url()).search, body: request.postDataJSON() });
}
});
const forwarding = page.waitForResponse(response => response.url().includes('/fleet/runs?machine=owner'));
await confirm.getByRole('button', { name: 'Start now', exact: true }).click();
// The fake GitHub refuses PR admission. This still exercises real forwarding,
// destination authorization, CSRF and attribution, without starting a Harness.
const response = await forwarding;
const failure = (await response.json()).error;
assert.equal(response.status(), 422, failure);
await page.locator('[data-start-result]').filter({ hasText: failure }).waitFor();
assert.deepEqual(requests, [{ path: '/fleet/runs?machine=owner', body: {
command: 'fix', repository: 'acme/widgets', pull_request: 'https://github.com/acme/widgets/pull/42'
} }]);
assert.match(owner.cli(['console', 'log', '--json']), /owner@example.test from the Console on fix-caller/);
assert.equal(await page.evaluate(() => document.documentElement.scrollWidth <= innerWidth), true);
await page.context().close();
}
const page = await caller.page();
t.after(() => page.context().close());
const row = page.locator('.command[data-machine="owner"]');
await row.getByRole('button', { name: 'Start Fix run', exact: true }).waitFor();
owner.scopes('read');
await row.getByRole('button', { name: 'Start Fix run', exact: true }).waitFor({ state: 'detached' });
await row.getByRole('button', { name: 'Copy fix command', exact: true }).waitFor();
});

test('Start requires confirmation, names its Harness and shows the Command in Inbox', async t => {
for (const viewport of [{ width: 1440, height: 900 }, { width: 390, height: 844 }]) {
await t.test(`${viewport.width}px`, async t => {
Expand Down Expand Up @@ -110,6 +318,7 @@ test('Start reviews every shared option before submission and remains inert with
t.after(() => noScript.context().close());
await noScript.getByText('Start work', { exact: true }).click();
assert.equal(await noScript.getByRole('button', { name: 'Review start', exact: true }).isDisabled(), true);
assert.equal(await noScript.getByRole('button', { name: 'Start Fix run', exact: true }).isDisabled(), true);
await noScript.goto(`${fixture.origin}/cronjobs`);
assert.equal(await noScript.getByRole('button', { name: 'Run now', exact: true }).isDisabled(), true);
});
Expand Down
12 changes: 8 additions & 4 deletions docs/guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ Ready for review rows show the pull request's latest head CI as **CI green**, **

A conflicting pull request also shows a separate **Merge conflict** pill beside CI, even when CI is green. It stays in Ready for review. The same GitHub read refreshes mergeability and wakes open pages when it changes; `status` prints **Merge conflict** too. `status --json` includes `github_mergeable` (`mergeable`, `conflicting` or `unknown`). Only `conflicting` shows the pill; unknown and older saved answers do not claim a conflict. Failed reads retain the last answer with the same freshness warning.

Ready for review rows with a pull request and either **CI red** or **Merge conflict** show `thirdshift fix <PR URL>` and a **Copy fix command** button, both on the Inbox row and in Command details (Peek). Clicking it copies the command; run it from a clone of that repository to start a [Fix run](#fix-runs). A later Command on the same pull request URL takes up the earlier waiting row in Ready for review or Needs you, just as one on the same issue does, including when neither Command has a linked issue. The row says `taken up by a later Command started at <time>`, and retention counts from that later start.
Ready for review rows with a pull request and either **CI red** or **Merge conflict** offer **Start Fix run** when the owning machine currently permits `run` and knows a Launch directory for that repository. Review the pull request, repository and owning machine, then confirm to start a [Fix run](#fix-runs) there. Cancel or Escape starts nothing. Omitted options use that machine's User config and CLI defaults; the earlier Command's options are not replayed. Other-account Commands stay read-only. The row and Command details (Peek) also retain `thirdshift fix <PR URL>` with **Copy fix command**; copy it and run it from a clone of that repository. A later Command on the same pull request URL takes up the earlier waiting row in Ready for review or Needs you, just as one on the same issue does, including when neither Command has a linked issue. The row says `taken up by a later Command started at <time>`, and retention counts from that later start.

Each row with a kept answer also carries `github`: `subjects` maps its own or dispatched issue and pull request URLs to their `facts` and RFC 3339 `answered_at`. An unresolved URL has null facts; an older cache without an observation time has null `answered_at`. Finding rows include `github.findings` with per-URL waiting flags, the first observed `triaged_at`, and `answered_at`, without private write-ups. History freezes these observation times with its facts. The existing `github_state` and `github_at` fields still describe the row's own pull request and its merge or close event.

Expand Down Expand Up @@ -3205,9 +3205,13 @@ Running the Spec again picks up where the last Spec run stopped. The Spec branch
## Fix runs

Use a **Fix run** when a delivered pull request needs its Base branch merged in,
a merge conflict resolved or CI repaired. The Console offers **Copy fix command**
on Ready for review rows with CI red or Merge conflict and in their Command
details. Starting it takes up earlier waiting Inbox rows on that pull request,
a merge conflict resolved or CI repaired. On Ready for review rows with CI red
or Merge conflict, the Console offers **Start Fix run** with confirmation of the
pull request, repository and owning machine. It requires current `run` permission
and a known Launch directory on that machine, and uses its User config and CLI
defaults. The Console shows the returned Command or failure; after an uncertain
response, check Inbox and logs before retrying. **Copy fix command** remains on
the row and in Command details. Starting a Fix run takes up earlier waiting Inbox rows on that pull request,
whether or not they name the same issue. Other later Commands on that pull
request make the same hand-off.

Expand Down
Loading