morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #7665] [Bug] Skill setup joins ENOTDIR diagnostics to recovery text and recommends permission repair #4589

New issue

Closed

opened 2026-10-31 00:51:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:51:03 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 28, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/7665

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

Setup, packaging, or upgrade

Observed behavior

With a regular file occupying .agents in an isolated HOME, node scripts/rea.mjs setup --yes --skill --json correctly refuses installation with exit 1, status: needs_human, code: skill_install_failed and no applied actions. Its remediation nevertheless joins the native ENOTDIR diagnostic directly to the next instruction as ) Use a regular skill directory, repeats the selected destination in parentheses after the native error already named it, and ends with Check permissions and rerun setup.

The fixture has a non-directory ancestor, rather than a demonstrated permission denial. The same response already supplies useful regular-directory and registration-only guidance; the defect is the misleading and poorly separated recovery wording, not the refusal itself.

Reproduction

From the built baseline checkout, select Node.js v24.22.1 first on PATH. Use the existing compiled runtime and bundled skill; no installation is needed.
Run this inline recipe from the checkout root. It discards inherited configuration overrides, isolates HOME/XDG/npm/temp paths, places recording-only npx/npm/bun fixtures first on PATH, and automatically removes the fixture tree. REPRO_SCRATCH can select an existing disposable parent directory.

python3 -I - <<'PY_REPRO'
import json, os, pathlib, shutil, subprocess, tempfile
repo = pathlib.Path.cwd()
node = str(pathlib.Path(shutil.which('node')).resolve())
with tempfile.TemporaryDirectory(dir=os.environ.get('REPRO_SCRATCH')) as trial:
    root = pathlib.Path(trial)
    home, fixtures = root / 'home', root / 'bin'
    home.mkdir(); fixtures.mkdir()
    runner = '''#!/bin/sh
printf '%s\\n' "${0##*/} $*" >> "$FIXTURE_LOG"
printf '6.8.0\\n'
'''
    for name in ('npx', 'npm', 'bun'):
        executable = fixtures / name
        executable.write_text(runner); executable.chmod(0o755)
    env = {'HOME': str(home), 'USERPROFILE': str(home),
           'PATH': f'{fixtures}:{pathlib.Path(node).parent}:/usr/bin:/bin:/usr/sbin:/sbin',
           'FIXTURE_LOG': str(root / 'runners.log'), 'CI': '1', 'NO_COLOR': '1'}
    for key, name in (('XDG_CONFIG_HOME', 'config'), ('XDG_CACHE_HOME', 'cache'),
                      ('XDG_DATA_HOME', 'data'), ('XDG_STATE_HOME', 'state'),
                      ('TMPDIR', 'tmp'), ('npm_config_prefix', 'npm-global')):
        directory = home / name; directory.mkdir(); env[key] = str(directory)
    for name in ('npx', 'npm', 'bun'):
        assert shutil.which(name, path=env['PATH']) == str(fixtures / name)
    cli = [node, str(repo / 'scripts/rea.mjs')]
    print('REA:', repr(subprocess.check_output(cli + ['--version'], env=env, cwd=home, text=True)))
    blocker = home / '.agents'
    blocker.write_text('blocking regular file\n')
    def run():
        result = subprocess.run(cli + ['setup', '--yes', '--skill', '--json'],
                                cwd=home, env=env, capture_output=True, text=True, timeout=60)
        assert result.stderr == ''
        data = json.loads(result.stdout)
        return result.returncode, data
    for attempt in range(1, 3):
        code, data = run()
        assert code == 1 and data['status'] == 'needs_human'
        assert data['code'] == 'skill_install_failed' and data['appliedActions'] == []
        text = data['remediation']
        assert 'ENOTDIR:' in text and ') Use a regular skill directory' in text
        assert text.endswith('Check permissions and rerun setup.')
        assert text.count(str(home / '.agents/skills/reverse-engineer-anything')) == 3
        assert blocker.read_text() == 'blocking regular file\n'
        print(json.dumps({'attempt': attempt, 'exit': code, 'status': data['status'],
                          'code': data['code'], 'appliedActions': data['appliedActions'],
                          'remediation': text}).replace(str(home), '<HOME>'))
    blocker.rename(home / 'saved-blocker')
    blocker.mkdir()
    code, data = run()
    assert code == 0 and data['status'] == 'ready'
    selected = home / '.agents/skills/reverse-engineer-anything'
    files = [p for p in selected.rglob('*') if p.is_file()]
    assert files and all(p.read_bytes() == (repo / 'skills/reverse-engineer-anything' /
                                          p.relative_to(selected)).read_bytes() for p in files)
    assert (home / 'saved-blocker').read_text() == 'blocking regular file\n'
    print(json.dumps({'control': 'directory repaired', 'exit': code, 'status': data['status'],
                      'canonical_files': len(files)}))
    log = root / 'runners.log'
    print('Package runners:', log.read_text() if log.exists() else 'none invoked')
