Skip to content

Commit db3c226

Browse files
dmealingclaude
andcommitted
fix(cli): the hash-manifest advisory named a file that may not hold the rule
It hardcoded `.metaobjects/.gitignore` and the literal pattern `.gen-state/`. That is one of several places the exclusion can sit — any `.gitignore` up the tree, `.git/info/exclude`, a per-user excludes file. Found by running an adopter estate against the RC: its rule was in the ROOT `.gitignore` as `.metaobjects/.gen-state/`, so following the advisory to the letter edited a file that did not hold the rule, changed nothing, and printed the same line again. The loop shape, in a second command — and I hit it myself before reading the code. `git check-ignore -v` returns the exact `<file>:<line>:<pattern>` that matched, and this check already shells out to `check-ignore` for the verdict, so nothing new has to be assumed. New `gitIgnoreSource()` asks it; the advisory names the real rule and builds the `/*` + `!` negation from the pattern it found. When git cannot say, the wording degrades to "find it with `git check-ignore -v …`" instead of asserting a location — the same discipline the rest of the check follows, where "cannot say" is never reported as a fact. The VERDICT still comes from `isGitIgnored`, so a failure in the new lookup weakens the wording and never the check. Gated by the estate's own shape (the rule in a parent .gitignore), and proved by BREAKING it: reverting the source alone reds both new cases. cli: 920 pass, 0 fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTcEKXTQMYt84fAjuw5A2M
1 parent 8b8ec1f commit db3c226

4 files changed

Lines changed: 116 additions & 6 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,21 @@ integration cases generated with an explicit `--out <dir>/docs` while `verify --
7272
the config default, and passed only because the two coincided. Sixteen cases failed the moment
7373
they stopped. They now take the config path, which is what the gate reads.
7474

75+
### Fixed — the hash-manifest advisory prescribed editing a file that may not hold the rule
76+
77+
It hardcoded `.metaobjects/.gitignore` and the literal pattern `.gen-state/`. That is only one
78+
of the places the exclusion can sit: it can be in ANY `.gitignore` up the tree, in
79+
`.git/info/exclude`, or in a per-user excludes file. Found by running an adopter estate against
80+
the RC — its rule was in the **ROOT** `.gitignore` as `.metaobjects/.gen-state/`, so following
81+
the advisory to the letter edited a file that did not hold the rule, changed nothing, and
82+
printed the same line again. The loop shape, in a second command.
83+
84+
`git check-ignore -v` returns the exact `<file>:<line>:<pattern>` that matched, and the check
85+
already shells out to `check-ignore` for the verdict. The advisory now names the real rule and
86+
builds the `/*` + `!` negation from the pattern it found. When git cannot say, the wording
87+
degrades to "find it with `git check-ignore -v …`" rather than asserting a location — the same
88+
discipline the rest of this check follows, where "cannot say" is never reported as a fact.
89+
7590
### Changed — `--limit` is documented as per printed LIST, which is what it always was
7691

7792
Its doc said "per SECTION, never shared across them". The anti-pattern tier prints TWO

‎server/typescript/packages/cli/src/lib/git-ignore.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,47 @@ export function isGitIgnored(
113113
if ("unavailable" in res) return undefined;
114114
return res.ignored.has(toPosix(rel));
115115
}
116+
117+
/**
118+
* WHICH rule ignores `rel` — the `.gitignore` file, its line, and the pattern.
119+
*
120+
* Advisories that tell an adopter to fix an ignore rule have to name the rule that is
121+
* actually in force, not a conventional location. The exclusion can live in ANY
122+
* `.gitignore` up the tree, in `.git/info/exclude`, or in a per-user excludes file, and an
123+
* advisory that assumes one of them sends the reader to edit a file that does not hold the
124+
* rule — after which the same advisory prints again, unchanged. `git check-ignore -v`
125+
* answers it exactly, so nothing has to be assumed.
126+
*
127+
* `undefined` when git cannot say or nothing matches. The verdict itself still comes from
128+
* `isGitIgnored`; this is only for the message, so a failure here degrades the wording
129+
* rather than the check.
130+
*/
131+
export function gitIgnoreSource(
132+
dir: string,
133+
rel: string,
134+
opts: GitIgnoreOptions,
135+
): { file: string; line: string; pattern: string } | undefined {
136+
const gitBin = process.env.META_GEN_GIT ?? "git";
137+
const args: string[] = [];
138+
if (!opts.honourGlobalExcludes) {
139+
args.push("-c", `core.excludesFile=${NO_SUCH_EXCLUDES_FILE}`);
140+
}
141+
args.push("-C", dir, "check-ignore", "-v", "--", toPosix(rel));
142+
143+
let res;
144+
try {
145+
res = spawnSync(gitBin, args, { encoding: "utf-8" });
146+
} catch {
147+
return undefined;
148+
}
149+
if (res.error !== undefined || res.status !== 0) return undefined;
150+
// `<source>:<line>:<pattern>\t<path>` — the source may itself contain a colon on
151+
// Windows, so split from the RIGHT of the first tab-delimited field.
152+
const first = (res.stdout ?? "").split("\n")[0];
153+
if (first === undefined || first === "") return undefined;
154+
const lhs = first.split("\t")[0];
155+
if (lhs === undefined) return undefined;
156+
const m = /^(.*):(\d+):(.*)$/.exec(lhs);
157+
if (m === null) return undefined;
158+
return { file: m[1]!, line: m[2]!, pattern: m[3]! };
159+
}

