Skip to content

feat(archive): use sequence numbers instead of date prefix for deterministic ordering #787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Br1an67 wants to merge 3 commits into
base: main
Choose a base branch
from
+55 −16
Open

feat(archive): use sequence numbers instead of date prefix for deterministic ordering #787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16b8462
feat(archive): use sequence numbers instead of date prefix for determ…
Br1an67 Mar 1, 2026
9e02675
fix(archive): handle non-ENOENT errors and add sequence overflow guard
Br1an67 Mar 2, 2026
943d156
docs: add single-user assumption comment on sequence assignment
Br1an67 Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions src/core/archive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,11 @@ export class ArchiveCommand {
}
}

// Create archive directory with date prefix
const archiveName = `${this.getArchiveDate()}-${changeName}`;
// Create archive directory with sequence number prefix.
// Note: sequence assignment is non-atomic, but this is a single-user CLI
// tool where concurrent archive operations are not a realistic scenario.
const sequenceNumber = await this.getNextSequenceNumber(archiveDir);
const archiveName = `${sequenceNumber}-${changeName}`;
const archivePath = path.join(archiveDir, archiveName);

// Check if archive already exists
Expand Down Expand Up @@ -332,8 +335,27 @@ export class ArchiveCommand {
}
}

private getArchiveDate(): string {
// Returns date in YYYY-MM-DD format
return new Date().toISOString().split('T')[0];
private async getNextSequenceNumber(archiveDir: string): Promise<string> {
Copy link

@tanght1994 tanght1994 Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a newbie. I have a simple question. What happens after 999? Or will there be no situation after 999?

Copy link
Author

@Br1an67 Br1an67 Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With padStart(3, '0'), once we hit 1000 the prefix becomes 4 digits, which breaks lexical sorting ("1000" sorts before "999"). I'll add an overflow guard that throws a clear error if it exceeds 999. In practice, 999 archived specs should be more than sufficient for any project.

let max = 0;
try {
const entries = await fs.readdir(archiveDir);
for (const entry of entries) {
const match = entry.match(/^(\d+)-/);
if (match) {
const num = parseInt(match[1], 10);
if (num > max) max = num;
}
}
} catch (error: any) {
// Archive directory may not exist yet
if (error?.code !== 'ENOENT') {
throw error;
}
}
const next = max + 1;
if (next > 999) {
throw new Error('Archive sequence overflow (>999). Increase prefix width before continuing.');
}
return String(next).padStart(3, '0');
}
Comment on lines +338 to 360
Copy link

@greptile-apps greptile-apps bot Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race condition: if two archive operations run concurrently for different changes, both may read the same max value and use the same sequence number (e.g., both get "006"), breaking deterministic ordering. Consider using atomic operations or file locking, or document that concurrent archiving is unsupported.

Copy link
Author

@Br1an67 Br1an67 Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a single-user CLI tool — concurrent archive operations aren't a realistic scenario. (Same point discussed in the coderabbit thread above.)

Comment on lines +338 to 360
Copy link

@greptile-apps greptile-apps bot Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider handling concurrent archiving: if two archives run simultaneously, they may read the same max and use duplicate sequence numbers, undermining deterministic ordering. Atomic operations or a note in docs would help.

Copy link
Author

@Br1an67 Br1an67 Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate of the above — answered in the coderabbit thread. Single-user CLI, concurrency is not applicable.

}
39 changes: 28 additions & 11 deletions test/core/archive.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('ArchiveCommand', () => {
const archives = await fs.readdir(archiveDir);

expect(archives.length).toBe(1);
expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`));
expect(archives[0]).toMatch(new RegExp(`\\d{3}-${changeName}`));

// Verify original change directory no longer exists
await expect(fs.access(changeDir)).rejects.toThrow();
Expand Down Expand Up @@ -265,15 +265,15 @@ New feature description.
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
await fs.mkdir(changeDir, { recursive: true });

// Create existing archive with same date
const date = new Date().toISOString().split('T')[0];
const archivePath = path.join(tempDir, 'openspec', 'changes', 'archive', `${date}-${changeName}`);
// Create existing archive with sequence number prefix
const archivePath = path.join(tempDir, 'openspec', 'changes', 'archive', `001-${changeName}`);
await fs.mkdir(archivePath, { recursive: true });

// Try to archive
await expect(
archiveCommand.execute(changeName, { yes: true })
).rejects.toThrow(`Archive '${date}-${changeName}' already exists.`);
// Try to archive — next sequence is 002 so no collision
await archiveCommand.execute(changeName, { yes: true });
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
const archives = await fs.readdir(archiveDir);
expect(archives).toContain(`002-${changeName}`);
});

it('should handle changes without tasks.md', async () => {
Expand All @@ -295,6 +295,23 @@ New feature description.
expect(archives.length).toBe(1);
});

it('should use monotonically increasing sequence numbers for archive names', async () => {
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');

// Archive first change
const change1 = 'first-feature';
await fs.mkdir(path.join(tempDir, 'openspec', 'changes', change1), { recursive: true });
await archiveCommand.execute(change1, { yes: true });

// Archive second change
const change2 = 'second-feature';
await fs.mkdir(path.join(tempDir, 'openspec', 'changes', change2), { recursive: true });
await archiveCommand.execute(change2, { yes: true });

const archives = (await fs.readdir(archiveDir)).sort();
expect(archives).toEqual([`001-${change1}`, `002-${change2}`]);
});

it('should handle changes without specs', async () => {
const changeName = 'no-specs-feature';
const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
Expand Down Expand Up @@ -340,7 +357,7 @@ New feature description.
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
const archives = await fs.readdir(archiveDir);
expect(archives.length).toBe(1);
expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`));
expect(archives[0]).toMatch(new RegExp(`\\d{3}-${changeName}`));
});

it('should skip validation when commander sets validate to false (--no-validate)', async () => {
Expand Down Expand Up @@ -376,7 +393,7 @@ The system will log all events.
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
const archives = await fs.readdir(archiveDir);
expect(archives.length).toBe(1);
expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`));
expect(archives[0]).toMatch(new RegExp(`\\d{3}-${changeName}`));
} finally {
deltaSpy.mockRestore();
specContentSpy.mockRestore();
Expand Down Expand Up @@ -433,7 +450,7 @@ Then expected result happens`;
const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive');
const archives = await fs.readdir(archiveDir);
expect(archives.length).toBe(1);
expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`));
expect(archives[0]).toMatch(new RegExp(`\\d{3}-${changeName}`));
});

it('should support header trim-only normalization for matching', async () => {
Expand Down