* fix(#4112): ban GNU-only grep -P in workflow markdown to prevent macOS regressions Add scripts/lint-portable-grep.cjs, wire it into lint:ci, and add tests/lint-portable-grep.test.cjs. The #4112 shell-syntax fix newly exposed a pre-existing grep -oP invocation that silently resolves to "" on stock macOS's BSD grep (no -P support). This ratchet catches the same class of GNU-coreutils assumption before it merges, mirroring lint-portable-timeout.cjs. * fix(#4112): drop unneeded ls -l long-format that broke basename/dirname extraction The previous commit on this branch replaced grep -oP 'phases/\K[^/]+' (GNU-only, silently fails on macOS's BSD grep) with basename "$(dirname "$phase")"), but left ls -lt (long format, -l) in place. -l output is a full detail line (permissions, owner, size, date, path), not a bare path, so dirname on that string throws "illegal option -- r" (BSD) / errors under GNU coreutils too -- the extraction never produced a usable value, on any platform. -l was never needed here; only mtime-sort plus the bare path mattered. Dropping it to ls -t restores one-bare-path-per-line output, which basename/dirname actually requires. Confirmed on gsd-test's Linux bench: the #4112 regression test (tests/pause-work-context-detection.test.cjs) now passes phase, spike, and sketch resolution. Emitted-Drift-Ack-Growth: pause-work.md — portability fix (#4112): dropping grep -oP for a portable basename/dirname extraction is a few characters longer per line; no functional growth beyond the fix. Emitted-Drift-Ack-Growth: sync-skills.md — portability fix (#4112): replacing grep -oP '(?<=--from )\S+' with a portable sed -E capture-group equivalent (no PCRE lookbehind available) is a longer expression; growth is the direct cost of the fix, not new functionality. * fix(#3859): pin git commit itself against diff.ignoreSubmodules=all, not just the probe git 2.39.x (the exact version on the CI Linux bench) resolves a pathspec-scoped `git commit -- <path>` through the same diff.ignoreSubmodules-gated machinery as `git diff`, so a genuinely bumped submodule gitlink was silently dropped by the real commit even though the #3859 empty-diff probe correctly saw the change. The commit was misclassified as generic commit_failed because git's refusal text never says "nothing to commit". Prefix the pathspec-scoped commit invocation with -c diff.ignoreSubmodules=dirty, the same override the probe already carries, so the two can no longer disagree. Confirmed on git 2.50.1 (no-op) and git 2.39.5 via the actual gsd-tester-linux v1.8.0-node24 bench image (turns the silent refusal into a commit). * fix(#3859): carry the diff.ignoreSubmodules override via env, not argv The prior commit prepended `-c diff.ignoreSubmodules=dirty` to the real commit's argv, which shifted `commitArgs[0]` off `'commit'` and broke several pre-existing #3859 regression tests (tests/commit-files-pathspec.test.cjs) that assert on the raw argv captured at the execGit seam, e.g. `gitCalls.some((a) => a[0] === 'commit')`. Carry the same override via `GIT_CONFIG_COUNT` / `GIT_CONFIG_KEY_0` / `GIT_CONFIG_VALUE_0` env vars instead, which git honours identically and leaves argv untouched. Re-confirmed on the gsd-tester-linux v1.8.0-node24 bench (git 2.39.5): the submodule commit still succeeds. * test(#3859): pin the commitEnv GIT_CONFIG_* override to canScope directly The commitEnv/GIT_CONFIG_* override added in e935694fc/b3d37b929 was only exercised indirectly through pre-existing submodule integration tests. Add two dedicated regression tests that pin the canScope=false side of the scoping decision: a whole-index commit (no --files) and an --amend commit must both keep recording a bumped submodule gitlink under diff.ignoreSubmodules=all with no override applied, since neither shape carries a pathspec for that git internal check to consult. * fix(#4112): match grep invocations after then/do/else/elif shell keywords lint-portable-grep.cjs's GREP_INVOCATION_RE only anchored to line-start or right after `| & ; ( \` {`, so a grep call positioned right after `then`, `do`, `else`, or `elif` (e.g. `if x; then grep -oP '...'; fi`) was never flagged, letting the GNU-only grep -P defect this lint exists to catch reappear undetected in that shape. * fix(#3859): apply the diff.ignoreSubmodules override to every commit, not just scoped ones The commitEnv GIT_CONFIG_* override landed in e935694fc/b3d37b929 only when canScope was true, on the assumption that git 2.39.5's silent refusal of a bumped submodule gitlink under diff.ignoreSubmodules=all only affects a pathspec-limited `git commit -- <paths>`. Reproduced directly against the pinned CI tester image (ghcr.io/open-gsd/gsd-tester-linux:v1.8.0-node24, git 2.39.5): a bare whole-index `git commit -m ...` with no pathspec at all is refused identically when the only staged change is a submodule gitlink, since git's "nothing to commit" check is a real diff against HEAD that honours diff.ignoreSubmodules regardless of whether a pathspec narrows it. Apply the override unconditionally instead of gating it on canScope; it remains a confirmed no-op for --amend, which never hits this refusal at all. Caught by the new regression test added in 579ac9f8f, which failed against the real bench git version before this change. * fix(#3859): apply diff.ignoreSubmodules override to commit-to-subrepo and pr-subrepo cmdCommit already carries the GIT_CONFIG_* override that forces diff.ignoreSubmodules=dirty on the actual git commit call so a bumped submodule gitlink is not spuriously refused under git 2.39.5. The two sibling multi-repo commit paths, cmdCommitToSubrepo and cmdPrSubrepo, build the identical canScope-branched commit invocation but never carried the override, so the same refusal there surfaces as a generic commit_failed/error instead of a recorded gitlink. Apply the override unconditionally to both, matching the corrected cmdCommit shape. * fix(#3859): pin pr-subrepo's change detection against diff.ignoreSubmodules cmdPrSubrepo discovers what to commit via `git status --porcelain`, which (like the empty-diff probe fixed for cmdCommit) honors a local diff.ignoreSubmodules=all config. Under that config, a genuinely bumped submodule gitlink is invisible to the status scan, so changedFiles comes back empty and the function reports nothing_to_commit before ever reaching the commit call this same issue already fixed. Pin the status probe with --ignore-submodules=dirty, mirroring the flag cmdCommit's diff probe already uses, so a real gitlink bump is detected regardless of local config. Found while adding a regression test for the previous #3859 follow-up fix: the test failed not on the commit step but on this earlier detection step. * docs(#3859): update changeset to cover all three fixed commit sites * docs(#4112): backfill changeset PR number (pr:0 -> pr:4149) --------- Co-authored-by: sim <sim@local>
177 lines
7.6 KiB
JavaScript
177 lines
7.6 KiB
JavaScript
#!/usr/bin/env node
|
|
'use strict';
|
|
|
|
/**
|
|
* lint-portable-grep.cjs — ban GNU-only `grep -P`/`--perl-regexp` in gsd
|
|
* workflow / agent / reference / command markdown (#4112 macOS regression).
|
|
*
|
|
* ## Why
|
|
*
|
|
* `-P`/`--perl-regexp` selects GNU grep's PCRE engine. Stock macOS ships BSD
|
|
* grep, which does not implement `-P` at all — the call errors out ("grep:
|
|
* invalid option -- P" or "unknown option") on that host, and a pipeline like
|
|
* `... | grep -oP '...' || true` swallows that error and silently resolves to
|
|
* an empty string instead of failing loudly.
|
|
*
|
|
* This is exactly what happened in `gsd-core/workflows/pause-work.md`'s
|
|
* Context Detection step (#4112, merged as #4140): the fix for the `$((`
|
|
* shell-syntax bug left `grep -oP 'phases/\K[^/]+'` in place. That construct
|
|
* had never been exercised on macOS before (the syntax error masked it), so
|
|
* fixing the syntax bug newly exposed a second, pre-existing macOS-only
|
|
* defect — phase/spike/sketch detection silently resolving to "" on macOS —
|
|
* which the PR's own new regression test then caught, but only in the
|
|
* post-merge `full test (macos-latest, ...)` matrix. That lane runs on push
|
|
* to the base branch, not on the pull_request event, so nothing pre-merge
|
|
* (gsd-test's Linux-only benches included) could have caught it before `next`
|
|
* went red.
|
|
*
|
|
* This is the same class of defect `lint-portable-timeout.cjs` (#2351) exists
|
|
* for: a GNU-coreutils assumption baked into workflow markdown that every
|
|
* pre-merge gate is blind to because none of them run on macOS. The fix here
|
|
* is the same shape — a static ratchet that catches the assumption in the
|
|
* markdown itself, before a shell ever executes it on the platform that lacks
|
|
* the flag.
|
|
*
|
|
* ## What PASSES
|
|
*
|
|
* - `grep -o '...'`, `grep -E '...'`, `egrep`/`fgrep` without `-P`.
|
|
* - `-P` appearing as part of an unrelated long option (`--path`, `--perl`) —
|
|
* only a `-P`/short-cluster-containing-`P` token or the literal
|
|
* `--perl-regexp` counts.
|
|
* - Prose that merely mentions "grep -P" outside of a fenced command context
|
|
* is still flagged (this lint does not parse fences) — the roots below are
|
|
* restricted to directive markdown where a `grep -P` mention is always
|
|
* either a real invocation or a specimen to fix, never neutral prose.
|
|
*
|
|
* ## What FAILS
|
|
*
|
|
* A `grep`/`egrep`/`fgrep` invocation whose flag cluster includes `P`
|
|
* (`-P`, `-oP`, `-Po`, `-iP`, ...) or the long form `--perl-regexp`.
|
|
*
|
|
* ## Portable replacements
|
|
*
|
|
* - Extracting a path component: `basename "$(dirname "$path")")` instead of
|
|
* `grep -oP 'seam/\K[^/]+'`.
|
|
* - Extracting a token after a flag: `sed -E 's#.*--flag[[:space:]]+([^[:space:]]+).*#\1#'`
|
|
* instead of `grep -oP '(?<=--flag )\S+'` — POSIX ERE (`-E`) has no
|
|
* lookbehind, so capture the group and let `sed` emit just it. `-E` (not
|
|
* GNU-only `-r`) is supported by both BSD and GNU `sed`. (A `/`-delimited
|
|
* sed command ending in a star-quantifier would place a star immediately
|
|
* before a slash, closing this very JSDoc block comment early — hence the
|
|
* `#`-delimited form used above.)
|
|
*/
|
|
|
|
const fs = require('fs');
|
|
const path = require('path');
|
|
const { ExitError, runMain } = require('./lib/cli-exit.cjs');
|
|
|
|
const ROOT = path.join(__dirname, '..');
|
|
|
|
// Surfaces whose markdown carries agent-executed bash. Mirrors
|
|
// lint-portable-timeout.cjs's roots — the same files that can ship shell
|
|
// snippets which a bash/zsh host on macOS actually executes.
|
|
const DEFAULT_ROOTS = ['gsd-core/workflows', 'gsd-core/references', 'agents', 'commands'];
|
|
|
|
// A `grep`/`egrep`/`fgrep` token, anchored to a command position (line start,
|
|
// right after `| & ; ( \` {`, or right after a shell keyword that opens a new
|
|
// command — `then`/`do`/`else`/`elif`) so prose mentioning "grep -P" in
|
|
// running text is still caught in these directive-markdown roots (see file
|
|
// header) without also matching an unrelated word ending in "grep". The
|
|
// keyword alternative is `\b`-bounded on both sides, so a bare mention of the
|
|
// word "then"/"do"/etc. with no following grep never matches on its own.
|
|
const GREP_INVOCATION_RE = /(?:^|[|&;(`{]|\b(?:then|do|else|elif)\b)[ \t]*(?:e|f)?grep\b/g;
|
|
|
|
// A `-P` short-flag cluster (e.g. `-P`, `-oP`, `-Po`, `-iP`) or the long form
|
|
// `--perl-regexp`, as a standalone token.
|
|
const PERL_FLAG_RE = /(?:^|\s)-[a-zA-Z]*P[a-zA-Z]*(?=\s|$)|--perl-regexp\b/;
|
|
|
|
/**
|
|
* Locate `grep -P`/`--perl-regexp` invocations in a block of text.
|
|
*
|
|
* Pure (no I/O): callers pass file contents; the caller reads files. For each
|
|
* `grep`-family invocation on a line, inspects only the segment from that
|
|
* invocation to the next `|`/`;`/end-of-line, so a `-P`-bearing token in a
|
|
* later, unrelated command on the same line is never mistaken for a grep flag.
|
|
*
|
|
* @param {string} text file contents
|
|
* @returns {{ line: number, snippet: string }[]} findings (empty array = clean)
|
|
*/
|
|
function findPerlGrepInvocations(text) {
|
|
const findings = [];
|
|
const lines = String(text).split(/\r?\n/);
|
|
for (let i = 0; i < lines.length; i += 1) {
|
|
const line = lines[i];
|
|
GREP_INVOCATION_RE.lastIndex = 0;
|
|
let match;
|
|
while ((match = GREP_INVOCATION_RE.exec(line)) !== null) {
|
|
const startAt = match.index + match[0].length;
|
|
const rest = line.slice(startAt);
|
|
const nextPipe = rest.search(/[|;`]/);
|
|
const segment = nextPipe === -1 ? rest : rest.slice(0, nextPipe);
|
|
if (PERL_FLAG_RE.test(segment)) {
|
|
findings.push({ line: i + 1, snippet: line.trim() });
|
|
break; // one finding per line is enough context to fix it
|
|
}
|
|
}
|
|
}
|
|
return findings;
|
|
}
|
|
|
|
function walkMarkdown(dir) {
|
|
const out = [];
|
|
let entries;
|
|
try {
|
|
entries = fs.readdirSync(dir, { withFileTypes: true });
|
|
} catch {
|
|
return out; // a missing root is not an error — some surfaces are optional
|
|
}
|
|
for (const entry of entries) {
|
|
const full = path.join(dir, entry.name);
|
|
if (entry.isDirectory()) out.push(...walkMarkdown(full));
|
|
else if (entry.isFile() && entry.name.endsWith('.md')) out.push(full);
|
|
}
|
|
return out;
|
|
}
|
|
|
|
/**
|
|
* Scan the given roots (repo-relative) for `grep -P`/`--perl-regexp` invocations.
|
|
* @param {string[]} roots
|
|
* @returns {{ file: string, line: number, snippet: string }[]}
|
|
*/
|
|
function scan(roots = DEFAULT_ROOTS) {
|
|
const offenders = [];
|
|
for (const rel of roots) {
|
|
const abs = path.isAbsolute(rel) ? rel : path.join(ROOT, rel);
|
|
for (const file of walkMarkdown(abs)) {
|
|
const findings = findPerlGrepInvocations(fs.readFileSync(file, 'utf8'));
|
|
for (const f of findings) {
|
|
offenders.push({ file: path.relative(ROOT, file), line: f.line, snippet: f.snippet });
|
|
}
|
|
}
|
|
}
|
|
return offenders;
|
|
}
|
|
|
|
function main() {
|
|
const rootsEnv = process.env.GSD_LINT_PORTABLE_GREP_ROOTS;
|
|
const roots = rootsEnv ? rootsEnv.split(path.delimiter).filter(Boolean) : DEFAULT_ROOTS;
|
|
const offenders = scan(roots);
|
|
if (offenders.length > 0) {
|
|
const detail = offenders.map((o) => ` ${o.file}:${o.line} ${o.snippet}`).join('\n');
|
|
throw new ExitError(
|
|
1,
|
|
'lint-portable-grep: `grep -P`/`--perl-regexp` is not portable — stock macOS ships\n' +
|
|
'BSD grep, which has no `-P`, so the call errors and a `|| true`/`$(...)` fallback\n' +
|
|
'silently resolves to empty output instead of failing loudly (#4112). Use\n' +
|
|
'`basename "$(dirname "$path")")` for path-component extraction, or\n' +
|
|
'`sed -E \'s/.../\\1/\'` (POSIX ERE, no lookbehind needed) for token extraction:\n' +
|
|
detail,
|
|
);
|
|
}
|
|
console.log(`ok lint-portable-grep: no GNU-only grep -P invocations in ${roots.length} root(s)`);
|
|
}
|
|
|
|
module.exports = { findPerlGrepInvocations, scan, DEFAULT_ROOTS };
|
|
|
|
if (require.main === module) runMain(main);
|