‎server/typescript/packages/cli/src/lib/manifest-ignored-check.ts‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
import { existsSync } from "node:fs";
1919
import { join } from "node:path";
2020
import { log } from "./log.js";
21-
import { isGitIgnored } from "./git-ignore.js";
21+
import { gitIgnoreSource, isGitIgnored } from "./git-ignore.js";
2222

2323
/** Project-relative path of the manifest — the one artifact of `.gen-state/` that is
2424
* meant to be committed. */
@@ -40,13 +40,28 @@ export function warnIfManifestIgnored(cwd: string): void {
4040
// per-user ignore file answers it just as truly as a committed .gitignore does.
4141
if (isGitIgnored(cwd, HASH_MANIFEST_REL, { honourGlobalExcludes: true }) !== true) return;
4242

43+
// WHERE the rule actually lives, asked of git rather than assumed. This used to name
44+
// `.metaobjects/.gitignore` and the literal pattern `.gen-state/`, which is only one of
45+
// the places the exclusion can sit: an adopter estate had it in the ROOT `.gitignore` as
46+
// `.metaobjects/.gen-state/`, so following this advisory to the letter edited a file that
47+
// did not hold the rule, changed nothing, and printed this same line again — the loop
48+
// shape. `git check-ignore -v` knows the answer, so nothing is guessed. When git cannot
49+
// say, the wording degrades to the generic form rather than asserting a location.
50+
const src = gitIgnoreSource(cwd, HASH_MANIFEST_REL, { honourGlobalExcludes: true });
51+
const where =
52+
src !== undefined
53+
? `The rule is '${src.pattern}' at ${src.file}:${src.line} — replace it there with ` +
54+
`a '/*' form plus a '!' negation for the manifest ` +
55+
`(e.g. '${src.pattern.replace(/\/$/, "")}/*' and '!${src.pattern.replace(/\/$/, "")}/.hashes.json')`
56+
: `Find the rule with 'git check-ignore -v ${HASH_MANIFEST_REL}' and replace it with ` +
57+
`a '/*' form plus a '!' negation for the manifest`;
58+
4359
log.warn(
4460
`${HASH_MANIFEST_REL} is git-ignored, so it never reaches another machine — ` +
4561
`on a fresh clone or CI runner 'meta gen' cannot tell your hand edits from its ` +
46-
`own stale output, and will refuse to overwrite rather than guess. Fix it in ` +
47-
`.metaobjects/.gitignore by replacing '.gen-state/' with '.gen-state/*' plus ` +
48-
`'!.gen-state/.hashes.json' (the glob matters — git will not descend into an ` +
49-
`excluded directory, so a negation under '.gen-state/' can never apply), then ` +
50-
`commit the manifest. If you deliberately do not commit generated output, ignore this.`,
62+
`own stale output, and will refuse to overwrite rather than guess. ${where} ` +
63+
`(the glob matters — git will not descend into an excluded directory, so a negation ` +
64+
`INSIDE an excluded directory can never apply), then commit the manifest. ` +
65+
`If you deliberately do not commit generated output, ignore this.`,
5166
);
5267
}

‎server/typescript/packages/cli/test/manifest-ignored-check.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,39 @@ describe("warnIfManifestIgnored", () => {
9898
expect(captureWarnings(() => warnIfManifestIgnored(root))).toEqual([]);
9999
});
100100
});
101+
102+
describe("the advisory names the rule that is actually in force", () => {
103+
// Found on an adopter estate: the exclusion was in the ROOT `.gitignore` as
104+
// `.metaobjects/.gen-state/`, not in `.metaobjects/.gitignore` as the message assumed.
105+
// Editing the named file changed nothing and the same advisory printed again — the loop
106+
// shape. The remedy has to be derived from git, not from a convention.
107+
test("a rule in the ROOT .gitignore is named, not the .metaobjects one", () => {
108+
gitInit();
109+
writeManifest();
110+
writeFileSync(join(root, ".gitignore"), ".metaobjects/.gen-state/\n");
111+
const warns = captureWarnings(() => warnIfManifestIgnored(root));
112+
expect(warns).toHaveLength(1);
113+
// It must point at the file that HOLDS the rule...
114+
expect(warns[0]).toContain(".gitignore:1");
115+
expect(warns[0]).toContain(".metaobjects/.gen-state/");
116+
// ...and must NOT prescribe editing a file that does not hold it.
117+
expect(warns[0]).not.toContain("Fix it in .metaobjects/.gitignore");
118+
});
119+
120+
test("a rule in .metaobjects/.gitignore is named too, with its own pattern", () => {
121+
gitInit();
122+
writeManifest();
123+
writeIgnore(".gen-state/\n");
124+
const warns = captureWarnings(() => warnIfManifestIgnored(root));
125+
expect(warns).toHaveLength(1);
126+
expect(warns[0]).toContain(".metaobjects/.gitignore:1");
127+
expect(warns[0]).toContain("!.gen-state/.hashes.json");
128+
});
129+
130+
test("when git cannot say, it degrades to the generic form and asserts no location", () => {
131+
// No repository at all: the verdict path returns undefined, so this warning never
132+
// fires — proving the advisory cannot invent a location it does not have.
133+
writeManifest();
134+
expect(captureWarnings(() => warnIfManifestIgnored(root))).toEqual([]);
135+
});
136+
});

0 commit comments

Comments
 (0)