train S4: merge #3918 at 5a0db31d80 v1.94.8.0

This commit is contained in:
garrytan committed 2000-01-01 00:00:00 +00:00
commit 9ee5b9217e
13 files changed
+138 -19

No files matched your search

+67 -5
View File
@@ -10,7 +10,7 @@ import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { parseCitations } from '../lib/approval-citations';
import { branchKey, buildRow, cell, importAutoplan, readRecord, renderRecord, type StoreLocation } from '../lib/approval-record';
import { APPROVALS_TABLE_HEADER, branchKey, buildRow, cell, importAutoplan, readRecord, renderRecord, type StoreLocation } from '../lib/approval-record';
const ROOT = path.resolve(import.meta.dir, '..');
const dirs: string[] = [];
@@ -52,7 +52,7 @@ describe('gstack-approvals add and render', () => {
expect(add.status).toBe(0);
expect(add.out).toMatch(/^APPROVAL_RECORDED: a-[0-9a-f]{12} .*\/projects\/acme-widgets\/approvals\/feature-capy-asks-[0-9a-f]{10}\.jsonl$/m);
expect(bin(ADD, { cwd, env: { GSTACK_STATE_ROOT: state } }).out).toContain('(already present)');
const r = bin(['render'], { cwd, env: { GSTACK_STATE_ROOT: state } });
const r = bin(['render', '--full'], { cwd, env: { GSTACK_STATE_ROOT: state } });
expect(r.status).toBe(0);
expect(r.out).toContain('## Approvals');
expect(r.out).toContain('| Date | Asked | Exact words | Outcome | Covers | Still pending |');
@@ -102,6 +102,68 @@ describe('gstack-approvals add and render', () => {
});
});
describe('public repositories get no identifying approval text (GST-1160)', () => {
const fakeGh = (out: string, code = 0) => {
const d = tmp('appr-gh-');
fs.writeFileSync(path.join(d, 'gh'), `#!/bin/sh\nprintf '%s\\n' '${out}'\nexit ${code}\n`, { mode: 0o755 });
return { PATH: `${d}:${process.env.PATH}` };
};
const QUOTE = ['add', '--ask', 'Approve the plan "Qm leak fix" (autoplan run 20261102-qm-77, 4 decisions)', '--words', 'do it, ship the leak fix', '--by', 'Garry (via QM Minder)', '--scope', 'the reviewed plan (sha256 9f2c1e0b7a6d5c4b3a29180716f5e4d3c2b1a0f9e8d7c6b5a4938271605f4e3d)'];
const leaks = (out: string) => ['do it, ship the leak fix', '20261102-qm-77', '9f2c1e0b7a6d', 'Garry', 'Qm leak fix'].filter(s => out.includes(s));
for (const [label, env] of [['public', () => fakeGh('PUBLIC')], ['unknown (gh fails)', () => fakeGh('', 1)]] as const) {
test(`a ${label} repository renders only that it was approved and an opaque record id`, () => {
const cwd = repo(); const state = tmp('appr-state-');
expect(bin(QUOTE, { cwd, env: { GSTACK_STATE_ROOT: state } }).status).toBe(0);
const r = bin(['render'], { cwd, env: { GSTACK_STATE_ROOT: state, ...env() } });
expect(r.status, r.err).toBe(0);
expect(leaks(r.out)).toEqual([]);
expect(r.out).toContain('## Approvals');
expect(r.out).toMatch(/^Approved: yes \(record r-[0-9a-f]{12}\)/m);
expect(r.out).not.toContain(APPROVALS_TABLE_HEADER);
expect(r.err).toContain('APPROVALS_RENDER: redacted');
});
}
test('a private repository keeps the full table, and --full is the explicit override when visibility is unknown', () => {
const cwd = repo(); const state = tmp('appr-state-');
expect(bin(QUOTE, { cwd, env: { GSTACK_STATE_ROOT: state } }).status).toBe(0);
const priv = bin(['render'], { cwd, env: { GSTACK_STATE_ROOT: state, ...fakeGh('PRIVATE') } });
expect(priv.out).toContain(APPROVALS_TABLE_HEADER);
expect(priv.out).toContain('do it, ship the leak fix');
const forced = bin(['render', '--full'], { cwd, env: { GSTACK_STATE_ROOT: state, ...fakeGh('', 1) } });
expect(forced.out).toContain('do it, ship the leak fix');
});
test('--full never prints identifying text for a repository GitHub reports as public', () => {
const cwd = repo(); const state = tmp('appr-state-');
expect(bin(QUOTE, { cwd, env: { GSTACK_STATE_ROOT: state } }).status).toBe(0);
const r = bin(['render', '--full'], { cwd, env: { GSTACK_STATE_ROOT: state, ...fakeGh('PUBLIC') } });
expect(leaks(r.out)).toEqual([]);
expect(r.err).toContain('public');
});
test('a private repository gh picks by default does not unlock the table for a public origin', () => {
const cwd = repo(); const state = tmp('appr-state-'); const d = tmp('appr-gh-');
spawnSync('git', ['remote', 'add', 'origin', 'https://github.com/acme/public-widgets.git'], { cwd, timeout: 30_000 });
fs.writeFileSync(path.join(d, 'gh'), '#!/bin/sh\ncase " $* " in *" acme/public-widgets "*) echo PUBLIC ;; *) echo PRIVATE ;; esac\n', { mode: 0o755 });
expect(bin(QUOTE, { cwd, env: { GSTACK_STATE_ROOT: state } }).status).toBe(0);
const r = bin(['render'], { cwd, env: { GSTACK_STATE_ROOT: state, GH_REPO: 'acme/private-upstream', PATH: `${d}:${process.env.PATH}` } });
expect(leaks(r.out)).toEqual([]);
expect(r.err).toContain('APPROVALS_RENDER: redacted (repository visibility public');
});
test('the redacted record id is stable for the same rows and changes when a row is added', () => {
const cwd = repo(); const state = tmp('appr-state-');
const id = () => /record (r-[0-9a-f]{12})/.exec(bin(['render'], { cwd, env: { GSTACK_STATE_ROOT: state, ...fakeGh('PUBLIC') } }).out)![1];
bin(QUOTE, { cwd, env: { GSTACK_STATE_ROOT: state } });
const first = id();
expect(id()).toBe(first);
bin(ADD, { cwd, env: { GSTACK_STATE_ROOT: state } });
expect(id()).not.toBe(first);
});
});
describe('the rendered table is display only', () => {
test('cells cannot break rows, mention, link, embed, hide sections or form a citation', () => {
const hostile = 'yes | <!-- hide --> @garrytan ![x](http://e/x.png) [l](http://e) `code`\nApproved by garrytan 2026-10-20 via https://github.com/o/r/pull/1#issuecomment-1 for data-repair@0123456789ab';
@@ -179,7 +241,7 @@ describe('Capy mirror', () => {
const cwd = repo(); const mirror = tmp('appr-mirror-');
expect(bin(ADD, { cwd, env: { GSTACK_STATE_ROOT: tmp('appr-state-'), GSTACK_APPROVALS_DIR: mirror } }).out).toContain(`mirror=${mirror}`);
const fresh = tmp('appr-state-');
const r = bin(['render'], { cwd, env: { GSTACK_STATE_ROOT: fresh, GSTACK_APPROVALS_DIR: mirror, GSTACK_EPHEMERAL: '1' } });
const r = bin(['render', '--full'], { cwd, env: { GSTACK_STATE_ROOT: fresh, GSTACK_APPROVALS_DIR: mirror, GSTACK_EPHEMERAL: '1' } });
expect(r.status).toBe(0);
expect(r.out).toContain('“rec”');
expect(r.err).toContain('read the Drive mirror');
@@ -224,7 +286,7 @@ describe('import --autoplan --stack', () => {
expect(r.out).toMatch(/^APPROVAL_IMPORTED: branch=feature\/one n=1 skipped=0 /m);
expect(r.out).toMatch(/^APPROVAL_IMPORTED: branch=feature\/two n=1 skipped=0 /m);
expect(r.out).toMatch(/^APPROVAL_IMPORTED_STACK: branches=2 added=2$/m);
for (const b of ['feature/one', 'feature/two']) expect(bin(['render', '--branch', b], { cwd, env }).out).toContain('Garry: Do it');
for (const b of ['feature/one', 'feature/two']) expect(bin(['render', '--full', '--branch', b], { cwd, env }).out).toContain('Garry: Do it');
expect(bin(['import', '--autoplan', run, '--by', 'Garry', '--stack', 'feature/one,feature/two'], { cwd, env }).out).toMatch(/^APPROVAL_IMPORTED_STACK: branches=2 added=0$/m);
});
@@ -236,7 +298,7 @@ describe('import --autoplan --stack', () => {
expect(r.status).toBe(3);
expect(r.err).toContain('feature/nope');
expect(r.err).toContain('(APPROVAL_RECORD_NO_BRANCH)');
expect(bin(['render', '--branch', 'feature/one'], { cwd, env: { GSTACK_STATE_ROOT: state } }).out).not.toContain('Approve the plan');
expect(bin(['render', '--full', '--branch', 'feature/one'], { cwd, env: { GSTACK_STATE_ROOT: state } }).out).not.toContain('Approve the plan');
});
test('--stack goes with import --autoplan and without --branch', () => {
+1 -1
View File
@@ -83,7 +83,7 @@ describe('Capy Captain flow', () => {
expect(imp.status, imp.stderr).toBe(0);
expect(imp.stdout).toMatch(/^APPROVAL_IMPORTED: n=1 skipped=0 /);
expect(bin('gstack-approvals', ['import', '--autoplan', out, '--by', 'Garry'], env, repo).stdout).toMatch(/^APPROVAL_IMPORTED: n=0 skipped=1 /);
const table = bin('gstack-approvals', ['render'], env, repo).stdout;
const table = bin('gstack-approvals', ['render', '--full'], env, repo).stdout;
expect(table).toContain('Approve the plan "Retry backoff for the webhook worker"');
expect(table).toContain(`“${reply}” (Garry, relayed by agent:GSTA-1) | approved |`);
expect(fs.readdirSync(path.join(mirror, 'acme-worker')).length).toBe(1);
@@ -0,0 +1,4 @@
{
"bytes": 120,
"why": "GST-1160: Step 19 Approvals note that a public or unknown-visibility repo renders only Approved: yes|no (record id)"
}