fix(milestone): make requirements mark-complete idempotent (#948)
Previously, calling `mark-complete` on already-completed requirements reported them as `not_found`, since the regex only matched unchecked `[ ]` checkboxes and `Pending` table cells. Now detects `[x]` checkboxes and `Complete` table cells and returns them in a new `already_complete` array instead of `not_found`. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -33,6 +33,7 @@ function cmdRequirementsMarkComplete(cwd, reqIdsRaw, raw) {
|
||||
|
||||
let reqContent = fs.readFileSync(reqPath, 'utf-8');
|
||||
const updated = [];
|
||||
const alreadyComplete = [];
|
||||
const notFound = [];
|
||||
|
||||
for (const reqId of reqIds) {
|
||||
@@ -60,7 +61,14 @@ function cmdRequirementsMarkComplete(cwd, reqIdsRaw, raw) {
|
||||
if (found) {
|
||||
updated.push(reqId);
|
||||
} else {
|
||||
notFound.push(reqId);
|
||||
// Check if already complete before declaring not_found
|
||||
const doneCheckbox = new RegExp(`-\\s*\\[x\\]\\s*\\*\\*${reqEscaped}\\*\\*`, 'gi');
|
||||
const doneTable = new RegExp(`\\|\\s*${reqEscaped}\\s*\\|[^|]+\\|\\s*Complete\\s*\\|`, 'gi');
|
||||
if (doneCheckbox.test(reqContent) || doneTable.test(reqContent)) {
|
||||
alreadyComplete.push(reqId);
|
||||
} else {
|
||||
notFound.push(reqId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -71,6 +79,7 @@ function cmdRequirementsMarkComplete(cwd, reqIdsRaw, raw) {
|
||||
output({
|
||||
updated: updated.length > 0,
|
||||
marked_complete: updated,
|
||||
already_complete: alreadyComplete,
|
||||
not_found: notFound,
|
||||
total: reqIds.length,
|
||||
}, raw, `${updated.length}/${reqIds.length} requirements marked complete`);
|
||||
|
||||
@@ -583,8 +583,8 @@ describe('requirements mark-complete command', () => {
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
// Regex only matches [ ] (space), not [x], so TEST-03 goes to not_found
|
||||
assert.ok(output.not_found.includes('TEST-03'), 'already-complete ID should be in not_found');
|
||||
assert.ok(output.already_complete.includes('TEST-03'), 'already-complete ID should be in already_complete');
|
||||
assert.deepStrictEqual(output.not_found, [], 'should not appear in not_found');
|
||||
|
||||
const content = readRequirements(tmpDir);
|
||||
// File should not be corrupted — no [xx] or doubled markers
|
||||
@@ -593,6 +593,35 @@ describe('requirements mark-complete command', () => {
|
||||
assert.ok(!content.includes('- [x] [x]'), 'should not have duplicate checkbox');
|
||||
});
|
||||
|
||||
test('returns already_complete for idempotent calls on completed requirements', () => {
|
||||
writeRequirements(tmpDir, STANDARD_REQUIREMENTS);
|
||||
|
||||
// TEST-03 is already [x] in the fixture
|
||||
const result = runGsdTools('requirements mark-complete TEST-03', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.deepStrictEqual(output.already_complete, ['TEST-03'],
|
||||
'should report TEST-03 as already_complete');
|
||||
assert.deepStrictEqual(output.not_found, [],
|
||||
'should not report already-complete IDs as not_found');
|
||||
});
|
||||
|
||||
test('mixed: updates pending, reports already-complete, and flags missing', () => {
|
||||
writeRequirements(tmpDir, STANDARD_REQUIREMENTS);
|
||||
|
||||
const result = runGsdTools('requirements mark-complete TEST-01,TEST-03,FAKE-99', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
|
||||
const output = JSON.parse(result.output);
|
||||
assert.deepStrictEqual(output.marked_complete, ['TEST-01'],
|
||||
'should mark TEST-01 complete');
|
||||
assert.deepStrictEqual(output.already_complete, ['TEST-03'],
|
||||
'should report TEST-03 as already_complete');
|
||||
assert.deepStrictEqual(output.not_found, ['FAKE-99'],
|
||||
'should report FAKE-99 as not_found');
|
||||
});
|
||||
|
||||
test('missing REQUIREMENTS.md returns expected error structure', () => {
|
||||
// createTempProject does not create REQUIREMENTS.md — so it's already missing
|
||||
|
||||
|
||||
Reference in New Issue
Block a user