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
40 changes: 30 additions & 10 deletions bin/multiagent-safety.js
Original file line number Diff line number Diff line change
Expand Up @@ -611,9 +611,27 @@ function toDestinationPath(relativeTemplatePath) {
throw new Error(`Unsupported template path: ${relativeTemplatePath}`);
}

function ensureParentDir(filePath, dryRun) {
function ensureParentDir(repoRoot, filePath, dryRun) {
if (dryRun) return;
fs.mkdirSync(path.dirname(filePath), { recursive: true });

const parentDir = path.dirname(filePath);
const relativeParentDir = path.relative(repoRoot, parentDir);
const segments = relativeParentDir.split(path.sep).filter(Boolean);
let currentPath = repoRoot;

for (const segment of segments) {
currentPath = path.join(currentPath, segment);
if (fs.existsSync(currentPath) && !fs.statSync(currentPath).isDirectory()) {
const blockingPath = path.relative(repoRoot, currentPath) || path.basename(currentPath);
const targetPath = path.relative(repoRoot, filePath) || path.basename(filePath);
throw new Error(
`Path conflict: ${blockingPath} exists as a file, but ${targetPath} needs it to be a directory. ` +
`Remove or rename ${blockingPath} and rerun '${SHORT_TOOL_NAME} setup'.`,
);
}
}

fs.mkdirSync(parentDir, { recursive: true });
}

function ensureExecutable(destinationPath, relativePath, dryRun) {
Expand Down Expand Up @@ -648,7 +666,7 @@ function copyTemplateFile(repoRoot, relativeTemplatePath, force, dryRun) {
}
}

ensureParentDir(destinationPath, dryRun);
ensureParentDir(repoRoot, destinationPath, dryRun);
if (!dryRun) {
fs.writeFileSync(destinationPath, sourceContent, 'utf8');
ensureExecutable(destinationPath, destinationRelativePath, dryRun);
Expand Down Expand Up @@ -686,7 +704,7 @@ function ensureTemplateFilePresent(repoRoot, relativeTemplatePath, dryRun) {
return { status: 'skipped-conflict', file: destinationRelativePath };
}

ensureParentDir(destinationPath, dryRun);
ensureParentDir(repoRoot, destinationPath, dryRun);
if (!dryRun) {
fs.writeFileSync(destinationPath, sourceContent, 'utf8');
ensureExecutable(destinationPath, destinationRelativePath, dryRun);
Expand Down Expand Up @@ -3973,16 +3991,17 @@ function runInstallInternal(options) {
}
const operations = [];

if (!options.skipGitignore) {
operations.push(ensureManagedGitignore(repoRoot, Boolean(options.dryRun)));
}

operations.push(...ensureOmxScaffold(repoRoot, Boolean(options.dryRun)));

for (const templateFile of TEMPLATE_FILES) {
operations.push(copyTemplateFile(repoRoot, templateFile, Boolean(options.force), Boolean(options.dryRun)));
}

operations.push(ensureLockRegistry(repoRoot, Boolean(options.dryRun)));
if (!options.skipGitignore) {
operations.push(ensureManagedGitignore(repoRoot, Boolean(options.dryRun)));
}

if (!options.skipPackageJson) {
operations.push(ensurePackageScripts(repoRoot, Boolean(options.dryRun), { force: Boolean(options.force) }));
Expand Down Expand Up @@ -4017,16 +4036,17 @@ function runFixInternal(options) {
}
const operations = [];

if (!options.skipGitignore) {
operations.push(ensureManagedGitignore(repoRoot, Boolean(options.dryRun)));
}

operations.push(...ensureOmxScaffold(repoRoot, Boolean(options.dryRun)));

for (const templateFile of TEMPLATE_FILES) {
operations.push(ensureTemplateFilePresent(repoRoot, templateFile, Boolean(options.dryRun)));
}

operations.push(ensureLockRegistry(repoRoot, Boolean(options.dryRun)));
if (!options.skipGitignore) {
operations.push(ensureManagedGitignore(repoRoot, Boolean(options.dryRun)));
}

const lockState = lockStateOrError(repoRoot);
if (!lockState.ok) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-04-21
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## Why

- `gx setup` and `gx doctor` currently fail with a raw `ENOTDIR` when a target repo contains a `.codex` file instead of a `.codex/` directory.
- On a fresh repo, Guardex also writes the managed `.gitignore` block too late, so a partial bootstrap can leave a noisy-looking working tree before the install aborts.

## What Changes

- Detect file-vs-directory path conflicts during template installation and throw a Guardex-specific error that explains the blocking path and the fix.
- Write the managed `.gitignore` block before scaffolding directories and templates in both setup and doctor/fix paths.
- Add regression coverage for the `.codex` file conflict so both `setup` and `doctor` fail clearly while still creating `.gitignore`.

## Impact

- Affected surfaces:
- `bin/multiagent-safety.js`
- `test/install.test.js`
- Risk is low and scoped to repo bootstrap/repair flows.
- No change to steady-state runtime behavior after a successful install.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## ADDED Requirements

### Requirement: codex path conflict error clarity
Guardex setup and doctor SHALL explain when a reserved path such as `.codex` is a file instead of a directory.

#### Scenario: .codex file blocks bootstrap
- **WHEN** the target repo contains a regular file at `.codex`
- **THEN** `gx setup` and `gx doctor` fail with a readable Guardex error naming `.codex`
- **AND** the error explains that the path must be removed or renamed before retrying.

### Requirement: early managed gitignore bootstrap
Guardex SHALL write its managed `.gitignore` block before later bootstrap steps that can fail on path conflicts.

#### Scenario: partial bootstrap on path conflict
- **WHEN** setup or doctor aborts after `.gitignore` is written but before the full scaffold completes
- **THEN** the repo still has the managed `.gitignore` entries for generated scripts, lock state, and Guardex-managed local paths.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## 1. Specification

- [x] 1.1 Finalize proposal scope and acceptance criteria for `agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35`.
- [x] 1.2 Define normative requirements in `specs/codex-path-conflict-bootstrap/spec.md`.

## 2. Implementation

- [x] 2.1 Patch installer path handling so `.codex` file conflicts fail with a readable Guardex error.
- [x] 2.2 Move managed `.gitignore` creation earlier in setup/doctor bootstrap.
- [x] 2.3 Add regression coverage for `.codex`-as-file setup/doctor failures.

## 3. Verification

- [x] 3.1 Run targeted `node --test test/install.test.js` coverage for the new repo bootstrap case.
- [x] 3.2 Run `openspec validate agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35 --type change --strict`.
- [x] 3.3 Run `openspec validate --specs`.

## 4. Cleanup

- [ ] 4.1 Run `bash scripts/agent-branch-finish.sh --branch agent/codex/handle-codex-file-path-and-early-gitigno-2026-04-21-10-35 --base main --via-pr --wait-for-merge --cleanup`.
- [ ] 4.2 Record PR URL and final merge state.
- [ ] 4.3 Confirm sandbox worktree and refs are cleaned up.
25 changes: 25 additions & 0 deletions test/install.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,31 @@ test('setup provisions workflow files and repo config', () => {
assert.equal(secondRun.status, 0, secondRun.stderr || secondRun.stdout);
});

test('setup and doctor explain .codex file conflicts and still write managed gitignore first', () => {
const repoDir = initRepo();
fs.writeFileSync(path.join(repoDir, '.codex'), '', 'utf8');

let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir);
assert.notEqual(result.status, 0, 'setup should fail when .codex is a file');
let combined = `${result.stdout}\n${result.stderr}`;
assert.match(combined, /Path conflict: \.codex exists as a file/);
assert.match(combined, /\.codex\/skills\/gitguardex\/SKILL\.md needs it to be a directory/);

let gitignoreContent = fs.readFileSync(path.join(repoDir, '.gitignore'), 'utf8');
assert.match(gitignoreContent, /# multiagent-safety:START/);
assert.match(gitignoreContent, /scripts\/agent-branch-start\.sh/);
assert.match(gitignoreContent, /scripts\/agent-file-locks\.py/);
assert.match(gitignoreContent, /\.codex\/skills\/gitguardex\/SKILL\.md/);

result = runNode(['doctor', '--target', repoDir], repoDir);
assert.notEqual(result.status, 0, 'doctor should fail when .codex is a file');
combined = `${result.stdout}\n${result.stderr}`;
assert.match(combined, /Path conflict: \.codex exists as a file/);

gitignoreContent = fs.readFileSync(path.join(repoDir, '.gitignore'), 'utf8');
assert.match(gitignoreContent, /scripts\/agent-file-locks\.py/);
});

test('setup and doctor skip repo bootstrap when repo .env disables Guardex', () => {
const repoDir = initRepo();
fs.writeFileSync(path.join(repoDir, '.env'), 'GUARDEX_ON=0\n', 'utf8');
Expand Down