From b01089c05aa17de08d781d10e9f773d70d74a2d0 Mon Sep 17 00:00:00 2001 From: Fernando Castillo <8614435+fr1j0@users.noreply.github.com> Date: Mon, 18 May 2026 23:42:43 +0700 Subject: [PATCH] fix(#3689): use find instead of chained ls for continue-here scan (#3693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#3689): use find instead of chained ls for continue-here scan The check_incomplete_work step in resume-project.md chained six bare-glob ls arguments to discover .planning/.continue-here*.md handoff files. Under zsh's default NOMATCH option (macOS default shell), the first non-matching glob aborts the entire command during word-expansion, silently dropping every pattern after it — including .planning/.continue-here*.md, which holds the canonical pause checkpoint for default-context handoffs. The 2>/dev/null || true guard suppresses ls's own stderr / exit code but has no effect on the shell's pre-exec glob-abort. Replace the chain with two find invocations: find .planning -maxdepth 3 -name '.continue-here*.md' -print 2>/dev/null find . -maxdepth 1 -name '.continue-here*.md' -print 2>/dev/null find does not use shell glob expansion and tolerates absent directories on both bash and zsh. Adds tests/bug-3689-resume-glob-nomatch.test.cjs covering: - zsh -o nomatch with .planning/.continue-here-AT-1234.md present and no spike/sketch/deliberation subdirs (the regression scenario) - bash default, same layout - zsh -o nomatch with no checkpoints anywhere (clean exit, no output) - text invariant: resume-project.md no longer carries the chained-ls pattern and does carry the find-based scan Closes #3689 * fix(#3689): guard find commands with || true for Windows safety Restore the trailing '|| true' guard that the original chained ls had, per the windows-robustness.test.cjs invariant (informational bash commands in critical workflows must not let an exit-1 from find on exotic platforms tank the resume-project.md flow). * fix(#3689): address CodeRabbit review feedback - Set changeset frontmatter pr: 3693 (was 3690 placeholder); the release- note link now points at the right PR. - Use createTempDir / cleanup from tests/helpers.cjs instead of local tmpdir/cleanup wrappers, per repo test standards. * fix(#3689): update bug-3446 discovery contract to new find-based scan bug-3446 enforced three text invariants on the chained-ls implementation that this PR replaces with find. Update the assertions to verify the same three discovery paths are still covered: - .planning/.continue-here*.md at depth 1 -> find .planning -maxdepth 3 - .planning/sketches/SKETCH-NNN/.continue-here*.md at depth 3 -> same find with -maxdepth >= 3 (assertion now reads the actual depth and enforces a lower bound so future changes can deepen but not shallow it) - repo-root .continue-here*.md legacy fallback -> find . -maxdepth 1 The discovery contract is preserved; only the implementation under inspection changes. * test(#3689): convert bug-3446 from source-grep to behavioral assertions CodeRabbit flagged the text-regex assertions on the workflow source as a violation of the no-source-grep testing standard. Replace them with a behavioral integration test that: 1. Extracts the actual check_incomplete_work bash block from resume-project.md (so the test stays in sync with whatever the workflow does, no string match required). 2. Plants three handoff files in a temp dir covering the three discovery surfaces bug #3446 originally filed: - .planning/.continue-here.md (depth 1 under .planning) - .planning/sketches/SKETCH-001/.continue-here.md (depth 3) - ./.continue-here.md (legacy repo root) 3. Runs the snippet under bash and asserts each planted file appears in stdout. Same contract; now validated through runtime behavior instead of regex-on-file. * fix(#3689): use \\r?\\n in bash-fence regex for Windows CRLF parity The Windows test-parity guard at tests/windows-test-parity-guard.test.cjs:106 flags any new fence-extraction regex that uses a literal \\n after the language tag — on Windows CRLF the byte after `bash` is \\r, the regex silently fails to match, and the extracted snippet is empty. Switch to the canonical /```(?:bash|sh)\\r?\\n([\\s\\S]*?)```/ pattern. * fix(#3689): harden extractCheckBlock against missing and CRLF closing fence Per CodeRabbit review: assert that the closing tag is present before slicing (otherwise slice(stepStart, -1) silently grabs the wrong block and the test fails misleadingly), and add \r?\n to the closing fence as well so Windows CRLF doesn't sneak a stray carriage return into the captured snippet. --- .changeset/3689-resume-glob-nomatch-fix.md | 5 + get-shit-done/workflows/resume-project.md | 16 +- ...46-resume-continue-here-discovery.test.cjs | 115 +++++++++++--- tests/bug-3689-resume-glob-nomatch.test.cjs | 142 ++++++++++++++++++ 4 files changed, 247 insertions(+), 31 deletions(-) create mode 100644 .changeset/3689-resume-glob-nomatch-fix.md create mode 100644 tests/bug-3689-resume-glob-nomatch.test.cjs diff --git a/.changeset/3689-resume-glob-nomatch-fix.md b/.changeset/3689-resume-glob-nomatch-fix.md new file mode 100644 index 000000000..477a4a714 --- /dev/null +++ b/.changeset/3689-resume-glob-nomatch-fix.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3693 +--- +**`/gsd-resume-work` no longer drops `.planning/.continue-here*.md` checkpoints under zsh's default `NOMATCH`** — the chained `ls` of bare globs in `resume-project.md`'s `check_incomplete_work` step aborted on the first non-matching pattern, silently swallowing every later pattern (including the one that surfaces top-level handoff files). Replaced with `find .planning -maxdepth 3` + `find . -maxdepth 1`, which doesn't use shell glob expansion and tolerates absent directories on both bash and zsh. diff --git a/get-shit-done/workflows/resume-project.md b/get-shit-done/workflows/resume-project.md index ec333fe35..6d4bcc019 100644 --- a/get-shit-done/workflows/resume-project.md +++ b/get-shit-done/workflows/resume-project.md @@ -66,13 +66,15 @@ Look for incomplete work that needs attention: # Check for structured handoff (preferred — machine-readable) cat .planning/HANDOFF.json 2>/dev/null || true -# Check for continue-here files (phase + non-phase + legacy fallback) -ls .planning/phases/*/.continue-here*.md \ - .planning/spikes/*/.continue-here*.md \ - .planning/sketches/*/.continue-here*.md \ - .planning/deliberations/.continue-here*.md \ - .planning/.continue-here*.md \ - .continue-here*.md 2>/dev/null || true +# Check for continue-here files (phase + non-phase + legacy fallback). +# Use `find` rather than a chained `ls` of bare globs: under zsh's default +# NOMATCH option (macOS default shell), a single non-matching glob aborts +# the entire command during word-expansion — silently dropping every +# pattern after the first miss, including `.planning/.continue-here*.md`. +# `find` does not use shell glob expansion and tolerates absent +# directories on both bash and zsh. +find .planning -maxdepth 3 -name '.continue-here*.md' -print 2>/dev/null || true +find . -maxdepth 1 -name '.continue-here*.md' -print 2>/dev/null || true # Check for plans without summaries (incomplete execution) for plan in .planning/phases/*/*-PLAN.md; do diff --git a/tests/bug-3446-resume-continue-here-discovery.test.cjs b/tests/bug-3446-resume-continue-here-discovery.test.cjs index d1151896b..041bf8255 100644 --- a/tests/bug-3446-resume-continue-here-discovery.test.cjs +++ b/tests/bug-3446-resume-continue-here-discovery.test.cjs @@ -1,44 +1,111 @@ +// allow-test-rule: source-text-is-the-product +// Workflow `.md` files are the runtime contract executed by Claude Code as +// embedded bash. This test extracts the actual `check_incomplete_work` bash +// block from resume-project.md and exercises it against a planted directory +// layout — that's a behavioral integration test of the workflow contract, +// not regex-on-source. + 'use strict'; -const { test, describe } = require('node:test'); +const { test, describe, before, after } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const WORKFLOW_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'resume-project.md'); + +// Extract the first ```bash``` code block inside the +// `` element. That's the snippet the +// runtime actually executes; it's what we want to validate. +function extractCheckBlock() { + const md = fs.readFileSync(WORKFLOW_PATH, 'utf8'); + const stepStart = md.indexOf(''); + assert.ok(stepStart >= 0, 'resume-project.md must contain a check_incomplete_work step'); + const stepEnd = md.indexOf('', stepStart); + assert.ok( + stepEnd >= 0, + 'check_incomplete_work step must have a closing tag', + ); + const stepBody = md.slice(stepStart, stepEnd); + const fenceMatch = stepBody.match(/```(?:bash|sh)\r?\n([\s\S]*?)\r?\n```/); + assert.ok(fenceMatch, 'check_incomplete_work step must embed a ```bash code block'); + return fenceMatch[1]; +} + +function runSnippet(cwd, snippet) { + // has_interrupted_agent is a downstream-orchestrator variable; default it + // to "false" so the embedded `if` branch is a no-op during this test. + return spawnSync('bash', ['-c', snippet], { + cwd, + encoding: 'utf8', + env: { ...process.env, has_interrupted_agent: 'false', interrupted_agent_id: '' }, + }); +} describe('bug #3446: resume-project detects non-phase and legacy continue-here handoffs', () => { - const workflowPath = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'resume-project.md'); - const workflowContent = fs.readFileSync(workflowPath, 'utf8'); + let tmpDir; + let snippet; - function readCheckBlock() { - const stepStart = workflowContent.indexOf(''); - const stepEnd = workflowContent.indexOf('', stepStart); - return workflowContent.slice(stepStart, stepEnd); - } + before(() => { + snippet = extractCheckBlock(); + tmpDir = createTempDir('gsd-bug-3446-'); - test('check_incomplete_work scans .planning-root continue-here fallback', () => { - const block = readCheckBlock(); - assert.match( - block, - /\.planning\/\.continue-here\*\.md/, - 'resume workflow must scan .planning/.continue-here*.md fallback path written by pause-work' + // Plant the three discovery surfaces that bug #3446 was originally + // filed to cover. + fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', '.continue-here.md'), + '---\ncontext: default\n---\nroot-of-.planning handoff\n', + 'utf8', + ); + + fs.mkdirSync(path.join(tmpDir, '.planning', 'sketches', 'SKETCH-001'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', 'sketches', 'SKETCH-001', '.continue-here.md'), + '---\ncontext: sketch\n---\nsketch handoff\n', + 'utf8', + ); + + fs.writeFileSync( + path.join(tmpDir, '.continue-here.md'), + '---\ncontext: legacy\n---\nlegacy repo-root handoff\n', + 'utf8', ); }); - test('check_incomplete_work scans sketch subdirectory continue-here checkpoints', () => { - const block = readCheckBlock(); + after(() => { + cleanup(tmpDir); + }); + + test('check_incomplete_work surfaces .planning/.continue-here.md (depth 1 under .planning)', () => { + const result = runSnippet(tmpDir, snippet); + assert.equal(result.status, 0, `snippet exited ${result.status}; stderr=${result.stderr}`); assert.match( - block, - /\.planning\/sketches\/\*\/\.continue-here\*\.md/, - 'resume workflow must scan .planning/sketches/*/.continue-here*.md for sketch checkpoint handoffs' + result.stdout, + /\.planning\/\.continue-here\.md/, + `expected .planning/.continue-here.md in stdout; got: ${JSON.stringify(result.stdout)}`, ); }); - test('check_incomplete_work scans legacy repo-root continue-here fallback', () => { - const block = readCheckBlock(); + test('check_incomplete_work surfaces .planning/sketches/SKETCH-001/.continue-here.md (depth 3 under .planning)', () => { + const result = runSnippet(tmpDir, snippet); + assert.equal(result.status, 0, `snippet exited ${result.status}; stderr=${result.stderr}`); assert.match( - block, - /\s\.continue-here\*\.md/, - 'resume workflow must scan legacy repo-root .continue-here*.md handoff path' + result.stdout, + /\.planning\/sketches\/SKETCH-001\/\.continue-here\.md/, + `expected sketch handoff in stdout; got: ${JSON.stringify(result.stdout)}`, + ); + }); + + test('check_incomplete_work surfaces legacy repo-root .continue-here.md', () => { + const result = runSnippet(tmpDir, snippet); + assert.equal(result.status, 0, `snippet exited ${result.status}; stderr=${result.stderr}`); + assert.match( + result.stdout, + /(^|\n)\.\/\.continue-here\.md(\n|$)/, + `expected legacy ./.continue-here.md in stdout; got: ${JSON.stringify(result.stdout)}`, ); }); }); diff --git a/tests/bug-3689-resume-glob-nomatch.test.cjs b/tests/bug-3689-resume-glob-nomatch.test.cjs new file mode 100644 index 000000000..752729436 --- /dev/null +++ b/tests/bug-3689-resume-glob-nomatch.test.cjs @@ -0,0 +1,142 @@ +// allow-test-rule: source-text-is-the-product +// Workflow `.md` files are the runtime contract executed by Claude Code as +// embedded bash. Asserting on the staged text of resume-project.md and on the +// behavior of the embedded snippet under real shells is a behavioral test of +// the workflow itself, not source-grep theater. + +/** + * Regression for #3689 — /gsd-resume-work silently drops + * `.planning/.continue-here*.md` checkpoints under zsh's default NOMATCH. + * + * Root cause: the `check_incomplete_work` step in + * `get-shit-done/workflows/resume-project.md` used a chained `ls` with six + * bare-glob arguments. Under zsh's default `NOMATCH` setopt the first + * non-matching glob aborts the entire command during word-expansion — every + * pattern after that point is never evaluated, including the one that holds + * valid pause checkpoints (`.planning/.continue-here*.md`). `2>/dev/null || + * true` only suppresses ls's own stderr / exit code; it has no effect on the + * shell's pre-exec abort. + * + * Fix: replace the chained `ls` with two `find` calls. `find` does not use + * shell glob expansion, and `find -maxdepth N -name PATTERN + * -print 2>/dev/null` tolerates absent directories on both bash and zsh. + * + * This test covers: + * 1. zsh under `-o nomatch`: checkpoint at `.planning/.continue-here-*.md` + * is listed even when `.planning/spikes`, `.planning/sketches`, + * `.planning/deliberations` are absent (the common new-project layout). + * 2. bash default: same behavior. + * 3. zsh `-o nomatch` with no `.continue-here` files anywhere: exits 0, + * no output, no error. + * 4. Text invariant: resume-project.md no longer carries the brittle + * chained-ls pattern. + */ + +'use strict'; + +const { describe, test, before, after } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { spawnSync } = require('node:child_process'); +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const REPO_ROOT = path.resolve(__dirname, '..'); +const WORKFLOW_PATH = path.join(REPO_ROOT, 'get-shit-done', 'workflows', 'resume-project.md'); + +// The exact snippet the workflow now embeds. Keep in sync with +// resume-project.md `check_incomplete_work` step. +const FIND_SNIPPET = [ + "find .planning -maxdepth 3 -name '.continue-here*.md' -print 2>/dev/null || true", + "find . -maxdepth 1 -name '.continue-here*.md' -print 2>/dev/null || true", +].join('\n'); + +function hasShell(name) { + const result = spawnSync('which', [name], { encoding: 'utf8' }); + return result.status === 0 && result.stdout.trim().length > 0; +} + +describe('bug #3689 — resume-project.md continue-here scan under zsh NOMATCH', () => { + let tmpDir; + + before(() => { + tmpDir = createTempDir('gsd-bug-3689-'); + // Reproduce the common new-project layout: a `.planning/` with a + // suffixed continue-here file and *no* spike / sketch / deliberation + // subdirectories. + fs.mkdirSync(path.join(tmpDir, '.planning'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, '.planning', '.continue-here-AT-1234.md'), + '---\ncontext: default\n---\nhandoff body\n', + 'utf8', + ); + }); + + after(() => { + cleanup(tmpDir); + }); + + test('zsh -o nomatch lists the .planning/.continue-here-* checkpoint', { skip: !hasShell('zsh') }, () => { + const result = spawnSync('zsh', ['-o', 'nomatch', '-c', FIND_SNIPPET], { + cwd: tmpDir, + encoding: 'utf8', + }); + assert.equal(result.status, 0, `zsh exited ${result.status}; stderr=${result.stderr}`); + assert.match( + result.stdout, + /\.planning\/\.continue-here-AT-1234\.md/, + `expected checkpoint in stdout, got: ${JSON.stringify(result.stdout)}`, + ); + }); + + test('bash default lists the .planning/.continue-here-* checkpoint', { skip: !hasShell('bash') }, () => { + const result = spawnSync('bash', ['-c', FIND_SNIPPET], { + cwd: tmpDir, + encoding: 'utf8', + }); + assert.equal(result.status, 0, `bash exited ${result.status}; stderr=${result.stderr}`); + assert.match( + result.stdout, + /\.planning\/\.continue-here-AT-1234\.md/, + `expected checkpoint in stdout, got: ${JSON.stringify(result.stdout)}`, + ); + }); +}); + +describe('bug #3689 — empty workspace exits cleanly', () => { + let tmpDir; + + before(() => { + tmpDir = createTempDir('gsd-bug-3689-'); + // No .planning/ at all, no .continue-here files. Pure greenfield. + }); + + after(() => { + cleanup(tmpDir); + }); + + test('zsh -o nomatch with no checkpoints exits 0, empty output', { skip: !hasShell('zsh') }, () => { + const result = spawnSync('zsh', ['-o', 'nomatch', '-c', FIND_SNIPPET], { + cwd: tmpDir, + encoding: 'utf8', + }); + assert.equal(result.status, 0, `zsh exited ${result.status}; stderr=${result.stderr}`); + assert.equal(result.stdout.trim(), '', `expected no stdout, got: ${JSON.stringify(result.stdout)}`); + }); +}); + +describe('bug #3689 — workflow text invariant', () => { + test('resume-project.md no longer chains bare globs through ls', () => { + const body = fs.readFileSync(WORKFLOW_PATH, 'utf8'); + assert.doesNotMatch( + body, + /ls\s+\.planning\/spikes\/\*\/\.continue-here/, + 'resume-project.md still contains the chained `ls .planning/spikes/*/.continue-here*.md` pattern that aborts under zsh NOMATCH; the find-based scan should replace it.', + ); + assert.match( + body, + /find \.planning -maxdepth 3 -name '\.continue-here\*\.md'/, + 'resume-project.md must use the find-based scan introduced by the #3689 fix.', + ); + }); +});