mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-06 00:44:07 +01:00
fix: pre-landing review fixes (GSTA-92)
A malformed or missing --accepted exceptions file now gets the full craft check with nothing accepted, after a warning, still exiting 2. Before, it printed nothing, and the craft section treats a missing CRAFT_RESULT as 'check unavailable', so one bad finalized.json switched the gate off (found by both adversarial passes). The header comment and the craft section now document the accepted-count suffix on CRAFT_RESULT. Tests cover duplicate carried-forward exception ids and the slop-gate touchfiles selection. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
3570a6e515
commit
1a1f2cebdc
5 files changed
+35
-17
No files matched your search
+2
-2
@@ -7,11 +7,11 @@
|
||||
- **Accepted craft exceptions persist.** /design-html's finalized.json records each accepted `CRAFT_CHECK` finding in `craft_exceptions` (`id`, `count`, `reason`) and carries forward earlier ones. A later run passes that file with `--accepted`, so a finding the user already accepted is reported as accepted instead of flagged again. A higher count is still flagged as a new issue.
|
||||
|
||||
### Added
|
||||
- **`gstack-design-craft --accepted <finalized.json>`**: a finding whose id an exception covers, at no more than the accepted count, prints `CRAFT_ACCEPTED: <id> <count> <reason>` and doesn't fail the check. `CRAFT_RESULT` adds the accepted count. Exit codes are unchanged: 0 when only accepted findings remain, 1 for unaccepted ones, and 2 for a missing, unreadable or malformed exceptions file or an unknown argument. A finalized.json with no `craft_exceptions` field, written before this change, means no exceptions.
|
||||
- **`gstack-design-craft --accepted <finalized.json>`**: a finding whose id an exception covers, at no more than the accepted count, prints `CRAFT_ACCEPTED: <id> <count> <reason>` and doesn't fail the check. `CRAFT_RESULT` adds the accepted count. Exit codes: 0 when only accepted findings remain, 1 for unaccepted ones, and 2 for an unknown argument or a missing, unreadable or malformed exceptions file. A bad exceptions file still gets the full check with nothing accepted, after a warning, so a broken record can't switch the craft check off. A finalized.json with no `craft_exceptions` field, written before this change, means no exceptions.
|
||||
|
||||
### Tests
|
||||
- `design-html-slop-gate` is now selected by changes to `lib/design-craft.ts` and `bin/gstack-design-craft` (`test/helpers/touchfiles-data.ts`), which it already exercised but did not list.
|
||||
- `test/design-craft.test.ts` also covers accepted exceptions: count coverage and overflow, the finalized.json block in design-html/SKILL.md parsed by the same reader the CLI uses, rejected malformed lists, CLI exit codes with `--accepted`, and the archetype handoff contract (the same four ids in /design-shotgun and the craft section, the approved.json step, Case A and the craft section's no-re-ask rule).
|
||||
- `test/design-craft.test.ts` also covers accepted exceptions: count coverage and overflow, duplicate ids carried forward (the highest count wins), a bad exceptions file still checked in full, the finalized.json block in design-html/SKILL.md parsed by the same reader the CLI uses, rejected malformed lists, CLI exit codes with `--accepted`, and the archetype handoff contract (the same four ids in /design-shotgun and the craft section, the approved.json step, Case A and the craft section's no-re-ask rule).
|
||||
|
||||
## [1.91.225.0] - 2026-10-26
|
||||
|
||||
|
||||
+11
-7
@@ -7,10 +7,13 @@
|
||||
// "CRAFT_RESULT: clean" (exit 0) or "CRAFT_RESULT: <n> issue(s)" (exit 1).
|
||||
// --accepted reads the craft_exceptions a finalized.json recorded: a tell an
|
||||
// exception covers (same id, count at most the accepted count) prints as
|
||||
// "CRAFT_ACCEPTED: <id> <count> <reason>" and does not fail the check.
|
||||
// Exit 2: usage error, a non-HTML file, an unreadable or empty file, or an
|
||||
// unreadable or malformed exceptions file. Head and navigation rules run only
|
||||
// on a full HTML document; a fragment gets the copy and code rules alone.
|
||||
// "CRAFT_ACCEPTED: <id> <count> <reason>" and does not fail the check; the
|
||||
// result line then adds " (<k> accepted)" or ", <k> accepted".
|
||||
// Exit 2: usage error, a non-HTML file, or an unreadable or empty file, with
|
||||
// nothing checked; or an unreadable or malformed exceptions file, after a
|
||||
// warning and a full check with no exceptions accepted, so a bad record never
|
||||
// hides the page's tells. Head and navigation rules run only on a full HTML
|
||||
// document; a fragment gets the copy and code rules alone.
|
||||
|
||||
import { readFileSync } from 'fs';
|
||||
import { acceptFindings, checkCraft, formatCraft, parseCraftExceptions, type CraftException } from '../lib/design-craft';
|
||||
@@ -37,14 +40,15 @@ if (source.trim() === '') {
|
||||
process.exit(2);
|
||||
}
|
||||
let exceptions: CraftException[] = [];
|
||||
let badExceptions = false;
|
||||
if (acceptedFile) {
|
||||
try {
|
||||
exceptions = parseCraftExceptions(readFileSync(acceptedFile, 'utf-8'));
|
||||
} catch (err) {
|
||||
console.error(`gstack-design-craft: cannot use exceptions from ${acceptedFile}: ${(err as Error).message}`);
|
||||
process.exit(2);
|
||||
console.error(`gstack-design-craft: cannot use exceptions from ${acceptedFile}: ${(err as Error).message}; checked with none accepted`);
|
||||
badExceptions = true;
|
||||
}
|
||||
}
|
||||
const { issues, accepted } = acceptFindings(checkCraft(source), exceptions);
|
||||
console.log(formatCraft(issues, accepted));
|
||||
process.exit(issues.length === 0 ? 0 : 1);
|
||||
process.exit(badExceptions ? 2 : issues.length === 0 ? 0 : 1);
|
||||
@@ -65,9 +65,9 @@ At Step 4, after the Slop Gate and before the screenshots, run the free checker
|
||||
~/.claude/skills/gstack/bin/gstack-design-craft <path-to-finalized.html>
|
||||
```
|
||||
|
||||
It prints one `CRAFT_CHECK: <id> <count> <fix>` line per tell (the realism rules above, `[Needs proof:` markers, scroll listeners, `100vh`, `transition: all`, `href="#"`, `alert()`, missing alt; on a full document a missing title, description, favicon, working skip link, or `aria-current` on route navigation), then `CRAFT_RESULT: clean` or `CRAFT_RESULT: <n> issue(s)`. No `CRAFT_RESULT` line: say the check was unavailable and continue. Same contract as the Slop Gate: one surgical fix pass, one rerun, the rest listed as accepted-with-reason (a real customer named Nexus, a supplied 50% figure). Rerun once when the refinement loop exits; report only new lines.
|
||||
It prints one `CRAFT_CHECK: <id> <count> <fix>` line per tell (the realism rules above, `[Needs proof:` markers, scroll listeners, `100vh`, `transition: all`, `href="#"`, `alert()`, missing alt; on a full document a missing title, description, favicon, working skip link, or `aria-current` on route navigation), then `CRAFT_RESULT: clean` or `CRAFT_RESULT: <n> issue(s)`, plus the accepted count when `--accepted` covers any. No `CRAFT_RESULT` line: say the check was unavailable and continue. Same contract as the Slop Gate: one surgical fix pass, one rerun, the rest listed as accepted-with-reason (a real customer named Nexus, a supplied 50% figure). Rerun once when the refinement loop exits; report only new lines.
|
||||
|
||||
**Accepted exceptions persist.** Step 5's finalized.json records each one in `craft_exceptions` as `{ id, count, reason }`, carrying forward earlier ones the check still reports. Whenever a finalized.json already sits beside the page, or beside the one you are evolving, append `--accepted <that finalized.json>` to the command: a tell it covers prints `CRAFT_ACCEPTED: <id> <count> <reason>` and does not fail the check, so name those as accepted earlier instead of fixing or re-flagging them. A count above the accepted one stays a `CRAFT_CHECK` line; it is new, so fix it or accept the new count with a reason.
|
||||
**Accepted exceptions persist.** Step 5's finalized.json records each one in `craft_exceptions` as `{ id, count, reason }`, carrying forward earlier ones the check still reports. Whenever a finalized.json already sits beside the page, or beside the one you are evolving, append `--accepted <that finalized.json>` to the command: a tell it covers prints `CRAFT_ACCEPTED: <id> <count> <reason>` and does not fail the check, so name those as accepted earlier instead of fixing or re-flagging them. A count above the accepted one stays a `CRAFT_CHECK` line; it is new, so fix it or accept the new count with a reason. A `cannot use exceptions` warning with exit 2 means that record is malformed: the page was still checked with none accepted, so treat every line as new and write `craft_exceptions` correctly at Step 5.
|
||||
|
||||
**Before this can ship.** Open the Step 4 hand-off, and repeat before Step 5's "Copy to project": every `[Needs proof: …]` marker, the dependencies the page cannot own (legal URLs, form endpoint, og:image, indexing, analytics and cookie consent where required), the accepted exceptions, and every claim the check made you change.
|
||||
|
||||
|
||||
@@ -63,9 +63,9 @@ At Step 4, after the Slop Gate and before the screenshots, run the free checker
|
||||
{{BIN_DIR}}/gstack-design-craft <path-to-finalized.html>
|
||||
```
|
||||
|
||||
It prints one `CRAFT_CHECK: <id> <count> <fix>` line per tell (the realism rules above, `[Needs proof:` markers, scroll listeners, `100vh`, `transition: all`, `href="#"`, `alert()`, missing alt; on a full document a missing title, description, favicon, working skip link, or `aria-current` on route navigation), then `CRAFT_RESULT: clean` or `CRAFT_RESULT: <n> issue(s)`. No `CRAFT_RESULT` line: say the check was unavailable and continue. Same contract as the Slop Gate: one surgical fix pass, one rerun, the rest listed as accepted-with-reason (a real customer named Nexus, a supplied 50% figure). Rerun once when the refinement loop exits; report only new lines.
|
||||
It prints one `CRAFT_CHECK: <id> <count> <fix>` line per tell (the realism rules above, `[Needs proof:` markers, scroll listeners, `100vh`, `transition: all`, `href="#"`, `alert()`, missing alt; on a full document a missing title, description, favicon, working skip link, or `aria-current` on route navigation), then `CRAFT_RESULT: clean` or `CRAFT_RESULT: <n> issue(s)`, plus the accepted count when `--accepted` covers any. No `CRAFT_RESULT` line: say the check was unavailable and continue. Same contract as the Slop Gate: one surgical fix pass, one rerun, the rest listed as accepted-with-reason (a real customer named Nexus, a supplied 50% figure). Rerun once when the refinement loop exits; report only new lines.
|
||||
|
||||
**Accepted exceptions persist.** Step 5's finalized.json records each one in `craft_exceptions` as `{ id, count, reason }`, carrying forward earlier ones the check still reports. Whenever a finalized.json already sits beside the page, or beside the one you are evolving, append `--accepted <that finalized.json>` to the command: a tell it covers prints `CRAFT_ACCEPTED: <id> <count> <reason>` and does not fail the check, so name those as accepted earlier instead of fixing or re-flagging them. A count above the accepted one stays a `CRAFT_CHECK` line; it is new, so fix it or accept the new count with a reason.
|
||||
**Accepted exceptions persist.** Step 5's finalized.json records each one in `craft_exceptions` as `{ id, count, reason }`, carrying forward earlier ones the check still reports. Whenever a finalized.json already sits beside the page, or beside the one you are evolving, append `--accepted <that finalized.json>` to the command: a tell it covers prints `CRAFT_ACCEPTED: <id> <count> <reason>` and does not fail the check, so name those as accepted earlier instead of fixing or re-flagging them. A count above the accepted one stays a `CRAFT_CHECK` line; it is new, so fix it or accept the new count with a reason. A `cannot use exceptions` warning with exit 2 means that record is malformed: the page was still checked with none accepted, so treat every line as new and write `craft_exceptions` correctly at Step 5.
|
||||
|
||||
**Before this can ship.** Open the Step 4 hand-off, and repeat before Step 5's "Copy to project": every `[Needs proof: …]` marker, the dependencies the page cannot own (legal URLs, form endpoint, og:image, indexing, analytics and cookie consent where required), the accepted exceptions, and every claim the check made you change.
|
||||
|
||||
|
||||
@@ -4,6 +4,8 @@ import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { acceptFindings, BUZZWORDS, checkCraft, formatCraft, parseCraftExceptions, visibleText } from '../lib/design-craft';
|
||||
import { catalogEntry } from '../lib/design-catalog';
|
||||
import { selectTests } from './helpers/test-selection';
|
||||
import { E2E_TOUCHFILES } from './helpers/touchfiles-data';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const BIN = path.join(ROOT, 'bin', 'gstack-design-craft');
|
||||
@@ -129,6 +131,10 @@ describe('accepted craft exceptions (finalized.json craft_exceptions)', () => {
|
||||
expect(issues.find(f => f.id === 'round-number')?.fix).toEndWith('(2 accepted earlier: supplied figures)');
|
||||
expect(issues).toHaveLength(findings.length - 2);
|
||||
expect(formatCraft([], accepted)).toBe('CRAFT_ACCEPTED: placeholder-brand 1 Acme is the real customer\nCRAFT_ACCEPTED: scroll-listener 1 unused\nCRAFT_RESULT: clean (2 accepted)');
|
||||
const carried = [{ id: 'round-number', count: 1, reason: 'first run' }, { id: 'round-number', count: 3, reason: 'supplied figures' }];
|
||||
for (const list of [carried, [...carried].reverse()]) {
|
||||
expect(acceptFindings([{ id: 'round-number', count: 3, fix: 'use the supplied figure' }], list)).toEqual({ issues: [], accepted: [{ id: 'round-number', count: 3, reason: 'supplied figures' }] });
|
||||
}
|
||||
});
|
||||
|
||||
test('parseCraftExceptions reads the finalized.json design-html writes and rejects a malformed list', () => {
|
||||
@@ -143,7 +149,7 @@ describe('accepted craft exceptions (finalized.json craft_exceptions)', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('the CLI reports accepted tells as CRAFT_ACCEPTED lines: exit 0 when only accepted ones remain, 1 for the rest, 2 on a bad exceptions file', () => {
|
||||
test('the CLI reports accepted tells as CRAFT_ACCEPTED lines: exit 0 when only accepted ones remain, 1 for the rest, 2 on a bad exceptions file after a full check with none accepted', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'design-craft-accepted-'));
|
||||
try {
|
||||
const page = path.join(dir, 'finalized.html');
|
||||
@@ -164,10 +170,12 @@ describe('accepted craft exceptions (finalized.json craft_exceptions)', () => {
|
||||
fs.writeFileSync(path.join(dir, 'old.json'), JSON.stringify({ mode: 'freeform' }));
|
||||
expect(run(page, '--accepted', path.join(dir, 'old.json')).out).toBe(run(page).out);
|
||||
fs.writeFileSync(path.join(dir, 'bad.json'), '{"craft_exceptions":[{"id":"oops"}]}');
|
||||
expect(run(page, '--accepted', path.join(dir, 'bad.json'))).toMatchObject({ code: 2, out: '' });
|
||||
fs.writeFileSync(path.join(dir, 'broken.json'), '{');
|
||||
expect(run(page, '--accepted', path.join(dir, 'broken.json')).code).toBe(2);
|
||||
expect(run(page, '--accepted', path.join(dir, 'missing.json')).code).toBe(2);
|
||||
for (const name of ['bad.json', 'broken.json', 'missing.json']) {
|
||||
const bad = run(page, '--accepted', path.join(dir, name));
|
||||
expect(bad).toMatchObject({ code: 2, out: run(page).out });
|
||||
expect(bad.err).toContain('checked with none accepted');
|
||||
}
|
||||
expect(run(page, '--accepted').code).toBe(2);
|
||||
expect(run(page, '--strict').code).toBe(2);
|
||||
expect(run(page, '--accepted', record, 'extra').code).toBe(2);
|
||||
@@ -195,6 +203,12 @@ describe('design-shotgun to design-html handoff', () => {
|
||||
expect(craft).toMatch(/When approved\.json carries an `archetype`[^.]*: take it and its `sections` outline as given[^.]*do not re-pick or re-ask/);
|
||||
});
|
||||
|
||||
test('a change to the craft checker selects the design-html-slop-gate E2E that runs it', () => {
|
||||
for (const file of ['lib/design-craft.ts', 'bin/gstack-design-craft']) {
|
||||
expect(selectTests([file], E2E_TOUCHFILES).selected).toContain('design-html-slop-gate');
|
||||
}
|
||||
});
|
||||
|
||||
test('design-html persists accepted exceptions and passes them back with --accepted', () => {
|
||||
const html = read('design-html', 'SKILL.md');
|
||||
const craft = read('design-html', 'sections', 'craft.md');
|
||||
|
||||
Reference in new issue
Block a user