From ba7a57aaa71d5b6a882f677842fa243658ed1b45 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Tue, 21 Apr 2026 10:47:22 +0200 Subject: [PATCH] Clarify Guardex bootstrap failures before scaffold writes New repos can already contain .codex as a file. The bootstrap path previously surfaced a raw ENOTDIR and did not write the managed .gitignore until later in setup/doctor. The installer now checks parent path segments, reports the blocking file with the target path, and writes the managed ignore block before scaffold/template work so Guardex-owned files are ignored even on early failure. Constraint: Some repos already reserve .codex as a file Rejected: Ignore the whole scripts/ directory | would hide user-owned automation in new repos Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep managed .gitignore initialization ahead of scaffold/template writes Tested: node --test --test-name-pattern "setup and doctor explain \.codex file conflicts and still write managed gitignore first" test/install.test.js Tested: node bin/multiagent-safety.js doctor --target /home/deadpool/Documents/machine-monitoring-system-esp-firmware Tested: openspec validate agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35 --type change --strict Tested: openspec validate --specs Not-tested: full install test suite --- bin/multiagent-safety.js | 40 ++++++++++++++----- .../.openspec.yaml | 2 + .../proposal.md | 18 +++++++++ .../codex-path-conflict-bootstrap/spec.md | 16 ++++++++ .../tasks.md | 22 ++++++++++ test/install.test.js | 25 ++++++++++++ 6 files changed, 113 insertions(+), 10 deletions(-) create mode 100644 openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/.openspec.yaml create mode 100644 openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/proposal.md create mode 100644 openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/specs/codex-path-conflict-bootstrap/spec.md create mode 100644 openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/tasks.md diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index 182d0de9..2025e494 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -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) { @@ -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); @@ -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); @@ -3973,6 +3991,10 @@ 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) { @@ -3980,9 +4002,6 @@ function runInstallInternal(options) { } 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) })); @@ -4017,6 +4036,10 @@ 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) { @@ -4024,9 +4047,6 @@ function runFixInternal(options) { } operations.push(ensureLockRegistry(repoRoot, Boolean(options.dryRun))); - if (!options.skipGitignore) { - operations.push(ensureManagedGitignore(repoRoot, Boolean(options.dryRun))); - } const lockState = lockStateOrError(repoRoot); if (!lockState.ok) { diff --git a/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/.openspec.yaml b/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/.openspec.yaml new file mode 100644 index 00000000..4b8c565f --- /dev/null +++ b/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-04-21 diff --git a/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/proposal.md b/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/proposal.md new file mode 100644 index 00000000..13af665c --- /dev/null +++ b/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/proposal.md @@ -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. diff --git a/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/specs/codex-path-conflict-bootstrap/spec.md b/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/specs/codex-path-conflict-bootstrap/spec.md new file mode 100644 index 00000000..453c41a4 --- /dev/null +++ b/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/specs/codex-path-conflict-bootstrap/spec.md @@ -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. diff --git a/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/tasks.md b/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/tasks.md new file mode 100644 index 00000000..402ca3f5 --- /dev/null +++ b/openspec/changes/agent-codex-handle-codex-file-path-and-early-gitigno-2026-04-21-10-35/tasks.md @@ -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. diff --git a/test/install.test.js b/test/install.test.js index 8f760b27..c939b1cb 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -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');