PY_REPRO

Read the two failure records and the directory-repair control. Both failures assert the missing sentence boundary, three destination occurrences, generic permission advice, safe refusal and unchanged blocker bytes. The control preserves the blocking file, provides a directory, and reruns the same setup command successfully.

The complete recipe passed twice in this verification, including a replay extracted from this report, producing four matching failure records and two successful controls. Two earlier independent CLI runs reproduced the same failure, for six failure observations overall. No package-manager fixture was invoked.

Expected behavior

A refused skill installation should clearly distinguish the filesystem diagnostic from the recovery instruction. When an ancestor is a regular file, remediation should explain that a directory is required or offer the existing registration-only alternative. It should preserve the selected path and native reason without needlessly repeating the destination, and avoid presenting permission repair as the remedy for every filesystem failure.

The repository recognizes confusing or incorrect user instructions as reportable and requires meaningful boundary failure reasons. The error catalogue also claims an implemented skill-recovery sentence that differs from the current output. That stale record should describe the current recovery contract and its actual verification; restoring its older permission-only sentence verbatim would not address this input.

Impact and scope

P3: misleading recovery wording for a local configuration that is already refused safely. The raw ENOTDIR reason remains visible, and the response includes a useful directory instruction. No data loss, wrong successful result, permission change or consequential automated recovery failure was observed.

The demonstrated workaround is to preserve the regular file and provide the required directory. The identical setup command then returned ready, installed eight files matching the bundled bytes, and preserved the saved blocker. Registration-only --skill=false is advertised in the response but was not exercised here.

Verified scope is prepare-stage skill setup on this macOS host. Other skill stages, actual permission failures, MCP transport, other operating systems and real-world frequency remain untested.

REA version

Exact node scripts/rea.mjs --version stdout:

6.8.0

The stdout ends with one newline. Source baseline: 65f1c447344a423fa2785847dbcea1e9a9e2454a. The checkout's existing compiled CLI was executed.

Runtime and platform

Node.js v24.22.1; macOS 27.0.1; Darwin arm64. No analysis provider was intentionally launched.

Target and artifact context

A disposable text file at .agents in an isolated HOME, followed by a directory-repair control using the bundled reverse-engineering skill. No executable analysis target or MCP client.

Interface

CLI

Earliest failure stage

Setup or provider admission

Relevant output or evidence

Both failures in the inline recipe produced these fields. Only the generated HOME path is replaced with <HOME>:

{
  "status": "needs_human",
  "code": "skill_install_failed",
  "appliedActions": [],
  "remediation": "REA skill prepare failed at <HOME>/.agents/skills/reverse-engineer-anything: ENOTDIR: not a directory, lstat '<HOME>/.agents/skills/reverse-engineer-anything' (<HOME>/.agents/skills/reverse-engineer-anything) Use a regular skill directory, or run setup with --skill=false to configure the client without installing the skill. Check permissions and rerun setup."
}

