Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/show-resolves-proposalless-changes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@fission-ai/openspec': patch
---

Change lookup no longer requires `proposal.md`. `openspec show`, `openspec change list/show/validate`, and shell completion now resolve a change by its directory, matching `openspec list`, `status`, `instructions`, and `validate`.

Previously a change created by `openspec new change` — which scaffolds only `.openspec.yaml` — was reported as `Unknown item` by `openspec show` and was missing from completions and `openspec change list` until a proposal was written, and a change from a schema with no proposal artifact was never resolvable. `openspec change list` now reports the same set as `openspec list`, keeps task counts for a change that has no proposal yet, and labels it `(no proposal.md yet)` rather than `(unable to read)`. Showing such a change explains that the proposal is not written yet and points at `openspec status --change <name>`.
122 changes: 77 additions & 45 deletions src/commands/change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,26 @@ import { isInteractive } from '../utils/interactive.js';
import { getActiveChangeIds } from '../utils/item-discovery.js';
import { getTaskProgressForChange } from '../utils/task-progress.js';

// Constants for better maintainability
const ARCHIVE_DIR = 'archive';
/**
* True only when `target` is definitively absent. An EACCES or I/O failure
* means existence cannot be determined, so callers fall through to their
* read-error path rather than claim the file was never written.
*/
async function isDefinitelyMissing(target: string): Promise<boolean> {
return fs
.access(target)
.then(() => false)
.catch((error: NodeJS.ErrnoException) => error?.code === 'ENOENT');
}

/**
* A change is a directory directly under changes/. Rejecting anything else up
* front keeps a traversing name (`../..`) from reading a proposal outside the
* changes directory, and keeps the missing-proposal message honest.
*/
function isChangeDirectoryName(changesPath: string, changeDir: string): boolean {
return path.dirname(path.resolve(changeDir)) === path.resolve(changesPath);
}

