From 183f8d89568688583fa9fc737188bbbbf7dfc4c0 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Sun, 25 Oct 2026 12:19:53 +0400 Subject: [PATCH] =?UTF-8?q?fix(dsh):=20#62=20=E2=80=94=20PATH-=D0=B8=D0=B7?= =?UTF-8?q?=D0=BE=D0=BB=D1=8F=D1=86=D0=B8=D1=8F=20=D1=82=D0=B5=D1=81=D1=82?= =?UTF-8?q?=D0=B0,=20WARN=20DSH=5FHOME,=20TDD-=D0=BF=D0=BE=D1=80=D1=8F?= =?UTF-8?q?=D0=B4=D0=BE=D0=BA=20=D0=B2=20sdd=20(#62)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- dsh/install.sh | 23 ++++++++++++++++++ dsh/skills/sdd/SKILL.md | 10 ++++---- dsh/tests/install.test.mjs | 39 ++++++++++++++++++++++++++++++ dsh/tests/openspec.test.mjs | 47 +++++++++++++++++++++++++++---------- 4 files changed, 103 insertions(+), 16 deletions(-) diff --git a/dsh/install.sh b/dsh/install.sh index 5ce3d81..8e7af16 100755 --- a/dsh/install.sh +++ b/dsh/install.sh @@ -21,6 +21,14 @@ set -euo pipefail ROOT="$(cd "$(dirname "$0")/.." && pwd)" BUNDLE="$ROOT/dsh/bundle" +# O-2: remember whether DSH_HOME came from the environment. Isolating HOME +# alone is not enough: an explicit DSH_HOME still writes outside the isolated +# tree (a temp HOME with the real DSH_HOME, typically a test or a detached +# shell), so the install steps warn about that mixed state. +DSH_HOME_FROM_ENV=0 +if [ -n "${DSH_HOME:-}" ]; then + DSH_HOME_FROM_ENV=1 +fi DSH_HOME="${DSH_HOME:-$HOME/.dsh}" DSH_SUPERPOWERS_REF="${DSH_SUPERPOWERS_REF:-v7.0.1}" DSH_SUPERPOWERS_REPO="${DSH_SUPERPOWERS_REPO:-https://github.com/obra/superpowers}" @@ -498,7 +506,21 @@ copy_skills_from() { echo "copied $count superpowers skills into $DSH_HOME/skills (source: $label)" } +# O-2: an explicit DSH_HOME that already exists while $HOME is another tree is +# the mixed case: the install writes into the existing DSH_HOME instead of the +# isolated one. Warn once per run and continue — the target is valid, only the +# HOME/DSH_HOME pair is not isolated together. +warn_mixed_dsh_home() { + if [ "$DSH_HOME_FROM_ENV" != 1 ]; then return 0; fi + if [ "$DSH_HOME" = "$HOME/.dsh" ]; then return 0; fi + if [ ! -d "$DSH_HOME" ]; then return 0; fi + if [ "${DSH_HOME_WARNED:-0}" = 1 ]; then return 0; fi + DSH_HOME_WARNED=1 + echo "WARN: DSH_HOME=$DSH_HOME points to an existing directory with a non-standard HOME; both HOME and DSH_HOME must be isolated together" >&2 +} + install_skills() { + warn_mixed_dsh_home mkdir -p "$DSH_HOME/skills" # Explicit source: sync it directly, no network, no pin enforcement. @@ -576,6 +598,7 @@ install_local_skills() { } install_roles() { + warn_mixed_dsh_home mkdir -p "$DSH_HOME/roles" local src="$ROOT/roles" local count=0 diff --git a/dsh/skills/sdd/SKILL.md b/dsh/skills/sdd/SKILL.md index e9fefab..c86da11 100644 --- a/dsh/skills/sdd/SKILL.md +++ b/dsh/skills/sdd/SKILL.md @@ -9,7 +9,7 @@ description: Use when executing an implementation plan in DeepSeek Harness (DSH) только в DSH-сессиях: у Claude Code синхронный `Task`, у DSH — асинхронный `subagent`. Это **не паритет** с Claude-SDD и не имитация: протокол построен на том, что DSH умеет (уведомление о завершении, `send_message`, ledger), а не -на том, чего у него нет (возврат результата из вызова инструмента). +на том, чего у него нет. ## Когда применять @@ -62,6 +62,9 @@ overlay ребёнок наследует модель сессии. 6. Замеченное расхождение с брифом или спекой — это `DONE_WITH_CONCERNS`, даже если задача формально выполнена: ревью его проверит, контроллер — решит. +**Порядок TDD.** Шаг «напиши тест и убедись, что он падает (RED)» обязателен +ДО кода; реализация без RED — `DONE_WITH_CONCERNS`, ревью это проверяет. + ## Ожидание без блокировки `subagent` возвращает id и сразу завершается; результата в значении @@ -135,9 +138,8 @@ ledger лежит в git-ignored scratch (после `git clean -fdx` восст фиксирует это наблюдением `O-n`. `model` нельзя передать один, «унаследовав» провайдера: DSH требует `provider` и `model` вместе и на односторонний маршрут отвечает «child LLM provider and model must be supplied together». -Эта ошибка означает, что в вызове не хватает `provider` (в прогоне #41 так -случилось дважды), а не сбой валидации: проверь аргументы и повтори вызов, -передав оба поля. +Эта ошибка означает, что в вызове не хватает `provider`, а не сбой валидации: +проверь аргументы и повтори вызов, передав оба поля. ## Отличие от Claude-SDD diff --git a/dsh/tests/install.test.mjs b/dsh/tests/install.test.mjs index 373817f..ad4e4e8 100644 --- a/dsh/tests/install.test.mjs +++ b/dsh/tests/install.test.mjs @@ -54,6 +54,9 @@ test('install.sh copies skills and roles instead of linking into ~/.claude', () assert.match(sddText, /name: sdd/) assert.match(sddText, /\/approve/, 'the sdd skill must check the plan approval marker before dispatch') assert.match(sddText, /approval\.md/, 'the sdd skill must name the approval marker file') + // T3.4: TDD order is part of the dispatch protocol: RED before the code. + assert.match(sddText, /падает \(RED\)/, 'the sdd skill must require a failing test (RED) before the code') + assert.match(sddText, /реализация без RED — `DONE_WITH_CONCERNS`/, 'a missing RED must degrade the report status') for (const role of ['dev', 'admin', 'analyst', 'qa']) { assert.ok(existsSync(join(dshHome, 'roles', `${role}.md`)), `missing ${role}.md`) @@ -638,6 +641,42 @@ test('--global installs DSH role paths and explains the instruction format', () } }) +test('--skills and --roles warn when DSH_HOME exists while HOME is isolated (O-2)', () => { + const work = tempDir() + try { + const home = join(work, 'home') + // An existing DSH_HOME outside the isolated HOME is the mixed state: the + // install would write outside the sandbox. The warning must not abort. + const sharedDsh = join(work, 'shared-dsh') + mkdirSync(home, { recursive: true }) + mkdirSync(sharedDsh, { recursive: true }) + const src = fixtureSuperpowers(home, null, join(home, 'src', 'superpowers')) + const warn = `WARN: DSH_HOME=${sharedDsh} points to an existing directory with a non-standard HOME; both HOME and DSH_HOME must be isolated together` + + const skills = runInstaller(home, ['--skills', '--superpowers-from', src], { DSH_HOME: sharedDsh }) + const skillsOut = skills.stdout + skills.stderr + assert.equal(skills.status, 0, skillsOut) + assert.ok(skillsOut.includes(warn), skillsOut) + assert.ok( + existsSync(join(sharedDsh, 'skills', 'using-superpowers', 'SKILL.md')), + 'the install still runs after the warning', + ) + + const roles = runInstaller(home, ['--roles'], { DSH_HOME: sharedDsh }) + const rolesOut = roles.stdout + roles.stderr + assert.equal(roles.status, 0, rolesOut) + assert.ok(rolesOut.includes(warn), rolesOut) + + // Isolated together ($DSH_HOME=$HOME/.dsh): no warning. + const normal = runInstaller(home, ['--skills', '--superpowers-from', src]) + const normalOut = normal.stdout + normal.stderr + assert.equal(normal.status, 0, normalOut) + assert.doesNotMatch(normalOut, /WARN: DSH_HOME=/) + } finally { + cleanup(work) + } +}) + test('a legacy cache without using-superpowers is not a source', () => { const home = tempDir() try { diff --git a/dsh/tests/openspec.test.mjs b/dsh/tests/openspec.test.mjs index ad71c8b..eb940cd 100644 --- a/dsh/tests/openspec.test.mjs +++ b/dsh/tests/openspec.test.mjs @@ -19,6 +19,32 @@ const NAMES = [ 'openspec-update-change', ] +// #62 T2.1/M-2: a negative test must not inherit the host PATH — on Linux a +// global npm prefix puts openspec into /usr/bin, so a fixed system PATH is not +// openspec-free. The child gets a guard dir holding symlinks to the binaries +// the script needs and nothing else. `bash` is addressed by absolute path +// because libuv resolves the command with the child's restricted PATH. +function which(name) { + const out = spawnSync('bash', ['-c', `command -v ${name}`], { encoding: 'utf8' }) + return out.stdout.trim() +} + +const BASH = which('bash') || '/bin/bash' + +function isolatedPath(home) { + const bin = join(home, 'guard-bin') + mkdirSync(bin) + // The toolchain --check verifies (python3, git, node) plus the POSIX tools + // install.sh calls on this path (dirname for ROOT, head for versions, the + // ls/sort/tail legacy lookup). openspec is deliberately absent. + for (const name of ['python3', 'git', 'node', 'dirname', 'head', 'ls', 'sort', 'tail']) { + const target = which(name) + assert.ok(target, `cannot resolve ${name}: the isolated PATH would be incomplete`) + symlinkSync(target, join(bin, name)) + } + return bin +} + test('repository ships six agents skills for DSH', () => { assert.equal(readFileSync(join(agentsSkills, '.openspec-target'), 'utf8').trim(), 'agents') for (const name of NAMES) { @@ -49,17 +75,16 @@ test('install.sh --check reports the openspec CLI and the agents skills', () => test('install.sh --check warns but does not fail without the openspec CLI', () => { const home = tempDir() try { - const bin = join(home, 'bin') - mkdirSync(bin, { recursive: true }) - symlinkSync(process.execPath, join(bin, 'node')) - // openspec ставится в /opt/homebrew/bin; здесь только системные каталоги — CLI не найден + // #62 T2.1: the guard dir is the whole PATH, so a host openspec CLI (e.g. + // /usr/bin on Linux) can never leak into this negative test. + const bin = isolatedPath(home) const env = { ...process.env, HOME: home, DSH_HOME: join(home, '.dsh'), - PATH: `${bin}:/usr/bin:/bin:/usr/sbin:/sbin`, + PATH: bin, } - const run = spawnSync('bash', [script, '--check'], { env, encoding: 'utf8' }) + const run = spawnSync(BASH, [script, '--check'], { env, encoding: 'utf8' }) assert.equal(run.status, 0, run.stderr) assert.match(run.stdout + run.stderr, /WARN: openspec not found/) } finally { @@ -134,17 +159,15 @@ test('--openspec fails without the openspec CLI', () => { try { const project = join(home, 'project') mkdirSync(join(project, 'openspec'), { recursive: true }) - const bin = join(home, 'bin') - mkdirSync(bin, { recursive: true }) - symlinkSync(process.execPath, join(bin, 'node')) - // openspec ставится в /opt/homebrew/bin; здесь только системные каталоги — CLI не найден + // #62 T2.1: same guard-dir isolation as the --check negative test. + const bin = isolatedPath(home) const env = { ...process.env, HOME: home, DSH_HOME: join(home, '.dsh'), - PATH: `${bin}:/usr/bin:/bin:/usr/sbin:/sbin`, + PATH: bin, } - const run = spawnSync('bash', [script, '--openspec'], { cwd: project, env, encoding: 'utf8' }) + const run = spawnSync(BASH, [script, '--openspec'], { cwd: project, env, encoding: 'utf8' }) assert.notEqual(run.status, 0) assert.match(run.stdout + run.stderr, /FAIL: openspec CLI not found/) } finally {