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
47 changes: 46 additions & 1 deletion .github/scripts/openspec-tracking.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,17 @@
*
* Usage:
* node .github/scripts/openspec-tracking.cjs < input.json
* node .github/scripts/openspec-tracking.cjs --list [changesDirectory]
*
* The second form is the one three workflows call to list unarchived change
* directories: it prints `listUnarchivedChanges()` as a sorted JSON array of
* names on stdout, and nothing else. That routes the listing through
* `readdirSync`, which behaves the same on every platform `node` runs on,
* rather than through a shell `find`, where `-printf` is a GNU extension that
* a checkout under BSD find (macOS) does not recognise. A missing changes
* directory prints `[]` and exits zero; an unreadable one throws and exits
* non-zero, and the calling workflow turns that into a `::warning::`
* annotation rather than either an empty list or a failed run.
*
* Reads one JSON object on stdin and prints the plan as JSON on stdout:
*
Expand Down Expand Up @@ -308,6 +319,15 @@ function planActions(input) {
// An open issue for a change that is no longer under `openspec/changes/` has
// been archived. This is the success path and the only way an issue closes
// without a human.
//
// PRECONDITION: `unarchived` is a listing that actually succeeded. Absence
// from it is read as "archived", so an EMPTY array means "every change is
// archived", never "the listing could not be read". A caller that cannot
// enumerate the directory must skip the run rather than pass `[]`: doing the
// latter closes every open tracking issue with "Archived. <change> reached
// openspec/changes/archive/", which is false. Each workflow gates its apply
// step on the listing having succeeded, and openspec-sweep.yml did not, which
// is what this note exists to stop happening again.
for (const issue of issues) {
if (issue.state === "open" && !seen.has(issue.change)) {
const action = {
Expand All @@ -328,6 +348,25 @@ function main(raw) {
return planActions(input);
}

const DEFAULT_CHANGES_DIRECTORY = "openspec/changes";

/**
* `--list` mode. Prints `listUnarchivedChanges(changesDirectory)` as a JSON
* array on stdout, and nothing else, so a caller can pipe stdout straight
* into `jq` without stripping any other output.
*
* A missing directory is not a fault (see `listUnarchivedChanges`) and prints
* `[]`. An unreadable one throws out of `readdirSync`, which this
* deliberately does not catch: letting it propagate is what turns it into a
* non-zero exit for `require.main` to report, which is the signal the calling
* workflow needs to tell "nothing to report" apart from "could not tell".
*/
function runList(changesDirectory) {
process.stdout.write(
`${JSON.stringify(listUnarchivedChanges(changesDirectory))}\n`
);
}

module.exports = {
ARCHIVE_DIRECTORY,
DEFAULT_STALE_DAYS,
Expand All @@ -340,11 +379,17 @@ module.exports = {
issueBody,
planActions,
main,
runList,
};

if (require.main === module) {
try {
process.stdout.write(`${JSON.stringify(main(), null, 2)}\n`);
const args = process.argv.slice(2);
if (args[0] === "--list") {
runList(args[1] ?? DEFAULT_CHANGES_DIRECTORY);
} else {
process.stdout.write(`${JSON.stringify(main(), null, 2)}\n`);
}
} catch (error) {
console.error(`openspec-tracking failed: ${error.message}`);
process.exitCode = 1;
Expand Down
72 changes: 72 additions & 0 deletions .github/scripts/openspec-tracking.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,25 @@ const {
issueBody,
planActions,
main,
runList,
} = require("./openspec-tracking.cjs");

/** Captures what `runList` writes to stdout, without a real process spawn. */
function captureList(changesDirectory) {
const original = process.stdout.write.bind(process.stdout);
let written = "";
process.stdout.write = (chunk) => {
written += chunk;
return true;
};
try {
runList(changesDirectory);
} finally {
process.stdout.write = original;
}
return JSON.parse(written);
}

/** The decision-bearing fields of an action, without the rendered prose. */
function shape(action) {
const { title, body, comment, ...rest } = action;
Expand Down Expand Up @@ -76,6 +93,38 @@ test("a missing changes directory is not a fault", () => {
assert.deepEqual(listUnarchivedChanges("/nonexistent/openspec/changes"), []);
});

test("--list prints the scan as a sorted JSON array, archive excluded", () => {
const root = mkdtempSync(join(tmpdir(), "openspec-tracking-list-"));
try {
const changes = join(root, "changes");
mkdirSync(join(changes, "archive", "2026-01-01-old"), { recursive: true });
mkdirSync(join(changes, "beta"), { recursive: true });
mkdirSync(join(changes, "alpha"), { recursive: true });
assert.deepEqual(captureList(changes), ["alpha", "beta"]);
} finally {
rmSync(root, { recursive: true, force: true });
}
});

test("--list against a missing directory prints [], not a fault", () => {
assert.deepEqual(captureList("/nonexistent/openspec/changes"), []);
});

test("--list round-trips a change name containing a space", () => {
const root = mkdtempSync(join(tmpdir(), "openspec-tracking-list-space-"));
try {
const changes = join(root, "changes");
mkdirSync(join(changes, "probe with spaces"), { recursive: true });
mkdirSync(join(changes, "probe-normal"), { recursive: true });
assert.deepEqual(captureList(changes), [
"probe with spaces",
"probe-normal",
]);
} finally {
rmSync(root, { recursive: true, force: true });
}
});

test("a pull request touching a change directory claims it", () => {
const claims = claimsFromPullRequests([
{ number: 265, files: ["openspec/changes/thing/tasks.md", "src/a.ts"] },
Expand Down Expand Up @@ -337,6 +386,29 @@ test("the sweep escalates at most once per window", () => {
);
});

test("an empty listing closes every tracked issue, so a caller must never guess it", () => {
// Pinning the hazard, not the feature. Absence from `unarchived` is read as
// "archived", so `[]` claims every change is archived. A workflow that could
// not read the directory and passed `[]` anyway would close every open
// tracking issue with a false comment. openspec-sweep.yml did exactly that
// until its apply step was gated on the listing having succeeded.
const plan = planActions({
mode: "sweep",
sha: "abc123",
staleDays: 7,
unarchived: [],
issues: [
{ number: 11, state: "open", body: marker("real-change") },
{ number: 12, state: "open", body: marker("other-change") },
],
});
assert.deepEqual(shapes(plan), [
{ type: "close", reason: "archived", change: "real-change", issue: 11 },
{ type: "close", reason: "archived", change: "other-change", issue: 12 },
]);
assert.match(plan.actions[0].comment, /reached `openspec\/changes\/archive\/`/);
});

test("an unknown mode is a caller defect, not a silent pass", () => {
assert.throws(() => planActions({ mode: "daily" }), /unknown mode/);
});
Expand Down
37 changes: 34 additions & 3 deletions .github/workflows/openspec-label.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,41 @@ jobs:
run: |
set -euo pipefail

# The listing is delegated to openspec-tracking.cjs --list rather
# than to a shell `find`. A `find` form can always be made portable
# for today's shapes (this file once carried `-printf '%f\n'`, a GNU
# extension BSD find rejects, fixed to `-exec basename {} \;`
# instead), but that is a property someone has to get right again
# every time the listing changes. Routing it through `readdirSync`,
# already covered by openspec-tracking.test.cjs, removes the
# question of which `find` flags are portable rather than answering
# it correctly this once.
#
# The script can still fail (an unreadable `openspec/changes`), and
# that must read as "could not tell" rather than as "resolved":
# reporting an unreadable directory as zero unarchived changes would
# tell a stack its OpenSpec change is done when nobody checked. So
# the exit status is captured outside the assignment position `set
# -e` watches, and an unreadable directory gets its own summary and
# an explicit annotation instead of either silence or an abort. This
# workflow's whole point is to never fail a check for a reason
# unrelated to the pull request.
changes=""
if [ -d openspec/changes ]; then
changes=$(find openspec/changes -mindepth 1 -maxdepth 1 -type d \
-not -name archive -printf '%f\n' | sort)
listing_failed=0
if ! raw=$(node .github/scripts/openspec-tracking.cjs --list 2>&1); then
echo "::warning::Could not list openspec/changes/ ($raw). Leaving the '$LABEL' label untouched rather than guessing."
listing_failed=1
else
changes=$(jq -r '.[]' <<<"$raw")
fi

if [ "$listing_failed" -eq 1 ]; then
{
echo "### OpenSpec: unknown"
echo ""
echo "Could not list \`openspec/changes/\` on this branch, so whether an unarchived change remains is unknown. The \`$LABEL\` label was left as-is."
} >> "$GITHUB_STEP_SUMMARY"
exit 0
fi

if [ -z "$changes" ]; then
Expand Down
35 changes: 31 additions & 4 deletions .github/workflows/openspec-sweep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ jobs:
fetch-depth: 0 # full history, so `git log` can date each change directory

- name: Plan escalations
id: plan
env:
GH_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
Expand All @@ -59,8 +60,33 @@ jobs:
set -euo pipefail

unarchived='[]'
if [ -d openspec/changes ]; then
now=$(date +%s)
now=$(date +%s)
# The listing is delegated to openspec-tracking.cjs --list rather
# than to a shell `find`. This file once carried `find ... -printf
# '%f\n'` inside a process substitution: `done < <(...)` hands the
# loop the substitution's own exit status, not find's, so a failed
# `-printf` on BSD find (macOS) ran the loop zero times and this
# step silently reported "0 unarchived changes" and exited 0. That
# is the exact silent case this workflow exists to avoid, and a
# different `find` form only fixes this one occurrence of it.
# Routing the listing through `readdirSync`, already covered by
# openspec-tracking.test.cjs, removes the question of which `find`
# flags are portable rather than answering it correctly once. The
# script's own exit status is captured by a plain assignment, not a
# process substitution, so its failure cannot go unseen the same way.
if ! changes_json=$(node .github/scripts/openspec-tracking.cjs --list 2>&1); then
# SKIP THE RUN, do not fall through. An empty `unarchived` is not
# "we could not tell", it is "every change is archived": the
# planner's close-as-archived pass closes every open tracking issue
# whose change is absent from the listing. Falling through here
# closed all of them with "Archived. <change> reached
# openspec/changes/archive/", which is false, on nothing worse than
# a transient read error. This mirrors openspec-tracking.yml, which
# gates its apply step the same way.
echo "::warning::Could not list openspec/changes/ ($changes_json). Reporting zero unarchived changes would close every open tracking issue as archived, so this run is skipped instead."
echo "skipped=true" >> "$GITHUB_OUTPUT"
exit 0
else
# Names and ages go into JSON as jq ARGUMENTS, never joined into a
# line and split back apart. A directory name containing a space
# desynced the split, `tonumber` threw on the wrong field, and under
Expand All @@ -80,8 +106,7 @@ jobs:
fi
unarchived=$(jq -c --arg name "$name" --argjson age "$age" \
'. + [{name: $name, ageDays: $age}]' <<<"$unarchived")
done < <(find openspec/changes -mindepth 1 -maxdepth 1 -type d \
-not -name archive -printf '%f\n' | sort)
done < <(jq -r '.[]' <<<"$changes_json")
fi

# Bodies are handed over raw. The marker is parsed in one place, in
Expand Down Expand Up @@ -114,8 +139,10 @@ jobs:
> /tmp/openspec-sweep-plan.json

cat /tmp/openspec-sweep-plan.json
echo "skipped=false" >> "$GITHUB_OUTPUT"

- name: Apply the plan
if: steps.plan.outputs.skipped == 'false'
env:
GH_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
Expand Down
24 changes: 20 additions & 4 deletions .github/workflows/openspec-tracking.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,28 @@ jobs:
--json number,state,body \
| jq -c '[.[] | {number, state: (.state | ascii_downcase), body: (.body // "")}]')

# The listing is delegated to openspec-tracking.cjs --list rather
# than to a shell `find`. This file once carried `-printf '%f\n'`, a
# GNU extension a checkout under BSD find (macOS) does not
# recognise, and its failure reached this pipe under `pipefail`,
# aborting the whole run. A different `find` form fixes that
# occurrence; routing the listing through `readdirSync`, already
# covered by openspec-tracking.test.cjs, removes the question of
# which `find` flags are portable rather than answering it once.
#
# A listing failure here (an unreadable `openspec/changes`) must not
# read as "nothing unarchived": that would close or skip a tracking
# issue for a change that is still there, nobody just could not see
# it. So this is the same skip this step already takes when
# `gh pr list` fails above: warn, skip this run, let the next push
# re-evaluate, and never fail the check.
unarchived='[]'
if [ -d openspec/changes ]; then
unarchived=$(find openspec/changes -mindepth 1 -maxdepth 1 -type d \
-not -name archive -printf '%f\n' | sort \
| jq -R -s -c 'split("\n") | map(select(length > 0) | {name: .})')
if ! names_json=$(node .github/scripts/openspec-tracking.cjs --list 2>&1); then
echo "::warning::Could not list openspec/changes/ ($names_json). Skipping this run rather than reporting a change as archived when nobody could tell."
echo "skipped=true" >> "$GITHUB_OUTPUT"
exit 0
fi
unarchived=$(jq -c '[.[] | {name: .}]' <<<"$names_json")

jq -n -c \
--arg sha "$SHA" \
Expand Down
Loading