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
163 changes: 144 additions & 19 deletions bin/multiagent-safety.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,20 @@ const TEMPLATE_FILES = [
'vscode/guardex-active-agents/README.md',
];

const LEGACY_WORKFLOW_SHIMS = [
'scripts/agent-branch-start.sh',
'scripts/agent-branch-finish.sh',
'scripts/agent-branch-merge.sh',
'scripts/codex-agent.sh',
'scripts/review-bot-watch.sh',
'scripts/agent-worktree-prune.sh',
'scripts/agent-file-locks.py',
'scripts/openspec/init-plan-workspace.sh',
'scripts/openspec/init-change-workspace.sh',
const LEGACY_WORKFLOW_SHIM_SPECS = [
{ relativePath: 'scripts/agent-branch-start.sh', kind: 'shell', command: ['branch', 'start'] },
{ relativePath: 'scripts/agent-branch-finish.sh', kind: 'shell', command: ['branch', 'finish'] },
{ relativePath: 'scripts/agent-branch-merge.sh', kind: 'shell', command: ['branch', 'merge'] },
{ relativePath: 'scripts/codex-agent.sh', kind: 'shell', command: ['internal', 'run-shell', 'codexAgent'] },
{ relativePath: 'scripts/review-bot-watch.sh', kind: 'shell', command: ['internal', 'run-shell', 'reviewBot'] },
{ relativePath: 'scripts/agent-worktree-prune.sh', kind: 'shell', command: ['worktree', 'prune'] },
{ relativePath: 'scripts/agent-file-locks.py', kind: 'python', command: ['locks'] },
{ relativePath: 'scripts/openspec/init-plan-workspace.sh', kind: 'shell', command: ['internal', 'run-shell', 'planInit'] },
{ relativePath: 'scripts/openspec/init-change-workspace.sh', kind: 'shell', command: ['internal', 'run-shell', 'changeInit'] },
];

const LEGACY_WORKFLOW_SHIMS = LEGACY_WORKFLOW_SHIM_SPECS.map((entry) => entry.relativePath);

const MANAGED_TEMPLATE_DESTINATIONS = TEMPLATE_FILES.map((entry) => toDestinationPath(entry));
const MANAGED_TEMPLATE_SCRIPT_FILES = MANAGED_TEMPLATE_DESTINATIONS.filter((entry) =>
entry.startsWith('scripts/'),
Expand Down Expand Up @@ -248,6 +250,13 @@ const OMX_SCAFFOLD_FILES = new Map([
['.omx/notepad.md', '\n\n## WORKING MEMORY\n'],
['.omx/project-memory.json', '{}\n'],
]);
const TARGETED_FORCEABLE_MANAGED_PATHS = new Set([
'AGENTS.md',
'.gitignore',
...Array.from(OMX_SCAFFOLD_FILES.keys()),
...REQUIRED_MANAGED_REPO_FILES,
...LEGACY_WORKFLOW_SHIMS,
]);
const COMMAND_TYPO_ALIASES = new Map([
['relaese', 'release'],
['realaese', 'release'],
Expand Down Expand Up @@ -1037,6 +1046,13 @@ function renderPythonDispatchShim(commandParts) {
);
}

function managedForceConflictMessage(relativePath) {
return (
`Refusing to overwrite existing file without --force: ${relativePath}\n` +
`Use '--force ${relativePath}' to rewrite only this managed file, or '--force' to rewrite all managed files.`
);
}

function renderManagedFile(repoRoot, relativePath, content, options = {}) {
const destinationPath = path.join(repoRoot, relativePath);
const destinationExists = fs.existsSync(destinationPath);
Expand All @@ -1050,7 +1066,7 @@ function renderManagedFile(repoRoot, relativePath, content, options = {}) {
return { status: 'unchanged', file: relativePath };
}
if (!force && !isCriticalGuardrailPath(relativePath)) {
throw new Error(`Refusing to overwrite existing file without --force: ${relativePath}`);
throw new Error(managedForceConflictMessage(relativePath));
}
}

Expand Down Expand Up @@ -1098,9 +1114,7 @@ function copyTemplateFile(repoRoot, relativeTemplatePath, force, dryRun) {
return { status: 'unchanged', file: destinationRelativePath };
}
if (!force && !isCriticalGuardrailPath(destinationRelativePath)) {
throw new Error(
`Refusing to overwrite existing file without --force: ${destinationRelativePath}`,
);
throw new Error(managedForceConflictMessage(destinationRelativePath));
}
}

Expand Down Expand Up @@ -1151,6 +1165,22 @@ function ensureTemplateFilePresent(repoRoot, relativeTemplatePath, dryRun) {
return { status: 'created', file: destinationRelativePath };
}

function ensureTargetedLegacyWorkflowShims(repoRoot, options) {
const targetedPaths = Array.isArray(options.forceManagedPaths) ? options.forceManagedPaths : [];
if (targetedPaths.length === 0) {
return [];
}

const operations = [];
for (const shim of LEGACY_WORKFLOW_SHIM_SPECS) {
if (!shouldForceManagedPath(options, shim.relativePath)) {
continue;
}
operations.push(ensureGeneratedScriptShim(repoRoot, shim, { dryRun: options.dryRun, force: true }));
}
return operations;
}

function lockFilePath(repoRoot) {
return path.join(repoRoot, LOCK_FILE_RELATIVE);
}
Expand Down Expand Up @@ -1457,8 +1487,65 @@ function requireValue(rawArgs, index, flagName) {
return value;
}

function normalizeManagedForcePath(rawPath) {
if (typeof rawPath !== 'string') {
return null;
}
const normalized = path.posix.normalize(rawPath.replace(/\\/g, '/'));
if (!normalized || normalized === '.' || normalized.startsWith('../') || path.posix.isAbsolute(normalized)) {
return null;
}
return normalized.startsWith('./') ? normalized.slice(2) : normalized;
}

function collectForceManagedPaths(rawArgs, startIndex) {
const forceManagedPaths = [];
let nextIndex = startIndex;

while (nextIndex + 1 < rawArgs.length) {
const candidate = rawArgs[nextIndex + 1];
if (!candidate || candidate.startsWith('-')) {
break;
}
const normalized = normalizeManagedForcePath(candidate);
if (!normalized || !TARGETED_FORCEABLE_MANAGED_PATHS.has(normalized)) {
throw new Error(`Unknown managed path after --force: ${candidate}`);
}
forceManagedPaths.push(normalized);
nextIndex += 1;
}

return { forceManagedPaths, nextIndex };
}

function appendForceArgs(args, options) {
if (!options.force) {
return;
}
args.push('--force');
for (const managedPath of options.forceManagedPaths || []) {
args.push(managedPath);
}
}

function shouldForceManagedPath(options, relativePath) {
if (!options.force) {
return false;
}
const targetedPaths = Array.isArray(options.forceManagedPaths) ? options.forceManagedPaths : [];
if (targetedPaths.length === 0) {
return true;
}
const normalized = normalizeManagedForcePath(relativePath);
return normalized !== null && targetedPaths.includes(normalized);
}

function parseCommonArgs(rawArgs, defaults) {
const options = { ...defaults };
const supportsForce = Object.prototype.hasOwnProperty.call(options, 'force');
if (supportsForce && !Array.isArray(options.forceManagedPaths)) {
options.forceManagedPaths = [];
}

for (let index = 0; index < rawArgs.length; index += 1) {
const arg = rawArgs[index];
Expand All @@ -1480,7 +1567,17 @@ function parseCommonArgs(rawArgs, defaults) {
continue;
}
if (arg === '--force') {
if (!supportsForce) {
throw new Error(`Unknown option: ${arg}`);
}
options.force = true;
const parsed = collectForceManagedPaths(rawArgs, index);
if (parsed.forceManagedPaths.length > 0) {
options.forceManagedPaths = Array.from(
new Set([...(options.forceManagedPaths || []), ...parsed.forceManagedPaths]),
);
}
index = parsed.nextIndex;
continue;
}
if (arg === '--keep-stale-locks') {
Expand Down Expand Up @@ -1598,6 +1695,7 @@ function parseSetupArgs(rawArgs, defaults) {
function parseDoctorArgs(rawArgs) {
const doctorDefaults = {
target: process.cwd(),
force: false,
dropStaleLocks: true,
skipAgents: false,
skipPackageJson: false,
Expand Down Expand Up @@ -1746,6 +1844,7 @@ function runSetupBootstrapInternal(options) {
target: installPayload.repoRoot,
dryRun: options.dryRun,
force: options.force,
forceManagedPaths: options.forceManagedPaths,
dropStaleLocks: true,
skipAgents: options.skipAgents,
skipPackageJson: options.skipPackageJson,
Expand Down Expand Up @@ -1783,7 +1882,7 @@ function resolveSandboxTarget(repoRoot, worktreePath, targetPath) {

function buildSandboxSetupArgs(options, sandboxTarget) {
const args = ['setup', '--target', sandboxTarget, '--no-global-install', '--no-recursive'];
if (options.force) args.push('--force');
appendForceArgs(args, options);
if (options.skipAgents) args.push('--skip-agents');
if (options.skipPackageJson) args.push('--skip-package-json');
if (options.skipGitignore) args.push('--no-gitignore');
Expand All @@ -1794,7 +1893,7 @@ function buildSandboxSetupArgs(options, sandboxTarget) {
function buildSandboxDoctorArgs(options, sandboxTarget) {
const args = ['doctor', '--target', sandboxTarget];
if (options.dryRun) args.push('--dry-run');
if (options.force) args.push('--force');
appendForceArgs(args, options);
if (options.skipAgents) args.push('--skip-agents');
if (options.skipPackageJson) args.push('--skip-package-json');
if (options.skipGitignore) args.push('--no-gitignore');
Expand Down Expand Up @@ -5154,10 +5253,24 @@ function runInstallInternal(options) {
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(
copyTemplateFile(
repoRoot,
templateFile,
shouldForceManagedPath(options, toDestinationPath(templateFile)),
Boolean(options.dryRun),
),
);
}
operations.push(...ensureTargetedLegacyWorkflowShims(repoRoot, options));
for (const hookName of HOOK_NAMES) {
operations.push(ensureHookShim(repoRoot, hookName, options));
const hookRelativePath = path.posix.join('.githooks', hookName);
operations.push(
ensureHookShim(repoRoot, hookName, {
dryRun: options.dryRun,
force: shouldForceManagedPath(options, hookRelativePath),
}),
);
}

operations.push(ensureLockRegistry(repoRoot, Boolean(options.dryRun)));
Expand Down Expand Up @@ -5198,10 +5311,21 @@ function runFixInternal(options) {
operations.push(...ensureOmxScaffold(repoRoot, Boolean(options.dryRun)));

for (const templateFile of TEMPLATE_FILES) {
if (shouldForceManagedPath(options, toDestinationPath(templateFile))) {
operations.push(copyTemplateFile(repoRoot, templateFile, true, Boolean(options.dryRun)));
continue;
}
operations.push(ensureTemplateFilePresent(repoRoot, templateFile, Boolean(options.dryRun)));
}
operations.push(...ensureTargetedLegacyWorkflowShims(repoRoot, options));
for (const hookName of HOOK_NAMES) {
operations.push(ensureHookShim(repoRoot, hookName, options));
const hookRelativePath = path.posix.join('.githooks', hookName);
operations.push(
ensureHookShim(repoRoot, hookName, {
dryRun: options.dryRun,
force: shouldForceManagedPath(options, hookRelativePath),
}),
);
}

operations.push(ensureLockRegistry(repoRoot, Boolean(options.dryRun)));
Expand Down Expand Up @@ -5715,6 +5839,7 @@ function doctor(rawArgs) {
'--single-repo',
'--target',
repoPath,
...(options.force ? ['--force', ...(options.forceManagedPaths || [])] : []),
...(options.dropStaleLocks ? [] : ['--keep-stale-locks']),
...(options.skipAgents ? ['--skip-agents'] : []),
...(options.skipPackageJson ? ['--skip-package-json'] : []),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## Why

- `gx doctor` currently surfaces managed-file conflicts like `scripts/review-bot-watch.sh`, but the recovery hint is incomplete: users can reasonably try `gx doctor --force scripts/review-bot-watch.sh` and hit `Unknown option`.
- `gx setup` has the same gap for managed template conflicts like `.github/workflows/cr.yml`.
- The CLI already distinguishes managed files from repo-owned package scripts and AGENTS content, so the remaining missing piece is a safe, explicit way to force only the named managed path instead of all managed files.

## What Changes

- Allow `gx setup`, `gx doctor`, and the shared repair/install aliases to accept managed relative paths after `--force`.
- Keep plain `--force` as the whole-surface rewrite path.
- Update managed-file conflict errors to explain both recovery options:
- `--force <managed-path>` to rewrite only that file
- `--force` to rewrite all managed files
- Add install regressions for targeted doctor/setup force-path recovery.

## Scope

- `bin/multiagent-safety.js`
- `test/install.test.js`

## Risks

- Path matching must stay relative and deterministic so targeted force rewrites only the named managed file.
- The parser change must not accidentally relax other commands into accepting stray positional arguments.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## ADDED Requirements

### Requirement: setup and doctor accept targeted managed-file force paths

`gx setup` and `gx doctor` SHALL accept one or more managed relative paths after `--force` so users can repair only the named managed files instead of rewriting the entire managed surface.

#### Scenario: doctor rewrites one named managed shim

- **GIVEN** a repo has a conflicting managed `scripts/review-bot-watch.sh`
- **WHEN** the user runs `gx doctor --force scripts/review-bot-watch.sh`
- **THEN** the command succeeds
- **AND** `scripts/review-bot-watch.sh` is rewritten to the current managed shim
- **AND** the path selector is not treated as an unknown option

#### Scenario: setup rewrites one named managed template

- **GIVEN** a repo has a conflicting managed `.github/workflows/cr.yml`
- **WHEN** the user runs `gx setup --force .github/workflows/cr.yml`
- **THEN** the command succeeds
- **AND** `.github/workflows/cr.yml` is rewritten to the current managed template

### Requirement: conflict output teaches targeted and global force recovery

When a managed file conflict blocks `gx setup` or `gx doctor`, the CLI SHALL tell the user how to recover with either a targeted `--force <managed-path>` or a full-surface `--force`.

#### Scenario: conflict message names both force paths

- **GIVEN** a managed file differs from the current Guardex output
- **WHEN** `gx setup` or `gx doctor` hits that conflict without `--force`
- **THEN** the error names the conflicting managed path
- **AND** the error teaches `--force <managed-path>` for one-file recovery
- **AND** the error teaches plain `--force` for rewriting all managed files
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
## 1. Spec

- [x] 1.1 Define the targeted managed-file `--force` behavior in `specs/doctor-setup-force-targets/spec.md`.
- [x] 1.2 Capture the recovery UX problem and bounded scope in `proposal.md`.

## 2. Tests

- [x] 2.1 Add a regression that `gx doctor --force scripts/review-bot-watch.sh` rewrites the named managed shim instead of throwing `Unknown option`.
- [x] 2.2 Add a regression that `gx setup --force .github/workflows/cr.yml` rewrites the named managed template.
- [x] 2.3 Lock the conflict message so it teaches both targeted `--force <managed-path>` and global `--force`.

## 3. Implementation

- [x] 3.1 Extend the shared setup/doctor/install/fix arg parsing to accept managed path selectors only after `--force`.
- [x] 3.2 Route targeted force-path matching through the managed file/template rewrite helpers.
- [x] 3.3 Preserve the existing plain `--force` behavior for whole-surface rewrites.

## 4. Verification

- [x] 4.1 Run `node --check bin/multiagent-safety.js`.
- [x] 4.2 Run targeted install regressions in `test/install.test.js`.
- [x] 4.3 Run `openspec validate agent-codex-fix-doctor-setup-force-conflict-ux-2026-04-22-08-58 --type change --strict`.
- [x] 4.4 Run `openspec validate --specs`.

## 5. Cleanup

- [x] 5.1 Confirm the OpenSpec tasks reflect the shipped behavior and note any residual risk. Residual risk: targeted `--force` selectors intentionally fail fast for unlisted paths, and this worktree currently has no main specs for `openspec validate --specs` beyond the clean `No items found to validate.` result.
- [ ] 5.2 Finish the agent branch via PR merge + cleanup (`gx finish --via-pr --wait-for-merge --cleanup` or `bash scripts/agent-branch-finish.sh --branch <agent-branch> --base <base-branch> --via-pr --wait-for-merge --cleanup`).
- [ ] 5.3 Record PR URL + final `MERGED` evidence in the completion handoff.
Loading