export class ChangeCommand {
private converter: JsonConverter;
Expand Down Expand Up @@ -39,7 +57,8 @@ export class ChangeCommand {

if (!changeName) {
const canPrompt = isInteractive(options);
const changes = await this.getActiveChanges(changesPath);
// Offer exactly the changes `show <name>` can resolve.
const changes = await getActiveChangeIds(this.rootPath ?? process.cwd());
if (canPrompt && changes.length > 0) {
const { select } = await import('@inquirer/prompts');
const selected = await select({
Expand All @@ -59,11 +78,32 @@ export class ChangeCommand {
}
}

const proposalPath = path.join(changesPath, changeName, 'proposal.md');
const changeDir = path.join(changesPath, changeName);
const proposalPath = path.join(changeDir, 'proposal.md');

if (!isChangeDirectoryName(changesPath, changeDir)) {
throw new Error(`Change "${changeName}" not found at ${proposalPath}`);
}

try {
await fs.access(proposalPath);
} catch {
// A change can exist without a proposal: `openspec new change` scaffolds
// only .openspec.yaml, and a custom schema need not define a proposal
// artifact. Say which of the two cases this is instead of reporting a
// change that does exist as missing. A stray file under changes/ is not a
// change, and naming it one would point the user at a `status --change`
// call that cannot work.
const isChangeDirectory = await fs
.stat(changeDir)
.then((stats) => stats.isDirectory())
.catch(() => false);
if (isChangeDirectory) {
throw new Error(
`Change "${changeName}" has no proposal.md yet. ` +
`Run "openspec status --change ${changeName}" to see which artifact comes next.`
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
throw new Error(`Change "${changeName}" not found at ${proposalPath}`);
}

Expand Down Expand Up @@ -102,36 +142,44 @@ export class ChangeCommand {
async list(options?: { json?: boolean; long?: boolean }): Promise<void> {
const changesPath = path.join(process.cwd(), 'openspec', 'changes');

const changes = await this.getActiveChanges(changesPath);

// Same directory-based resolution as `openspec list`, the command this
// deprecated alias points users at. Every output path below already
// tolerates a change whose proposal.md is missing or unreadable.
const changes = await getActiveChangeIds();

if (options?.json) {
const changeDetails = await Promise.all(
changes.map(async (changeName) => {
const proposalPath = path.join(changesPath, changeName, 'proposal.md');
const changeDir = path.join(changesPath, changeName);
const proposalPath = path.join(changeDir, 'proposal.md');

// Resolve task progress through the shared tracked-tasks helper so
// this deprecated noun-form list cannot re-fork the resolution
// (#1202). Tasks are independent of the proposal: a change can carry
// tasks before, or without, a proposal.md.
const taskStatus = await getTaskProgressForChange(changesPath, changeName, process.cwd());

// No proposal yet is an ordinary state (scaffolded change, or a
// schema with no proposal artifact), so name the change rather than
// labelling it Unknown. Unknown stays for a proposal that exists but
// cannot be read or parsed.
if (await isDefinitelyMissing(proposalPath)) {
return { id: changeName, title: changeName, deltaCount: 0, taskStatus };
}

try {
const content = await fs.readFile(proposalPath, 'utf-8');
const changeDir = path.join(changesPath, changeName);
const parser = new ChangeParser(content, changeDir);
const change = await parser.parseChangeWithDeltas(changeName);

// Resolve task progress through the shared tracked-tasks helper so
// this deprecated noun-form list cannot re-fork the resolution (#1202).
const taskStatus = await getTaskProgressForChange(changesPath, changeName, process.cwd());

return {
id: changeName,
title: this.extractTitle(content, changeName),
deltaCount: change.deltas.length,
taskStatus,
};
} catch (error) {
return {
id: changeName,
title: 'Unknown',
deltaCount: 0,
taskStatus: { total: 0, completed: 0 },
};
} catch {
return { id: changeName, title: 'Unknown', deltaCount: 0, taskStatus };
}
})
);
Expand All @@ -152,19 +200,23 @@ export class ChangeCommand {

// Long format: id: title and minimal counts
for (const changeName of sorted) {
const proposalPath = path.join(changesPath, changeName, 'proposal.md');
const changeDir = path.join(changesPath, changeName);
const proposalPath = path.join(changeDir, 'proposal.md');
const { total, completed } = await getTaskProgressForChange(changesPath, changeName, process.cwd());
const taskStatusText = total > 0 ? ` [tasks ${completed}/${total}]` : '';
if (await isDefinitelyMissing(proposalPath)) {
console.log(`${changeName}: (no proposal.md yet)${taskStatusText}`);
continue;
}
try {
const content = await fs.readFile(proposalPath, 'utf-8');
const title = this.extractTitle(content, changeName);
const { total, completed } = await getTaskProgressForChange(changesPath, changeName, process.cwd());
const taskStatusText = total > 0 ? ` [tasks ${completed}/${total}]` : '';
const changeDir = path.join(changesPath, changeName);
const parser = new ChangeParser(await fs.readFile(proposalPath, 'utf-8'), changeDir);
const parser = new ChangeParser(content, changeDir);
const change = await parser.parseChangeWithDeltas(changeName);
const deltaCountText = ` [deltas ${change.deltas.length}]`;
console.log(`${changeName}: ${title}${deltaCountText}${taskStatusText}`);
} catch {
console.log(`${changeName}: (unable to read)`);
console.log(`${changeName}: (unable to read)${taskStatusText}`);
}
}
}
Expand Down Expand Up @@ -227,26 +279,6 @@ export class ChangeCommand {
}
}

private async getActiveChanges(changesPath: string): Promise<string[]> {
try {
const entries = await fs.readdir(changesPath, { withFileTypes: true });
const result: string[] = [];
for (const entry of entries) {
if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === ARCHIVE_DIR) continue;
const proposalPath = path.join(changesPath, entry.name, 'proposal.md');
try {
await fs.access(proposalPath);
result.push(entry.name);
} catch {
// skip directories without proposal.md
}
}
return result.sort();
} catch {
return [];
}
}

private extractTitle(content: string, changeName: string): string {
const match = content.match(/^#\s+(?:Change:\s+)?(.+)$/im);
return match ? match[1].trim() : changeName;
Expand Down
51 changes: 27 additions & 24 deletions src/utils/item-discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,25 @@ import { promises as fs } from 'fs';
import path from 'path';
import { discoverSpecFiles } from './spec-discovery.js';

/**
* Returns the ids of active changes: every directory under openspec/changes/
* except the archive and hidden directories.
*
* A change is resolved by its directory alone - the same rule `list`,
* `status`, `instructions` and `validate` use (`getAvailableChanges`).
* Requiring proposal.md here made `openspec show` and shell completion miss
* changes those commands resolve: `openspec new change <name>` scaffolds only
* `.openspec.yaml`, and a custom schema need not define a proposal artifact at
* all (#1161).
*/
export async function getActiveChangeIds(root: string = process.cwd()): Promise<string[]> {
const changesPath = path.join(root, 'openspec', 'changes');
try {
const entries = await fs.readdir(changesPath, { withFileTypes: true });
const result: string[] = [];
for (const entry of entries) {
if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'archive') continue;
const proposalPath = path.join(changesPath, entry.name, 'proposal.md');
try {
await fs.access(proposalPath);
result.push(entry.name);
} catch {
// skip directories without proposal.md
}
}
return result.sort();
return entries
.filter((entry) => entry.isDirectory() && entry.name !== 'archive' && !entry.name.startsWith('.'))
.map((entry) => entry.name)
.sort();
} catch {
return [];
}
Expand All @@ -29,22 +32,22 @@ export async function getSpecIds(root: string = process.cwd()): Promise<string[]
return discovered.map((spec) => spec.id);
}

/**
* Returns the ids of archived changes: every directory under
* openspec/changes/archive/ except hidden directories.
*
* Resolved by directory for the same reason as `getActiveChangeIds`: a change
* archived from a schema without a proposal artifact has no proposal.md, and
* gating on it hid those entries from shell completion.
*/
export async function getArchivedChangeIds(root: string = process.cwd()): Promise<string[]> {
const archivePath = path.join(root, 'openspec', 'changes', 'archive');
try {
const entries = await fs.readdir(archivePath, { withFileTypes: true });
const result: string[] = [];
for (const entry of entries) {
if (!entry.isDirectory() || entry.name.startsWith('.')) continue;
const proposalPath = path.join(archivePath, entry.name, 'proposal.md');
try {
await fs.access(proposalPath);
result.push(entry.name);
} catch {
// skip directories without proposal.md
}
}
return result.sort();
return entries
.filter((entry) => entry.isDirectory() && !entry.name.startsWith('.'))
.map((entry) => entry.name)
.sort();
} catch {
return [];
}
Expand Down
47 changes: 47 additions & 0 deletions test/commands/show.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,53 @@ describe('top-level show command', () => {
}
});

it('resolves a scaffolded change that has no proposal.md yet', async () => {
// `openspec new change <name>` writes only .openspec.yaml, so `show` must
// resolve the change the same way `list` and `status` already do.
await fs.mkdir(path.join(changesDir, 'scaffolded'), { recursive: true });
await fs.writeFile(path.join(changesDir, 'scaffolded', '.openspec.yaml'), 'schema: spec-driven\n', 'utf-8');

const originalCwd = process.cwd();
try {
process.chdir(testDir);
let err: any;
try {
execFileSync('node', [openspecBin, 'show', 'scaffolded'], { encoding: 'utf-8' });
} catch (e) { err = e; }
expect(err).toBeDefined();
const stderr = err.stderr.toString();
// Resolved as a change, not rejected as an unknown item.
expect(stderr).not.toContain('Unknown item');
expect(stderr).toContain('has no proposal.md yet');
expect(stderr).toContain('openspec status --change scaffolded');
} finally {
process.chdir(originalCwd);
}
});

it('offers a scaffolded change when "change show" is called without a name', async () => {
await fs.mkdir(path.join(changesDir, 'scaffolded'), { recursive: true });
await fs.writeFile(path.join(changesDir, 'scaffolded', '.openspec.yaml'), 'schema: spec-driven\n', 'utf-8');

const originalCwd = process.cwd();
const originalEnv = { ...process.env };
try {
process.chdir(testDir);
process.env.OPEN_SPEC_INTERACTIVE = '0';
let err: any;
try {
execFileSync('node', [openspecBin, 'change', 'show'], { encoding: 'utf-8' });
} catch (e) { err = e; }
expect(err).toBeDefined();
const stderr = err.stderr.toString();
expect(stderr).toContain('Available IDs:');
expect(stderr).toContain('scaffolded');
} finally {
process.chdir(originalCwd);
process.env = originalEnv;
}
});

it('prints nearest matches when not found', () => {
const originalCwd = process.cwd();
try {
Expand Down
60 changes: 60 additions & 0 deletions test/core/commands/change-command.list.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,65 @@ describe('ChangeCommand.list', () => {
} finally {
console.log = origLog;
}

});
});

describe('ChangeCommand.list with a change that has no proposal.md', () => {
let cmd: ChangeCommand;
let tempRoot: string;
let originalCwd: string;

const capture = async (run: () => Promise<void>): Promise<string> => {
const logs: string[] = [];
const origLog = console.log;
try {
console.log = (msg?: any, ...args: any[]) => {
logs.push([msg, ...args].filter(Boolean).join(' '));
};
await run();
return logs.join('\n');
} finally {
console.log = origLog;
}
};

beforeAll(async () => {
cmd = new ChangeCommand();
originalCwd = process.cwd();
tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-change-list-noproposal-'));
// What `openspec new change` leaves behind, plus tasks: no proposal.md.
const scaffolded = path.join(tempRoot, 'openspec', 'changes', 'scaffolded');
await fs.mkdir(scaffolded, { recursive: true });
await fs.writeFile(path.join(scaffolded, '.openspec.yaml'), 'schema: spec-driven\n', 'utf-8');
await fs.writeFile(path.join(scaffolded, 'tasks.md'), '- [x] Task 1\n- [ ] Task 2\n', 'utf-8');
process.chdir(tempRoot);
});

afterAll(async () => {
process.chdir(originalCwd);
await fs.rm(tempRoot, { recursive: true, force: true });
});

it('lists it, matching what `openspec list` resolves', async () => {
expect(await capture(() => cmd.list({}))).toContain('scaffolded');
});

it('--long reports the missing proposal and keeps task counts', async () => {
const out = await capture(() => cmd.list({ long: true }));
expect(out).toContain('scaffolded: (no proposal.md yet)');
expect(out).toContain('[tasks 1/2]');
expect(out).not.toContain('(unable to read)');
});

it('--json names the change instead of "Unknown" and keeps task counts', async () => {
const parsed = JSON.parse(await capture(() => cmd.list({ json: true })));
expect(parsed).toHaveLength(1);
expect(parsed[0]).toMatchObject({
id: 'scaffolded',
title: 'scaffolded',
deltaCount: 0,
taskStatus: { total: 2, completed: 1 },
});
});
});
Loading
Loading