mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-06 08:54:04 +01:00
fix(ship): PR body drafts and rejected test paths are agent-written files
Co-authored-by: dgrant <dgrant@users.noreply.github.com> The PR body is written into two private drafts (through ## Documentation, and from ## Test plan on); the scan block splices Step 14.5's section between them into the scanned temp file and deletes the drafts. Rejected test paths are read from a written list, and paths outside the repository are skipped. Tests that ran the old heredoc blocks now write the drafts and list first.
This commit is contained in:
1 parent
5436d33fae
commit
e79b23f24f
6 files changed
+99
-38
No files matched your search
+23
-11
@@ -94,7 +94,8 @@ Unavailable/inconclusive is never PASS.>
|
||||
<Embed Step 14.5's vetted nonempty `documentation_section` for this invocation:
|
||||
its saved section file, inserted unchanged by the scan block's `DOCS_SECTION_FILE` lines.
|
||||
A blocked audit shipped under a user exception has no section file: state its
|
||||
blocked status, scope and exception here and omit those lines.>
|
||||
blocked status, scope and exception here and drop the block's `DOCS_SECTION_FILE` guard
|
||||
and its `cat -- "$DOCS_SECTION_FILE" && echo &&` step.>
|
||||
<Always include the status and reviewed scope: updated, current, or blocked with the actual user's named risk exception. Never omit this section or reuse another invocation's audit.>
|
||||
|
||||
## Test plan
|
||||
@@ -106,15 +107,29 @@ blocked status, scope and exception here and omit those lines.>
|
||||
#### Redaction scan (PR body + title) — runs before create AND edit
|
||||
|
||||
The PR body is world-readable on a public repo. Scan-at-sink before sending:
|
||||
write the composed body to a temp file, scan THAT file with the shared engine,
|
||||
compose the body into a temp file, scan THAT file with the shared engine,
|
||||
and pass the same file to `gh`/`glab`. Wrap any Codex / Greptile / eval output
|
||||
sections in tool-attributed fences (` ```codex-review ` / ` ```greptile `) so the
|
||||
engine WARN-degrades the example credentials those tools quote instead of blocking
|
||||
the PR (a live-format credential inside the fence still blocks).
|
||||
|
||||
Write the body from above into two private files: the first through the
|
||||
`## Documentation` heading line, the second from `## Test plan` on.
|
||||
|
||||
```bash
|
||||
_GT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)/.gstack/tmp"
|
||||
mkdir -p "$_GT" && chmod 700 "$_GT" || { echo "Not sent: cannot create $_GT for the text file." >&2; exit 1; }
|
||||
_EX=$(git rev-parse --git-path info/exclude 2>/dev/null) && mkdir -p "$(dirname "$_EX")" && { grep -qxF '/.gstack/tmp/' "$_EX" 2>/dev/null || echo '/.gstack/tmp/' >> "$_EX"; }
|
||||
BODY_TOP_FILE=$(mktemp "$_GT/pr-body-top.XXXXXX") && echo "BODY_TOP_FILE: $BODY_TOP_FILE (name: ${BODY_TOP_FILE##*/})"
|
||||
BODY_REST_FILE=$(mktemp "$_GT/pr-body-rest.XXXXXX") && echo "BODY_REST_FILE: $BODY_REST_FILE (name: ${BODY_REST_FILE##*/})"
|
||||
```
|
||||
|
||||
Write the text into each printed file with your file-write tool (Claude Code's Write tool needs a Read of the empty file first), exactly as it should appear. The text never goes into a shell command, heredoc or quoted argument. If a write fails or is refused, do not send: print the cause, the file path and the command below for sending by hand.
|
||||
|
||||
Use Step 18's `NEW_TITLE` unchanged; its version prefix is already present.
|
||||
In a new shell, restore the saved literal title before this block, and Step 14.5's
|
||||
saved section file path as `DOCS_SECTION_FILE`.
|
||||
saved section file path as `DOCS_SECTION_FILE`. Substitute the printed names. The
|
||||
block deletes both drafts; to change the body, write fresh ones.
|
||||
|
||||
```bash
|
||||
: "${NEW_TITLE:?Restore the saved Step 18 title before scanning}"
|
||||
@@ -123,14 +138,11 @@ REDACT_VIS=$(~/.claude/skills/gstack/bin/gstack-config get redact_repo_visibilit
|
||||
[ -z "$REDACT_VIS" ] && REDACT_VIS=$(gh repo view --json visibility -q .visibility 2>/dev/null | tr 'A-Z' 'a-z')
|
||||
REDACT_VIS="${REDACT_VIS:-unknown}"
|
||||
PR_BODY_FILE=$(mktemp "${TMPDIR:-/tmp}/gstack-pr-body.XXXXXX") || { echo "ERROR: mktemp failed — cannot scan the PR body; refusing to create the PR unscanned." >&2; exit 1; }
|
||||
{ cat <<'PR_BODY_EOF'
|
||||
<PR body from above, through the "## Documentation" heading line>
|
||||
PR_BODY_EOF
|
||||
cat -- "$DOCS_SECTION_FILE" && echo || exit 1
|
||||
cat <<'PR_BODY_EOF'
|
||||
<rest of the PR body from above>
|
||||
PR_BODY_EOF
|
||||
} > "$PR_BODY_FILE" || exit 1
|
||||
BODY_TOP_FILE="$(git rev-parse --show-toplevel 2>/dev/null || pwd)/.gstack/tmp/<body-top-file-name>"
|
||||
BODY_REST_FILE="$(git rev-parse --show-toplevel 2>/dev/null || pwd)/.gstack/tmp/<body-rest-file-name>"
|
||||
[ -s "$BODY_TOP_FILE" ] && [ -s "$BODY_REST_FILE" ] || { echo "Not scanned: $BODY_TOP_FILE or $BODY_REST_FILE is empty, so the body was never written. Write both, then rerun." >&2; exit 1; }
|
||||
{ awk 1 "$BODY_TOP_FILE" && cat -- "$DOCS_SECTION_FILE" && echo && awk 1 "$BODY_REST_FILE"; } > "$PR_BODY_FILE" || exit 1
|
||||
rm -f "$BODY_TOP_FILE" "$BODY_REST_FILE"
|
||||
~/.claude/skills/gstack/bin/gstack-redact --from-file "$PR_BODY_FILE" --repo-visibility "$REDACT_VIS" --self-email "$(git config user.email 2>/dev/null)" --json
|
||||
case $? in
|
||||
0) ;;
|
||||
|
||||
@@ -92,7 +92,8 @@ Unavailable/inconclusive is never PASS.>
|
||||
<Embed Step 14.5's vetted nonempty `documentation_section` for this invocation:
|
||||
its saved section file, inserted unchanged by the scan block's `DOCS_SECTION_FILE` lines.
|
||||
A blocked audit shipped under a user exception has no section file: state its
|
||||
blocked status, scope and exception here and omit those lines.>
|
||||
blocked status, scope and exception here and drop the block's `DOCS_SECTION_FILE` guard
|
||||
and its `cat -- "$DOCS_SECTION_FILE" && echo &&` step.>
|
||||
<Always include the status and reviewed scope: updated, current, or blocked with the actual user's named risk exception. Never omit this section or reuse another invocation's audit.>
|
||||
|
||||
## Test plan
|
||||
@@ -104,15 +105,21 @@ blocked status, scope and exception here and omit those lines.>
|
||||
#### Redaction scan (PR body + title) — runs before create AND edit
|
||||
|
||||
The PR body is world-readable on a public repo. Scan-at-sink before sending:
|
||||
write the composed body to a temp file, scan THAT file with the shared engine,
|
||||
compose the body into a temp file, scan THAT file with the shared engine,
|
||||
and pass the same file to `gh`/`glab`. Wrap any Codex / Greptile / eval output
|
||||
sections in tool-attributed fences (` ```codex-review ` / ` ```greptile `) so the
|
||||
engine WARN-degrades the example credentials those tools quote instead of blocking
|
||||
the PR (a live-format credential inside the fence still blocks).
|
||||
|
||||
Write the body from above into two private files: the first through the
|
||||
`## Documentation` heading line, the second from `## Test plan` on.
|
||||
|
||||
{{FREE_TEXT_FILE:BODY_TOP_FILE=pr-body-top:BODY_REST_FILE=pr-body-rest}}
|
||||
|
||||
Use Step 18's `NEW_TITLE` unchanged; its version prefix is already present.
|
||||
In a new shell, restore the saved literal title before this block, and Step 14.5's
|
||||
saved section file path as `DOCS_SECTION_FILE`.
|
||||
saved section file path as `DOCS_SECTION_FILE`. Substitute the printed names. The
|
||||
block deletes both drafts; to change the body, write fresh ones.
|
||||
|
||||
```bash
|
||||
: "${NEW_TITLE:?Restore the saved Step 18 title before scanning}"
|
||||
@@ -121,14 +128,11 @@ REDACT_VIS=$(~/.claude/skills/gstack/bin/gstack-config get redact_repo_visibilit
|
||||
[ -z "$REDACT_VIS" ] && REDACT_VIS=$(gh repo view --json visibility -q .visibility 2>/dev/null | tr 'A-Z' 'a-z')
|
||||
REDACT_VIS="${REDACT_VIS:-unknown}"
|
||||
PR_BODY_FILE=$(mktemp "${TMPDIR:-/tmp}/gstack-pr-body.XXXXXX") || { echo "ERROR: mktemp failed — cannot scan the PR body; refusing to create the PR unscanned." >&2; exit 1; }
|
||||
{ cat <<'PR_BODY_EOF'
|
||||
<PR body from above, through the "## Documentation" heading line>
|
||||
PR_BODY_EOF
|
||||
cat -- "$DOCS_SECTION_FILE" && echo || exit 1
|
||||
cat <<'PR_BODY_EOF'
|
||||
<rest of the PR body from above>
|
||||
PR_BODY_EOF
|
||||
} > "$PR_BODY_FILE" || exit 1
|
||||
BODY_TOP_FILE="$(git rev-parse --show-toplevel 2>/dev/null || pwd)/.gstack/tmp/<body-top-file-name>"
|
||||
BODY_REST_FILE="$(git rev-parse --show-toplevel 2>/dev/null || pwd)/.gstack/tmp/<body-rest-file-name>"
|
||||
[ -s "$BODY_TOP_FILE" ] && [ -s "$BODY_REST_FILE" ] || { echo "Not scanned: $BODY_TOP_FILE or $BODY_REST_FILE is empty, so the body was never written. Write both, then rerun." >&2; exit 1; }
|
||||
{ awk 1 "$BODY_TOP_FILE" && cat -- "$DOCS_SECTION_FILE" && echo && awk 1 "$BODY_REST_FILE"; } > "$PR_BODY_FILE" || exit 1
|
||||
rm -f "$BODY_TOP_FILE" "$BODY_REST_FILE"
|
||||
~/.claude/skills/gstack/bin/gstack-redact --from-file "$PR_BODY_FILE" --repo-visibility "$REDACT_VIS" --self-email "$(git config user.email 2>/dev/null)" --json
|
||||
case $? in
|
||||
0) ;;
|
||||
|
||||
@@ -336,15 +336,29 @@ After your analysis, output a single JSON object on the LAST LINE of your respon
|
||||
caller check is `needs_seam`). Move each failure to `tests_rejected` with its
|
||||
`reason_code`, then remove it before anything else reads the diff: an untracked new
|
||||
file is deleted; for a tracked file, revert only this run's hunk with Edit, never
|
||||
the whole file.
|
||||
the whole file. Write one rejected test path per line, relative to the repository
|
||||
root, into a private file:
|
||||
|
||||
```bash
|
||||
_GT="$(git rev-parse --show-toplevel 2>/dev/null || pwd)/.gstack/tmp"
|
||||
mkdir -p "$_GT" && chmod 700 "$_GT" || { echo "Not sent: cannot create $_GT for the text file." >&2; exit 1; }
|
||||
_EX=$(git rev-parse --git-path info/exclude 2>/dev/null) && mkdir -p "$(dirname "$_EX")" && { grep -qxF '/.gstack/tmp/' "$_EX" 2>/dev/null || echo '/.gstack/tmp/' >> "$_EX"; }
|
||||
REJECTED_FILE=$(mktemp "$_GT/rejected-tests.XXXXXX") && echo "REJECTED_FILE: $REJECTED_FILE (name: ${REJECTED_FILE##*/})"
|
||||
```
|
||||
|
||||
Write the text into each printed file with your file-write tool (Claude Code's Write tool needs a Read of the empty file first), exactly as it should appear. The text never goes into a shell command, heredoc or quoted argument. If a write fails or is refused, do not send: print the cause, the file path and the command below for sending by hand.
|
||||
|
||||
Then run, substituting the printed name for `<rejected-file-name>`:
|
||||
|
||||
```bash
|
||||
REJECTED_FILE="$(git rev-parse --show-toplevel 2>/dev/null || pwd)/.gstack/tmp/<rejected-file-name>"
|
||||
[ -s "$REJECTED_FILE" ] || { echo "Nothing removed: $REJECTED_FILE is empty; write the paths, then rerun." >&2; exit 1; }
|
||||
while IFS= read -r f; do
|
||||
[ -n "$f" ] || continue
|
||||
case "$f" in /*|..|../*|*/..|*/../*) echo "SKIPPED (outside repo): $f"; continue ;; esac
|
||||
if git ls-files --error-unmatch -- "$f" >/dev/null 2>&1; then echo "REVERT_HUNK: $f"; else rm -f -- "$f" && echo "REMOVED: $f"; fi
|
||||
done <<'REJECTED'
|
||||
<one rejected test path per line>
|
||||
REJECTED
|
||||
done < "$REJECTED_FILE"
|
||||
rm -f "$REJECTED_FILE"
|
||||
```
|
||||
|
||||
No `tests_rejected` path may remain on disk as a new file. If every test written in
|
||||
|
||||
@@ -58,15 +58,22 @@ After your analysis, output a single JSON object on the LAST LINE of your respon
|
||||
caller check is `needs_seam`). Move each failure to `tests_rejected` with its
|
||||
`reason_code`, then remove it before anything else reads the diff: an untracked new
|
||||
file is deleted; for a tracked file, revert only this run's hunk with Edit, never
|
||||
the whole file.
|
||||
the whole file. Write one rejected test path per line, relative to the repository
|
||||
root, into a private file:
|
||||
|
||||
{{FREE_TEXT_FILE:REJECTED_FILE=rejected-tests}}
|
||||
|
||||
Then run, substituting the printed name for `<rejected-file-name>`:
|
||||
|
||||
```bash
|
||||
REJECTED_FILE="$(git rev-parse --show-toplevel 2>/dev/null || pwd)/.gstack/tmp/<rejected-file-name>"
|
||||
[ -s "$REJECTED_FILE" ] || { echo "Nothing removed: $REJECTED_FILE is empty; write the paths, then rerun." >&2; exit 1; }
|
||||
while IFS= read -r f; do
|
||||
[ -n "$f" ] || continue
|
||||
case "$f" in /*|..|../*|*/..|*/../*) echo "SKIPPED (outside repo): $f"; continue ;; esac
|
||||
if git ls-files --error-unmatch -- "$f" >/dev/null 2>&1; then echo "REVERT_HUNK: $f"; else rm -f -- "$f" && echo "REMOVED: $f"; fi
|
||||
done <<'REJECTED'
|
||||
<one rejected test path per line>
|
||||
REJECTED
|
||||
done < "$REJECTED_FILE"
|
||||
rm -f "$REJECTED_FILE"
|
||||
```
|
||||
|
||||
No `tests_rejected` path may remain on disk as a new file. If every test written in
|
||||
|
||||
@@ -1,10 +1,18 @@
|
||||
import { expect, test } from 'bun:test';
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join, resolve } from 'node:path';
|
||||
import { expectMentions } from './helpers/prompt-structure';
|
||||
|
||||
const template = readFileSync(new URL('../ship/sections/pr-body.md.tmpl', import.meta.url), 'utf8');
|
||||
|
||||
/** The agent writes the body drafts with its file-write tool; the block reads them by printed name. */
|
||||
function writeDrafts(root: string, top = '## Summary\n\n## Documentation', rest = '## Test plan\n- [x] bun test: pass'): (block: string) => string {
|
||||
mkdirSync(join(root, '.gstack/tmp'), { recursive: true });
|
||||
writeFileSync(join(root, '.gstack/tmp/pr-body-top.abc123'), top);
|
||||
writeFileSync(join(root, '.gstack/tmp/pr-body-rest.abc123'), rest);
|
||||
return block => block.replace('<body-top-file-name>', 'pr-body-top.abc123').replace('<body-rest-file-name>', 'pr-body-rest.abc123');
|
||||
}
|
||||
const scanner = resolve(import.meta.dir, '../bin/gstack-redact');
|
||||
|
||||
for (const failure of ['none', 'body', 'title'] as const) {
|
||||
@@ -26,7 +34,7 @@ exec ${JSON.stringify(process.execPath)} ${JSON.stringify(scanner)} "$@"
|
||||
const block = template.match(/```bash\n(: "\$\{NEW_TITLE:[\s\S]*?)\n```/)?.[1];
|
||||
expect(block).toBeDefined();
|
||||
writeFileSync(join(root, 'section.md'), '**Status:** current — no edits.');
|
||||
const result = Bun.spawnSync(['bash', '-c', block!], {
|
||||
const result = Bun.spawnSync(['bash', '-c', writeDrafts(root)(block!)], {
|
||||
cwd: root,
|
||||
env: { ...process.env, HOME: root, NEW_TITLE: 'v1.2.3.4 fix: publication checks', FAIL_AT: failure,
|
||||
DOCS_SECTION_FILE: join(root, 'section.md') },
|
||||
@@ -58,15 +66,24 @@ test('publication composes the saved documentation section unchanged and refuses
|
||||
try {
|
||||
const section = '**Status:** current — no edits.\n\n- Diagram drift: none (no diagrams in any doc).';
|
||||
writeFileSync(join(root, 'section.md'), section);
|
||||
const run = (file: string) => Bun.spawnSync(['bash', '-c', `${compose}cat -- "$PR_BODY_FILE"`], {
|
||||
const run = (file: string) => Bun.spawnSync(['bash', '-c', `${writeDrafts(root)(compose)}cat -- "$PR_BODY_FILE"`], {
|
||||
cwd: root, env: { ...process.env, HOME: root, DOCS_SECTION_FILE: file }, stdout: 'pipe', stderr: 'pipe', timeout: 10_000,
|
||||
});
|
||||
const composed = run(join(root, 'section.md'));
|
||||
expect(composed.exitCode).toBe(0);
|
||||
expect(composed.stdout.toString()).toContain(`## Documentation" heading line>\n${section}\n<rest of the PR body`);
|
||||
expect(composed.stdout.toString()).toBe(`## Summary\n\n## Documentation\n${section}\n## Test plan\n- [x] bun test: pass\n`);
|
||||
expect(existsSync(join(root, '.gstack/tmp/pr-body-top.abc123'))).toBe(false);
|
||||
expect(existsSync(join(root, '.gstack/tmp/pr-body-rest.abc123'))).toBe(false);
|
||||
const missing = run(join(root, 'absent.md'));
|
||||
expect(missing.exitCode).toBe(1);
|
||||
expect(missing.stdout.toString()).toBe('');
|
||||
rmSync(join(root, '.gstack/tmp'), { recursive: true, force: true });
|
||||
const unwritten = Bun.spawnSync(['bash', '-c', `${compose.replace('<body-top-file-name>', 'never-written').replace('<body-rest-file-name>', 'never-written')}cat -- "$PR_BODY_FILE"`], {
|
||||
cwd: root, env: { ...process.env, HOME: root, DOCS_SECTION_FILE: join(root, 'section.md') }, stdout: 'pipe', stderr: 'pipe', timeout: 10_000,
|
||||
});
|
||||
expect(unwritten.exitCode).toBe(1);
|
||||
expect(unwritten.stdout.toString()).toBe('');
|
||||
expect(unwritten.stderr.toString()).toContain('the body was never written');
|
||||
const unset = Bun.spawnSync(['bash', '-c', block], {
|
||||
cwd: root, env: { ...process.env, HOME: root, NEW_TITLE: 'v1.2.3.4 fix: publication checks', DOCS_SECTION_FILE: '' },
|
||||
stdout: 'pipe', stderr: 'pipe', timeout: 10_000,
|
||||
|
||||
@@ -149,9 +149,11 @@ describe('ship parent decision rules', () => {
|
||||
});
|
||||
|
||||
test('the rejection step removes new rejected files and reports tracked ones for a hunk revert', () => {
|
||||
const block = shipSection.slice(shipSection.indexOf(' while IFS= read -r f; do'), shipSection.indexOf(' REJECTED\n') + ' REJECTED\n'.length)
|
||||
.split('\n').map(line => line.replace(/^ {3}/, '')).join('\n');
|
||||
expect(block).toContain('<one rejected test path per line>');
|
||||
const start = shipSection.indexOf(' REJECTED_FILE="$(git rev-parse');
|
||||
const end = shipSection.indexOf(' rm -f "$REJECTED_FILE"\n', start) + ' rm -f "$REJECTED_FILE"\n'.length;
|
||||
const block = shipSection.slice(start, end).split('\n').map(line => line.replace(/^ {3}/, '')).join('\n');
|
||||
expect(block).toContain('<rejected-file-name>');
|
||||
expect(block).toContain('done < "$REJECTED_FILE"');
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'test-value-rejected-'));
|
||||
try {
|
||||
const git = (...args: string[]) => spawnSync('git', args, { cwd: dir, encoding: 'utf8', timeout: 10_000 });
|
||||
@@ -162,13 +164,18 @@ describe('ship parent decision rules', () => {
|
||||
git('-c', 'user.name=t', '-c', 'user.email=t@t', 'commit', '-q', '-m', 'seed');
|
||||
fs.appendFileSync(path.join(dir, 'test/kept.test.ts'), 'test("dup", () => {});\n');
|
||||
fs.writeFileSync(path.join(dir, 'test/new dup.test.ts'), 'test("dup", () => {});\n');
|
||||
const script = block.replace('<one rejected test path per line>', 'test/new dup.test.ts\ntest/kept.test.ts');
|
||||
fs.mkdirSync(path.join(dir, '.gstack/tmp'), { recursive: true });
|
||||
const list = path.join(dir, '.gstack/tmp/rejected-tests.abc123');
|
||||
fs.writeFileSync(list, 'test/new dup.test.ts\ntest/kept.test.ts\n../outside.test.ts\n');
|
||||
const script = block.replace('<rejected-file-name>', 'rejected-tests.abc123');
|
||||
const run = spawnSync('bash', ['-c', script], { cwd: dir, encoding: 'utf8', timeout: 10_000 });
|
||||
expect(run.status).toBe(0);
|
||||
expect(run.stdout).toContain('REMOVED: test/new dup.test.ts');
|
||||
expect(run.stdout).toContain('REVERT_HUNK: test/kept.test.ts');
|
||||
expect(run.stdout).toContain('SKIPPED (outside repo): ../outside.test.ts');
|
||||
expect(fs.existsSync(path.join(dir, 'test/new dup.test.ts'))).toBe(false);
|
||||
expect(fs.existsSync(path.join(dir, 'test/kept.test.ts'))).toBe(true);
|
||||
expect(fs.existsSync(list)).toBe(false);
|
||||
} finally {
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user