The same recipe printed:

{"control": "directory repaired", "exit": 0, "status": "ready", "canonical_files": 8}
Package runners: none invoked

All setup invocations had empty stderr. The failure assertions verified exit 1 and unchanged blocker contents; the control assertions verified exit 0, canonical file bytes and preserved saved-file contents.

Contract and source checks at the baseline:

CONTRIBUTING.md:5-11 recognizes confusing or incorrect user instructions as standalone corrections. AGENTS.md, Architecture and Cleanup, requires meaningful failure reasons.
docs/user-visible-errors.csv:47, record harm_rank 35, covers skill installation/readback failure. It marks REA analysis skill could not be installed or verified. Check permissions for the shared skill directory then rerun setup as implemented, with focused-assertion verification. Its complete replacement string has zero literal matches in TypeScript source. This is catalogue/source drift, not evidence that the older wording is appropriate for ENOTDIR.
src/application/SetupSkill.ts:300-325 admits ENOENT but records ENOTDIR as an unreadable directory issue, preserving the native lstat reason. Lines 347-357 append the selected path and directory guidance; the raw diagnostic is not sentence-terminated at that join. Lines 120-143 add the stage/path wrapper and unconditional permission advice.
src/application/Setup.ts:266-269 checks selected directories during preparation and returns failSkill; lines 116-122 project the recovery text with skill_install_failed.
src/application/Setup.recovery.test.ts:102-128 intentionally checks stage/path context and permission text for an injected write failure. That inspected test does not establish the correctness of permission advice for the independently reproduced ENOTDIR input. It was not executed here.

Typed outcome or recovery expected

Keep the failure exit, needs_human, skill_install_failed, empty applied actions and native path/reason. Present a separate, accurate directory-repair instruction and retain the registration-only alternative. No particular new error code or exact replacement sentence is required.

Analysis or suspected cause

Source-derived explanation: directory inspection stores the native error as free text; the prepare-stage formatter adds that same path in parentheses and concatenates its next instruction with a space. The outer error formatter then appends permission guidance for every skill failure. This explains the directly observed output. Why the catalogue was not updated is unknown.

Additional context

Related open #7618 concerns concurrent first-install publication being described as an input-read change. It shares the unconditional permission suffix, but its closure requires concurrency-specific write-stage recovery. This case concerns a static non-directory ancestor and the prepare-stage diagnostic/guidance join; neither its path repetition nor its ENOTDIR recovery is covered by that report.

Open #7607 addresses reference-import ELOOP/ENAMETOOLONG recovery in another producer. Open PR #7163 only repoints catalogue owner locations and explicitly excludes stale replacement wording. Closed #5077 corrected the process-input size-policy catalogue entry. Closed #3057 and merged PR #3108 concern linked-directory mutation and preservation; this report does not claim those fixes regressed.

Open/closed issue, issue-comment and open/closed/merged PR searches found no matching prepare-stage ENOTDIR wording report. No checkout files were changed and no GitHub write was made during verification. No fix, released-package replay or known-good historical version was tested.

Triage assessment

Impact level: P3
Assessment status: supported
Impact basis: A safely refused non-directory configuration gets a repeated path, a missing sentence boundary and generic permission advice. The native reason and usable directory guidance remain visible; no consequential behavior change was demonstrated.
Workaround status: available
Workaround basis: Preserving the blocking file and making the required directory allowed the identical command to return ready and install eight canonical files, without altering permissions or losing the preserved file.

