fix(ci): reuse documentation requirement lookups (#3640)
This commit is contained in:
@@ -1111,6 +1111,8 @@ async function loadCoverageContents({ client, changedFiles, headSha }) {
|
||||
}
|
||||
const contents = {};
|
||||
const loaded = new Set();
|
||||
const requirementSearches = new Map();
|
||||
const requirementDirectories = new Map();
|
||||
let documentCount = 0;
|
||||
let totalBytes = 0;
|
||||
async function load(pathname) {
|
||||
@@ -1213,7 +1215,14 @@ async function loadCoverageContents({ client, changedFiles, headSha }) {
|
||||
const unresolvedRequirementIds = [];
|
||||
if (typeof client.searchCode === 'function') {
|
||||
for (const requirementId of workOrder.requirements ?? []) {
|
||||
const matches = await client.searchCode(requirementId, requirementDirectory);
|
||||
const searchKey = JSON.stringify([requirementDirectory, requirementId]);
|
||||
if (!requirementSearches.has(searchKey)) {
|
||||
requirementSearches.set(
|
||||
searchKey,
|
||||
await client.searchCode(requirementId, requirementDirectory),
|
||||
);
|
||||
}
|
||||
const matches = requirementSearches.get(searchKey);
|
||||
if (matches.length === 0) {
|
||||
unresolvedRequirementIds.push(requirementId);
|
||||
}
|
||||
@@ -1225,14 +1234,18 @@ async function loadCoverageContents({ client, changedFiles, headSha }) {
|
||||
unresolvedRequirementIds.push(...(workOrder.requirements ?? []));
|
||||
}
|
||||
if (unresolvedRequirementIds.length > 0 && typeof client.listDirectory === 'function') {
|
||||
let entries = [];
|
||||
try {
|
||||
entries = await client.listDirectory(requirementDirectory, headSha);
|
||||
} catch (error) {
|
||||
if (!isMissingResourceError(error)) {
|
||||
throw error;
|
||||
if (!requirementDirectories.has(requirementDirectory)) {
|
||||
let entries = [];
|
||||
try {
|
||||
entries = await client.listDirectory(requirementDirectory, headSha);
|
||||
} catch (error) {
|
||||
if (!isMissingResourceError(error)) {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
requirementDirectories.set(requirementDirectory, entries);
|
||||
}
|
||||
const entries = requirementDirectories.get(requirementDirectory);
|
||||
const candidateNames = new Set(unresolvedRequirementIds.map(requirementId => {
|
||||
const parts = requirementId.split('-');
|
||||
return `${parts.slice(2).join('-').replace(/-\d+$/, '').toLowerCase()}.md`;
|
||||
|
||||
@@ -798,6 +798,296 @@ test('coverage loading searches only the referenced requirement files', async ()
|
||||
]);
|
||||
});
|
||||
|
||||
function repeatedCoverageFixture() {
|
||||
const contents = uiFixtureContents();
|
||||
const planPath = 'docs/plans/recovery/plan.md';
|
||||
const workOrderPath = 'docs/plans/recovery/task-01-recovery.md';
|
||||
const designPath = 'docs/specs/ui/system-design/ui-coverage.md';
|
||||
const requirementPath = 'docs/specs/ui/requirements/ui-coverage.md';
|
||||
const requirementIds = [1, 2, 3].map(index => `REQ-UI-COVERAGE-00${index}`);
|
||||
for (const pathname of [planPath, workOrderPath, designPath]) {
|
||||
contents[pathname] = contents[pathname].replace(
|
||||
' - REQ-UI-COVERAGE-001',
|
||||
requirementIds.map(id => ` - ${id}`).join('\n'),
|
||||
);
|
||||
}
|
||||
contents[workOrderPath] = contents[workOrderPath].replace(
|
||||
' - AC-UI-COVERAGE-001.3',
|
||||
requirementIds.map(id => ` - ${id.replace('REQ-', 'AC-')}.3`).join('\n'),
|
||||
);
|
||||
contents[requirementPath] = requirementIds.map(id =>
|
||||
`### ${id}\n\n- **${id.replace('REQ-', 'AC-')}.3:** Runtime changes have a work order.\n`
|
||||
).join('\n');
|
||||
const changed = runtimeAndWorkOrderDiff();
|
||||
for (let index = 2; index <= 6; index += 1) {
|
||||
const basename = `task-0${index}-recovery.md`;
|
||||
const pathname = `docs/plans/recovery/${basename}`;
|
||||
contents[pathname] = contents[workOrderPath].replace('01-recovery', `0${index}-recovery`);
|
||||
contents[planPath] += `\n- [Task ${index}](${basename})\n`;
|
||||
changed.push({ filename: pathname, status: 'modified', changes: 1 });
|
||||
}
|
||||
return { contents, changed, requirementPath, requirementIds };
|
||||
}
|
||||
|
||||
function coverageClient(contents, changed, overrides = {}) {
|
||||
return {
|
||||
async getPullRequest() {
|
||||
return pullRequest(42, SHA_B, [], changed.length);
|
||||
},
|
||||
async listFiles() {
|
||||
return changed;
|
||||
},
|
||||
async getFile(pathname, ref) {
|
||||
assert.equal(ref, SHA_B);
|
||||
if (!Object.hasOwn(contents, pathname)) {
|
||||
throw new Error('GitHub API request failed with HTTP 404: Not Found');
|
||||
}
|
||||
return contents[pathname];
|
||||
},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
// @covers AC-CI-PR-DOCS-001.4, AC-CI-PR-DOCS-003.2
|
||||
test('six work orders sharing three requirements stay within the search quota', async () => {
|
||||
const { contents, changed, requirementPath, requirementIds } = repeatedCoverageFixture();
|
||||
const searches = [];
|
||||
const client = coverageClient(contents, changed, {
|
||||
async searchCode(requirementId, directory) {
|
||||
searches.push({ requirementId, directory });
|
||||
if (searches.length > 10) {
|
||||
throw new Error('GitHub API request failed with HTTP 403: API rate limit exceeded');
|
||||
}
|
||||
return [requirementPath];
|
||||
},
|
||||
});
|
||||
|
||||
const result = await validator.evaluatePullRequest({ client, pullNumber: 42 });
|
||||
|
||||
assert.equal(result.status, 'covered', result.errors.join('; '));
|
||||
assert.equal(result.workOrders.length, 6);
|
||||
assert.deepEqual(searches, requirementIds.map(requirementId => ({
|
||||
requirementId,
|
||||
directory: 'docs/specs/ui/requirements',
|
||||
})));
|
||||
});
|
||||
|
||||
// @covers AC-CI-PR-DOCS-001.4, AC-CI-PR-DOCS-003.2
|
||||
test('PR-only requirements and empty searches reuse a bounded directory lookup', async () => {
|
||||
const { contents, changed, requirementPath, requirementIds } = repeatedCoverageFixture();
|
||||
changed.push({ filename: requirementPath, status: 'added' });
|
||||
let searches = 0;
|
||||
let listings = 0;
|
||||
const client = coverageClient(contents, changed, {
|
||||
async searchCode() {
|
||||
searches += 1;
|
||||
return [];
|
||||
},
|
||||
async listDirectory(directory, ref) {
|
||||
assert.equal(directory, 'docs/specs/ui/requirements');
|
||||
assert.equal(ref, SHA_B);
|
||||
listings += 1;
|
||||
if (listings > 1) {
|
||||
throw new Error('GitHub API request failed with HTTP 429: Too Many Requests');
|
||||
}
|
||||
return [{ path: requirementPath, type: 'file' }];
|
||||
},
|
||||
});
|
||||
|
||||
const result = await validator.evaluatePullRequest({ client, pullNumber: 42 });
|
||||
|
||||
assert.equal(result.status, 'covered', result.errors.join('; '));
|
||||
assert.equal(searches, requirementIds.length);
|
||||
assert.equal(listings, 1);
|
||||
});
|
||||
|
||||
// @covers AC-CI-PR-DOCS-001.4, AC-CI-PR-DOCS-001.5
|
||||
test('directory fallback reuses exact-head entries without accepting missing requirements', async t => {
|
||||
for (const outcome of ['file', 'empty', 'missing']) {
|
||||
await t.test(outcome, async () => {
|
||||
const { contents, changed, requirementPath } = repeatedCoverageFixture();
|
||||
const fallbackPath = 'docs/specs/ui/requirements/coverage.md';
|
||||
contents[fallbackPath] = contents[requirementPath];
|
||||
delete contents[requirementPath];
|
||||
let listings = 0;
|
||||
const client = coverageClient(contents, changed, {
|
||||
async searchCode() {
|
||||
return [];
|
||||
},
|
||||
async listDirectory(directory, ref) {
|
||||
assert.equal(directory, 'docs/specs/ui/requirements');
|
||||
assert.equal(ref, SHA_B);
|
||||
listings += 1;
|
||||
if (outcome === 'missing') {
|
||||
throw new Error('GitHub API request failed with HTTP 404: Not Found');
|
||||
}
|
||||
return outcome === 'file' ? [{ path: fallbackPath, type: 'file' }] : [];
|
||||
},
|
||||
});
|
||||
|
||||
const result = await validator.evaluatePullRequest({ client, pullNumber: 42 });
|
||||
|
||||
assert.equal(result.status, outcome === 'file' ? 'covered' : 'invalid', result.errors.join('; '));
|
||||
assert.equal(listings, 1);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// @covers AC-CI-PR-DOCS-001.4, AC-CI-PR-DOCS-001.5
|
||||
test('designs sharing an ID reuse lookups only within their requirement directory', async () => {
|
||||
const contents = uiFixtureContents();
|
||||
const reference = ' - ../../specs/ui/system-design/ui-coverage.md';
|
||||
for (const pathname of ['docs/plans/recovery/plan.md', 'docs/plans/recovery/task-01-recovery.md']) {
|
||||
contents[pathname] = contents[pathname].replace(reference, `${reference}
|
||||
- ../../specs/ui/system-design/shared.md
|
||||
- ../../specs/ci/system-design/shared.md`);
|
||||
}
|
||||
contents['docs/specs/ui/system-design/shared.md'] = contents['docs/specs/ui/system-design/ui-coverage.md'];
|
||||
contents['docs/specs/ci/system-design/shared.md'] = contents['docs/specs/ui/system-design/ui-coverage.md']
|
||||
.replace('system: ui', 'system: ci');
|
||||
contents['docs/specs/ci/requirements/shared.md'] = contents['docs/specs/ui/requirements/ui-coverage.md'];
|
||||
const searches = [];
|
||||
const client = coverageClient(contents, runtimeAndWorkOrderDiff(), {
|
||||
async searchCode(requirementId, directory) {
|
||||
searches.push({ requirementId, directory });
|
||||
return [directory === 'docs/specs/ui/requirements'
|
||||
? 'docs/specs/ui/requirements/ui-coverage.md'
|
||||
: 'docs/specs/ci/requirements/shared.md'];
|
||||
},
|
||||
});
|
||||
|
||||
const result = await validator.evaluatePullRequest({ client, pullNumber: 42 });
|
||||
|
||||
assert.equal(result.status, 'covered', result.errors.join('; '));
|
||||
assert.deepEqual(searches, ['ui', 'ci'].map(system => ({
|
||||
requirementId: 'REQ-UI-COVERAGE-001',
|
||||
directory: `docs/specs/${system}/requirements`,
|
||||
})));
|
||||
});
|
||||
|
||||
// @covers AC-CI-PR-DOCS-003.2
|
||||
test('lookup errors fail closed without poisoning later evaluations', async t => {
|
||||
for (const boundary of ['searchCode', 'listDirectory']) {
|
||||
await t.test(boundary, async () => {
|
||||
const { contents, changed, requirementPath } = repeatedCoverageFixture();
|
||||
changed.push({ filename: requirementPath, status: 'added' });
|
||||
let fail = true;
|
||||
const client = coverageClient(contents, changed, {
|
||||
async searchCode() {
|
||||
if (fail && boundary === 'searchCode') {
|
||||
throw new Error('GitHub API request failed with HTTP 403: API rate limit exceeded');
|
||||
}
|
||||
return [];
|
||||
},
|
||||
async listDirectory() {
|
||||
if (fail && boundary === 'listDirectory') {
|
||||
throw new Error('GitHub API request failed with HTTP 503: Service Unavailable');
|
||||
}
|
||||
return [];
|
||||
},
|
||||
});
|
||||
|
||||
const failed = await validator.evaluatePullRequest({ client, pullNumber: 42 });
|
||||
assert.equal(failed.ok, false);
|
||||
assert.equal(failed.status, 'error');
|
||||
assert.match(failed.errors.join('; '), /GitHub API request failed/);
|
||||
fail = false;
|
||||
const recovered = await validator.evaluatePullRequest({ client, pullNumber: 42 });
|
||||
assert.equal(recovered.status, 'covered', recovered.errors.join('; '));
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// @covers AC-CI-PR-DOCS-001.5
|
||||
test('lookup reuse preserves ambiguous requirement detection', async t => {
|
||||
for (const source of ['search', 'PR-only file']) {
|
||||
await t.test(source, async () => {
|
||||
const { contents, changed, requirementPath } = repeatedCoverageFixture();
|
||||
const duplicatePath = 'docs/specs/ui/requirements/duplicate.md';
|
||||
contents[duplicatePath] = contents[requirementPath];
|
||||
if (source === 'PR-only file') {
|
||||
changed.push({ filename: duplicatePath, status: 'added' });
|
||||
}
|
||||
const client = coverageClient(contents, changed, {
|
||||
async searchCode() {
|
||||
return source === 'search' ? [requirementPath, duplicatePath] : [requirementPath];
|
||||
},
|
||||
});
|
||||
|
||||
const result = await validator.evaluatePullRequest({ client, pullNumber: 42 });
|
||||
|
||||
assert.equal(result.status, 'invalid');
|
||||
assert.match(result.errors.join('; '), /ambiguous definitions/);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// @covers AC-CI-PR-DOCS-003.1
|
||||
test('revision retries reload requirement lookups and files at the new exact head', async () => {
|
||||
const { contents, changed, requirementPath, requirementIds } = repeatedCoverageFixture();
|
||||
const renamedPath = 'docs/specs/ui/requirements/renamed.md';
|
||||
let metadataReads = 0;
|
||||
let searches = 0;
|
||||
const client = coverageClient(contents, changed, {
|
||||
async getPullRequest() {
|
||||
return pullRequest(42, metadataReads++ === 0 ? SHA_B : SHA_C, [], changed.length);
|
||||
},
|
||||
async searchCode() {
|
||||
searches += 1;
|
||||
return [metadataReads === 1 ? requirementPath : renamedPath];
|
||||
},
|
||||
async getFile(pathname, ref) {
|
||||
assert.equal(ref, metadataReads === 1 ? SHA_B : SHA_C);
|
||||
if (ref === SHA_C && pathname === requirementPath) {
|
||||
throw new Error('GitHub API request failed with HTTP 404: Not Found');
|
||||
}
|
||||
return contents[pathname === renamedPath ? requirementPath : pathname];
|
||||
},
|
||||
});
|
||||
|
||||
const result = await validator.evaluatePullRequest({ client, pullNumber: 42 });
|
||||
|
||||
assert.equal(result.status, 'covered', result.errors.join('; '));
|
||||
assert.equal(result.headSha, SHA_C);
|
||||
assert.equal(searches, requirementIds.length * 2);
|
||||
});
|
||||
|
||||
// @covers AC-CI-PR-DOCS-003.4
|
||||
test('merge-group members do not share cached requirement lookups or contents', async () => {
|
||||
const { contents, changed, requirementPath } = repeatedCoverageFixture();
|
||||
let currentMember;
|
||||
const client = coverageClient(contents, changed, {
|
||||
async getPullRequest(number) {
|
||||
currentMember = number;
|
||||
return pullRequest(number, number === 1 ? SHA_D : SHA_E, [], changed.length);
|
||||
},
|
||||
async searchCode() {
|
||||
return currentMember === 1 ? [requirementPath] : [];
|
||||
},
|
||||
async getFile(pathname, ref) {
|
||||
assert.equal(ref, currentMember === 1 ? SHA_D : SHA_E);
|
||||
if (currentMember === 2 && pathname === requirementPath) {
|
||||
throw new Error('GitHub API request failed with HTTP 404: Not Found');
|
||||
}
|
||||
return contents[pathname];
|
||||
},
|
||||
});
|
||||
|
||||
const result = await validator.evaluateMergeGroup({
|
||||
client,
|
||||
baseSha: SHA_A,
|
||||
headSha: SHA_C,
|
||||
entries: [
|
||||
{ baseCommit: { oid: SHA_A }, headCommit: { oid: SHA_B }, pullRequest: { number: 1, headRefOid: SHA_D } },
|
||||
{ baseCommit: { oid: SHA_B }, headCommit: { oid: SHA_C }, pullRequest: { number: 2, headRefOid: SHA_E } },
|
||||
],
|
||||
});
|
||||
|
||||
assert.equal(result.ok, false);
|
||||
assert.deepEqual(result.memberResults.map(member => member.status), ['covered', 'invalid']);
|
||||
assert.match(result.memberResults[1].errors.join('; '), /not defined in ui requirements/);
|
||||
});
|
||||
|
||||
test('missing referenced artifacts become invalid coverage through the API adapter', async () => {
|
||||
const contents = fixtureContents();
|
||||
const workOrderPath = 'docs/plans/recovery/task-01-recovery.md';
|
||||
|
||||
@@ -66,6 +66,15 @@ Execute sequentially. No subagents are authorized.
|
||||
|
||||
## Verification results
|
||||
|
||||
Task 01's 2026-09-13 regression fix reuses requirement searches and directory
|
||||
listings within each exact-head snapshot. Its deterministic quota regression
|
||||
now uses three searches for six work orders. All 50 Node tests, five workflow
|
||||
contract tests, documentation catalog/specification checks, and whitespace
|
||||
checks pass. A GET-only evaluation of exact PR #3626 head `93abff34efe8c9ab2bec68acb8e6aba1eb2a23d4`
|
||||
returned covered with three searches and no failed requests. See Task 01 for
|
||||
red/green evidence and limits. Consumption by trusted `main` and the new PR's
|
||||
CI/review gates remain delivery work; no status override or merge is claimed.
|
||||
|
||||
Design validation on 2026-09-10:
|
||||
|
||||
- `python3 scripts/lint-spec-files.test.py`: 36 tests passed.
|
||||
|
||||
@@ -7,12 +7,17 @@ depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-CI-PR-DOCS-001
|
||||
- REQ-CI-PR-DOCS-003
|
||||
acceptance_criteria:
|
||||
- AC-CI-PR-DOCS-001.2
|
||||
- AC-CI-PR-DOCS-001.3
|
||||
- AC-CI-PR-DOCS-001.4
|
||||
- AC-CI-PR-DOCS-001.5
|
||||
- AC-CI-PR-DOCS-001.6
|
||||
- AC-CI-PR-DOCS-003.1
|
||||
- AC-CI-PR-DOCS-003.2
|
||||
- AC-CI-PR-DOCS-003.3
|
||||
- AC-CI-PR-DOCS-003.4
|
||||
system_design:
|
||||
- ../../specs/ci/system-design/pull-request-documentation-coverage.md
|
||||
---
|
||||
@@ -37,6 +42,9 @@ Application code, unrelated workflow cleanup, live label changes, and live rules
|
||||
- Only explicitly exempt changes pass without a work order; the #3137 runtime-change fixture fails.
|
||||
- A changed work order and valid referenced package pass without cosmetic edits to existing specifications.
|
||||
- Missing, empty, deleted, escaping, or mismatched references produce precise failures.
|
||||
- Repeated requirement lookups reuse successful results only within one PR
|
||||
snapshot and requirement directory. Empty search results still load changed
|
||||
requirements at the exact head; failed lookups remain evaluation errors.
|
||||
|
||||
## Verification
|
||||
|
||||
@@ -44,6 +52,7 @@ Run from the repository root:
|
||||
|
||||
```bash
|
||||
node --test .github/scripts/pr-docs.test.cjs
|
||||
python3 .github/scripts/pr-docs-workflow-contract_test.py
|
||||
git diff --check
|
||||
```
|
||||
|
||||
@@ -85,3 +94,44 @@ Verification:
|
||||
|
||||
- `node --test .github/scripts/pr-docs.test.cjs`: 35 tests passed.
|
||||
- `git diff --check`: passed.
|
||||
|
||||
### Requirement lookup regression, 2026-09-13
|
||||
|
||||
Successful code-search results (including empty results) now reuse a cache
|
||||
keyed by requirement directory and ID within one exact-head snapshot.
|
||||
Directory fallback reuses the complete listing for that snapshot. Existing
|
||||
bounded file reads still load PR-only requirements and search candidates at
|
||||
the exact head. Each distinct ID is searched even when a previously loaded
|
||||
document defines it, so other candidate definitions remain visible to the
|
||||
ambiguity check. Lookup failures remain errors; retries and merge-group
|
||||
members get fresh caches.
|
||||
|
||||
TDD evidence:
|
||||
|
||||
- Red: six work orders sharing three IDs exhausted a deterministic ten-search
|
||||
budget on request 11 and returned `error` instead of `covered`.
|
||||
- Green: the same fixture returns `covered` with three searches. A second red
|
||||
regression exceeded a one-listing budget for PR-only requirements; it now
|
||||
passes with one listing and three empty search results.
|
||||
- `node --test .github/scripts/pr-docs.test.cjs`: passed.
|
||||
- `node --test-isolation=none --test --test-reporter=dot .github/scripts/pr-docs.test.cjs`:
|
||||
50 tests passed, including system/design boundaries, PR-only and missing
|
||||
requirements, ambiguous definitions, failure recovery, head retries, and
|
||||
independent merge-group members. Disabling process isolation exposes the
|
||||
individual tests in this executor, whose default runner reports only the file.
|
||||
- `python3 .github/scripts/pr-docs-workflow-contract_test.py`: 5 tests passed.
|
||||
- `python3 scripts/list-docs.py validate`: 265 decisions and 824 specifications validated.
|
||||
- `python3 scripts/lint-spec-files.py --all`: passed.
|
||||
- `git diff --check`: passed.
|
||||
|
||||
A GET-only diagnostic using this workspace's fixed evaluator on exact PR
|
||||
#3626 head `93abff34efe8c9ab2bec68acb8e6aba1eb2a23d4` returned `covered`: six
|
||||
work orders, nine accepted references, three searches, 18 total requests, and
|
||||
no failed requests. It published no status. The earlier CI run's caught
|
||||
exception was not observed, and distinct IDs or a depleted shared quota can
|
||||
still fail closed. The fix must reach trusted `main` before the privileged
|
||||
workflow consumes it; rerunning an older run does not substitute this branch's
|
||||
evaluator. Workflow permissions, bounds, policy, and status rules are unchanged.
|
||||
|
||||
Internal docs updated only. This CI implementation change has no public product
|
||||
documentation or screenshot impact.
|
||||
|
||||
Reference in New Issue
Block a user