Originally created by @coygeek on GitHub (Oct 28, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7665 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area Setup, packaging, or upgrade ### Observed behavior With a regular file occupying `.agents` in an isolated HOME, `node scripts/rea.mjs setup --yes --skill --json` correctly refuses installation with exit 1, `status: needs_human`, `code: skill_install_failed` and no applied actions. Its remediation nevertheless joins the native ENOTDIR diagnostic directly to the next instruction as `) Use a regular skill directory`, repeats the selected destination in parentheses after the native error already named it, and ends with `Check permissions and rerun setup.` The fixture has a non-directory ancestor, rather than a demonstrated permission denial. The same response already supplies useful regular-directory and registration-only guidance; the defect is the misleading and poorly separated recovery wording, not the refusal itself. ### Reproduction 1. From the built baseline checkout, select Node.js v24.22.1 first on PATH. Use the existing compiled runtime and bundled skill; no installation is needed. 2. Run this inline recipe from the checkout root. It discards inherited configuration overrides, isolates HOME/XDG/npm/temp paths, places recording-only npx/npm/bun fixtures first on PATH, and automatically removes the fixture tree. REPRO_SCRATCH can select an existing disposable parent directory. ```sh python3 -I - <<'PY_REPRO' import json, os, pathlib, shutil, subprocess, tempfile repo = pathlib.Path.cwd() node = str(pathlib.Path(shutil.which('node')).resolve()) with tempfile.TemporaryDirectory(dir=os.environ.get('REPRO_SCRATCH')) as trial: root = pathlib.Path(trial) home, fixtures = root / 'home', root / 'bin' home.mkdir(); fixtures.mkdir() runner = '''#!/bin/sh printf '%s\\n' "${0##*/} $*" >> "$FIXTURE_LOG" printf '6.8.0\\n' ''' for name in ('npx', 'npm', 'bun'): executable = fixtures / name executable.write_text(runner); executable.chmod(0o755) env = {'HOME': str(home), 'USERPROFILE': str(home), 'PATH': f'{fixtures}:{pathlib.Path(node).parent}:/usr/bin:/bin:/usr/sbin:/sbin', 'FIXTURE_LOG': str(root / 'runners.log'), 'CI': '1', 'NO_COLOR': '1'} for key, name in (('XDG_CONFIG_HOME', 'config'), ('XDG_CACHE_HOME', 'cache'), ('XDG_DATA_HOME', 'data'), ('XDG_STATE_HOME', 'state'), ('TMPDIR', 'tmp'), ('npm_config_prefix', 'npm-global')): directory = home / name; directory.mkdir(); env[key] = str(directory) for name in ('npx', 'npm', 'bun'): assert shutil.which(name, path=env['PATH']) == str(fixtures / name) cli = [node, str(repo / 'scripts/rea.mjs')] print('REA:', repr(subprocess.check_output(cli + ['--version'], env=env, cwd=home, text=True))) blocker = home / '.agents' blocker.write_text('blocking regular file\n') def run(): result = subprocess.run(cli + ['setup', '--yes', '--skill', '--json'], cwd=home, env=env, capture_output=True, text=True, timeout=60) assert result.stderr == '' data = json.loads(result.stdout) return result.returncode, data for attempt in range(1, 3): code, data = run() assert code == 1 and data['status'] == 'needs_human' assert data['code'] == 'skill_install_failed' and data['appliedActions'] == [] text = data['remediation'] assert 'ENOTDIR:' in text and ') Use a regular skill directory' in text assert text.endswith('Check permissions and rerun setup.') assert text.count(str(home / '.agents/skills/reverse-engineer-anything')) == 3 assert blocker.read_text() == 'blocking regular file\n' print(json.dumps({'attempt': attempt, 'exit': code, 'status': data['status'], 'code': data['code'], 'appliedActions': data['appliedActions'], 'remediation': text}).replace(str(home), '<HOME>')) blocker.rename(home / 'saved-blocker') blocker.mkdir() code, data = run() assert code == 0 and data['status'] == 'ready' selected = home / '.agents/skills/reverse-engineer-anything' files = [p for p in selected.rglob('*') if p.is_file()] assert files and all(p.read_bytes() == (repo / 'skills/reverse-engineer-anything' / p.relative_to(selected)).read_bytes() for p in files) assert (home / 'saved-blocker').read_text() == 'blocking regular file\n' print(json.dumps({'control': 'directory repaired', 'exit': code, 'status': data['status'], 'canonical_files': len(files)})) log = root / 'runners.log' print('Package runners:', log.read_text() if log.exists() else 'none invoked') PY_REPRO ``` 3. Read the two failure records and the directory-repair control. Both failures assert the missing sentence boundary, three destination occurrences, generic permission advice, safe refusal and unchanged blocker bytes. The control preserves the blocking file, provides a directory, and reruns the same setup command successfully. The complete recipe passed twice in this verification, including a replay extracted from this report, producing four matching failure records and two successful controls. Two earlier independent CLI runs reproduced the same failure, for six failure observations overall. No package-manager fixture was invoked. ### Expected behavior A refused skill installation should clearly distinguish the filesystem diagnostic from the recovery instruction. When an ancestor is a regular file, remediation should explain that a directory is required or offer the existing registration-only alternative. It should preserve the selected path and native reason without needlessly repeating the destination, and avoid presenting permission repair as the remedy for every filesystem failure. The repository recognizes confusing or incorrect user instructions as reportable and requires meaningful boundary failure reasons. The error catalogue also claims an implemented skill-recovery sentence that differs from the current output. That stale record should describe the current recovery contract and its actual verification; restoring its older permission-only sentence verbatim would not address this input. ### Impact and scope P3: misleading recovery wording for a local configuration that is already refused safely. The raw ENOTDIR reason remains visible, and the response includes a useful directory instruction. No data loss, wrong successful result, permission change or consequential automated recovery failure was observed. The demonstrated workaround is to preserve the regular file and provide the required directory. The identical setup command then returned `ready`, installed eight files matching the bundled bytes, and preserved the saved blocker. Registration-only `--skill=false` is advertised in the response but was not exercised here. Verified scope is prepare-stage skill setup on this macOS host. Other skill stages, actual permission failures, MCP transport, other operating systems and real-world frequency remain untested. ### REA version Exact `node scripts/rea.mjs --version` stdout: ```text 6.8.0 ``` The stdout ends with one newline. Source baseline: `65f1c447344a423fa2785847dbcea1e9a9e2454a`. The checkout's existing compiled CLI was executed. ### Runtime and platform Node.js v24.22.1; macOS 27.0.1; Darwin arm64. No analysis provider was intentionally launched. ### Target and artifact context A disposable text file at `.agents` in an isolated HOME, followed by a directory-repair control using the bundled reverse-engineering skill. No executable analysis target or MCP client. ### Interface CLI ### Earliest failure stage Setup or provider admission ### Relevant output or evidence Both failures in the inline recipe produced these fields. Only the generated HOME path is replaced with `<HOME>`: ```json { "status": "needs_human", "code": "skill_install_failed", "appliedActions": [], "remediation": "REA skill prepare failed at <HOME>/.agents/skills/reverse-engineer-anything: ENOTDIR: not a directory, lstat '<HOME>/.agents/skills/reverse-engineer-anything' (<HOME>/.agents/skills/reverse-engineer-anything) Use a regular skill directory, or run setup with --skill=false to configure the client without installing the skill. Check permissions and rerun setup." } ``` The same recipe printed: ```text {"control": "directory repaired", "exit": 0, "status": "ready", "canonical_files": 8} Package runners: none invoked ``` All setup invocations had empty stderr. The failure assertions verified exit 1 and unchanged blocker contents; the control assertions verified exit 0, canonical file bytes and preserved saved-file contents. Contract and source checks at the baseline: - `CONTRIBUTING.md:5-11` recognizes confusing or incorrect user instructions as standalone corrections. `AGENTS.md`, Architecture and Cleanup, requires meaningful failure reasons. - `docs/user-visible-errors.csv:47`, record harm_rank 35, covers skill installation/readback failure. It marks `REA analysis skill could not be installed or verified. Check permissions for` the shared skill directory `then rerun setup` as implemented, with focused-assertion verification. Its complete replacement string has zero literal matches in TypeScript source. This is catalogue/source drift, not evidence that the older wording is appropriate for ENOTDIR. - `src/application/SetupSkill.ts:300-325` admits ENOENT but records ENOTDIR as an unreadable directory issue, preserving the native lstat reason. Lines 347-357 append the selected path and directory guidance; the raw diagnostic is not sentence-terminated at that join. Lines 120-143 add the stage/path wrapper and unconditional permission advice. - `src/application/Setup.ts:266-269` checks selected directories during preparation and returns `failSkill`; lines 116-122 project the recovery text with `skill_install_failed`. - `src/application/Setup.recovery.test.ts:102-128` intentionally checks stage/path context and permission text for an injected write failure. That inspected test does not establish the correctness of permission advice for the independently reproduced ENOTDIR input. It was not executed here. ### Typed outcome or recovery expected Keep the failure exit, `needs_human`, `skill_install_failed`, empty applied actions and native path/reason. Present a separate, accurate directory-repair instruction and retain the registration-only alternative. No particular new error code or exact replacement sentence is required. ### Analysis or suspected cause Source-derived explanation: directory inspection stores the native error as free text; the prepare-stage formatter adds that same path in parentheses and concatenates its next instruction with a space. The outer error formatter then appends permission guidance for every skill failure. This explains the directly observed output. Why the catalogue was not updated is unknown. ### Additional context Related open [#7618](https://github.com/morluto/rea/issues/7618) concerns concurrent first-install publication being described as an input-read change. It shares the unconditional permission suffix, but its closure requires concurrency-specific write-stage recovery. This case concerns a static non-directory ancestor and the prepare-stage diagnostic/guidance join; neither its path repetition nor its ENOTDIR recovery is covered by that report. Open [#7607](https://github.com/morluto/rea/issues/7607) addresses reference-import ELOOP/ENAMETOOLONG recovery in another producer. Open [PR #7163](https://github.com/morluto/rea/pull/7163) only repoints catalogue owner locations and explicitly excludes stale replacement wording. Closed [#5077](https://github.com/morluto/rea/issues/5077) corrected the process-input size-policy catalogue entry. Closed [#3057](https://github.com/morluto/rea/issues/3057) and merged [PR #3108](https://github.com/morluto/rea/pull/3108) concern linked-directory mutation and preservation; this report does not claim those fixes regressed. Open/closed issue, issue-comment and open/closed/merged PR searches found no matching prepare-stage ENOTDIR wording report. No checkout files were changed and no GitHub write was made during verification. No fix, released-package replay or known-good historical version was tested. ## Triage assessment - Impact level: P3 - Assessment status: supported - Impact basis: A safely refused non-directory configuration gets a repeated path, a missing sentence boundary and generic permission advice. The native reason and usable directory guidance remain visible; no consequential behavior change was demonstrated. - Workaround status: available - Workaround basis: Preserving the blocking file and making the required directory allowed the identical command to return ready and install eight canonical files, without altering permissions or losing the preserved file.

bsakaguchi closed this issue

2026-10-31 00:51:03 +00:00

bsakaguchi referenced this issue

2026-10-31 01:13:35 +00:00

[PR #4589] feat(evidence): cluster artifacts from retained Evidence references and summaries #6325

bsakaguchi referenced this issue

2026-11-01 21:10:57 +00:00

[GH-ISSUE #3796] [Tracker] Open feature requests by area (October 2026) #1471

bsakaguchi referenced this issue

2026-11-01 21:11:07 +00:00

[GH-ISSUE #5654] [Tracker] Open PRs that can't merge yet: conflicts and red CI (October 2026) #3425