DSH: request_approval-инструмент — аппрув одним кликом владельца (#77) #78

Merged
aep merged 2 commits from feat/77-approve-tool into main 2026-10-25 10:20:04 +03:00
9 changed files with 736 additions and 191 deletions

View File

@ -1,11 +1,13 @@
/**
* Human-facing `/approve` command for DeepSeek Harness.
*
* `/approve <change-id> design|plan` is the only writer of
* `openspec/changes/<id>/approval.md` (D-1). The command checks the upstream
* artifacts, asks the owner through `userQuestions.ask` (D-12), and only then
* merges the new section into the existing file. A missing user-questions
* service refuses instead of writing, so no model tool can approve itself.
* `/approve <change-id> design|plan` is the manual front-end of the approval
* gate: it asks the owner through `userQuestions.ask` (D-12) and lets the
* shared `runApproval` (lib/approval.js) check the upstream artifacts and
* write `openspec/changes/<id>/approval.md` (D-1). The write itself lives in
* the shared module, which the model tool `request_approval` uses as well —
* both only write after the owner confirms. A missing user-questions service
* refuses instead of writing, so no model tool can approve itself.
*
* The repository root is resolved per invocation from the receiving agent's
* session (`workingDirectory.get(agent.session)`, T3.1) — the registration
@ -16,19 +18,8 @@
* `{ kind, text }` result shape.
*/
import { mkdirSync, readFileSync, readdirSync, statSync, writeFileSync } from 'node:fs'
import { dirname, join, relative, resolve } from 'node:path'
import {
APPROVAL_FILE,
CHANGE_DIR,
PLANS_DIR,
extractSpec,
hasDesignApproval,
hashPlan,
parseApproval,
resolveChangeFromSpec,
serializeApproval,
} from './lib/gates.js'
import { resolve } from 'node:path'
import { CHANGE_ID, runApproval } from './lib/approval.js'
const USAGE = [
'Usage: /approve <change-id> design|plan',
@ -36,27 +27,8 @@ const USAGE = [
' plan — record the owner approval of the plan whose **Spec:** is <change-id>',
].join('\n')
const DESIGN_ARTIFACTS = ['proposal.md', 'design.md', 'builder-check.md']
const APPROVE_LABEL = 'Approve'
const CANCEL_LABEL = 'Cancel'
/** Change ids are directory names: keep them one safe path segment. */
const CHANGE_ID = /^[A-Za-z0-9][A-Za-z0-9._-]*$/u
function isFile(path) {
try {
return statSync(path).isFile()
} catch {
return false
}
}
function readText(path) {
try {
return readFileSync(path, 'utf8')
} catch {
return null
}
}
/** Session working directory of the invoking agent; process.cwd() fallback. */
function currentDirectory(workingDirectory, agent) {
@ -99,148 +71,51 @@ function isApproved(answer) {
}
/**
* Ask the owner and return `{ ok: true }` or a refusal `{ ok: false, text }`.
* The detail shows exactly what is being approved; the answer is never
* inferred from the absence of an error.
* A refusal produced while asking the owner (missing service, failed dialog).
* The handler turns it into the same `Refused: ...` result as `runApproval`
* refusals; any other exception is reported as an `/approve failure`.
*/
async function askOwner(userQuestions, { changeId, kind, detail, agent, signal }) {
if (typeof userQuestions?.ask !== 'function') {
return { ok: false, text: 'userQuestions service unavailable; /approve requires it' }
}
class ConfirmationRefusal extends Error {}
const request = {
questions: [
{
id: `approve-${kind}`,
header: 'Approve',
question: `Approve ${changeId} ${kind}?`,
detail,
options: [
{
label: APPROVE_LABEL,
description: `Write the ${kind} approval to openspec/changes/${changeId}/approval.md.`,
},
{ label: CANCEL_LABEL, description: 'Write nothing.' },
],
},
],
}
if (agent !== undefined) request.agent = agent
if (signal !== undefined) request.signal = signal
/**
* The `ask` implementation of this front-end: one Approve/Cancel dialog whose
* detail shows exactly what is being approved. Returns `true` only for an
* explicit Approve answer; a missing service or a failed ask throws a
* `ConfirmationRefusal` carrying the reason, so nothing is written.
*/
function ownerAsker({ userQuestions, changeId, kind, agent, signal }) {
return async (questionText, detail) => {
if (typeof userQuestions?.ask !== 'function') {
throw new ConfirmationRefusal('userQuestions service unavailable; /approve requires it')
}
let answer
try {
answer = await userQuestions.ask(request)
} catch (error) {
return { ok: false, text: `userQuestions failed: ${String(error?.message ?? error)}` }
}
const request = {
questions: [
{
id: `approve-${kind}`,
header: 'Approve',
question: questionText,
detail,
options: [
{
label: APPROVE_LABEL,
description: `Write the ${kind} approval to openspec/changes/${changeId}/approval.md.`,
},
{ label: CANCEL_LABEL, description: 'Write nothing.' },
],
},
],
}
if (agent !== undefined) request.agent = agent
if (signal !== undefined) request.signal = signal
return isApproved(answer) ? { ok: true } : { ok: false, text: 'not confirmed' }
}
/** Merge one section into approval.md, leaving every other section as is. */
function writeApproval(root, changeId, mutate) {
const path = join(root, CHANGE_DIR, changeId, APPROVAL_FILE)
const next = mutate(parseApproval(readText(path)))
mkdirSync(dirname(path), { recursive: true })
writeFileSync(path, serializeApproval(next), 'utf8')
return path
}
/** Existing plans under docs/superpowers/plans/ whose **Spec:** is `<id>`. */
function plansForChange(root, changeId) {
const plansDir = join(root, PLANS_DIR)
let names
try {
names = readdirSync(plansDir)
} catch {
return []
}
const found = []
for (const name of [...names].sort()) {
if (!name.endsWith('.md')) continue
const path = join(plansDir, name)
if (!isFile(path)) continue
const content = readText(path)
if (content === null) continue
if (resolveChangeFromSpec(extractSpec(content)) === changeId) found.push(path)
}
return found
}
function approvalText(root, path) {
return relative(root, path).split('\\').join('/')
}
async function approveDesign({ root, changeId, userQuestions, agent, signal }) {
const changeDir = join(root, CHANGE_DIR, changeId)
const missing = DESIGN_ARTIFACTS.filter((name) => !isFile(join(changeDir, name)))
if (missing.length > 0) {
const listed = missing.map((name) => `openspec/changes/${changeId}/${name}`).join(', ')
return refusal(`design approval requires the change artifacts; missing: ${listed}.`)
}
const asked = await askOwner(userQuestions, {
changeId,
kind: 'design',
detail: `Change: openspec/changes/${changeId} (proposal.md, design.md, builder-check.md).`,
agent,
signal,
})
if (!asked.ok) return refusal(asked.text)
const at = new Date().toISOString()
const path = writeApproval(root, changeId, (approval) => ({
...approval,
design: { by: 'owner', at },
}))
return {
kind: 'success',
text: `Approved ${changeId} design at ${at}; wrote ${approvalText(root, path)}.`,
}
}
async function approvePlan({ root, changeId, userQuestions, agent, signal }) {
const approval = parseApproval(readText(join(root, CHANGE_DIR, changeId, APPROVAL_FILE)))
if (!hasDesignApproval(approval)) {
return refusal(`design not approved; run /approve ${changeId} design`)
}
const plans = plansForChange(root, changeId)
if (plans.length === 0) {
return refusal(
`no plan for ${changeId} found under ${PLANS_DIR.split('\\').join('/')}/ with **Spec:** openspec/changes/${changeId}.`,
)
}
if (plans.length > 1) {
const listed = plans.map((path) => approvalText(root, path)).join(', ')
return refusal(`several plans reference ${changeId}: ${listed}. Keep exactly one.`)
}
const [planPath] = plans
const sha256 = hashPlan(planPath)
if (sha256 === '') {
return refusal(`cannot read plan ${approvalText(root, planPath)}.`)
}
const asked = await askOwner(userQuestions, {
changeId,
kind: 'plan',
detail: `Plan: ${approvalText(root, planPath)} (sha256 ${sha256}).`,
agent,
signal,
})
if (!asked.ok) return refusal(asked.text)
const at = new Date().toISOString()
const path = writeApproval(root, changeId, (current) => ({
...current,
plan: { by: 'owner', at, sha256 },
}))
return {
kind: 'success',
text: `Approved ${changeId} plan at ${at} (sha256 ${sha256}); wrote ${approvalText(root, path)}.`,
let answer
try {
answer = await userQuestions.ask(request)
} catch (error) {
throw new ConfirmationRefusal(`userQuestions failed: ${String(error?.message ?? error)}`)
}
return isApproved(answer)
}
}
@ -267,11 +142,17 @@ export function registerApproveCommand({ commands, userQuestions, workingDirecto
const root = resolve(currentDirectory(workingDirectory, agent))
try {
if (kind === 'design') {
return await approveDesign({ root, changeId, userQuestions, agent, signal })
}
return await approvePlan({ root, changeId, userQuestions, agent, signal })
const result = await runApproval({
repoRoot: root,
changeId,
kind,
ask: ownerAsker({ userQuestions, changeId, kind, agent, signal }),
})
return result.approved
? { kind: 'success', text: result.message }
: refusal(result.message)
} catch (error) {
if (error instanceof ConfirmationRefusal) return refusal(error.message)
logger?.warn?.('[dsh-harness] /approve failed: %s', error?.message ?? error)
return refusal(`/approve failed: ${String(error?.message ?? error)}`)
}

View File

@ -25,6 +25,7 @@ import { fileURLToPath } from 'node:url'
import { registerRoleCommand } from './command-role.js'
import { registerCheckCommand } from './command-check.js'
import { registerApproveCommand } from './command-approve.js'
import { createApproveTool } from './tool-approve.js'
import { createGuard } from './lib/guards.js'
import {
checkApprovalWrite,
@ -322,6 +323,23 @@ export function apply(ctx, rawConfig = {}) {
})
}
// `request_approval` is the model-driven front-end of the same gate: the
// model initiates the approval, the owner only confirms in the dialog, and
// the marker is still written by lib/approval.js. `userQuestions` is
// resolved lazily, never injected — a missing service must refuse this tool
// instead of parking the whole bundle.
if (config.enabled && config.approvalGates && typeof ctx.tools?.register === 'function') {
ctx.tools.register(
createApproveTool({
getWorkingDirectory: () => ctx.workingDirectory,
getUserQuestions: () => (typeof ctx.get === 'function' ? (ctx.get('userQuestions') ?? null) : null),
logger: ctx.logger,
now: () => new Date(),
}),
)
ctx.logger?.info?.('[dsh-harness] request_approval tool registered')
}
ctx.logger?.info?.(
'[dsh-harness] ready: hooks=%s, bootstrap=%s, /role=%s',
config.guards ? hooksDir : 'off',

187
dsh/bundle/lib/approval.js Normal file
View File

@ -0,0 +1,187 @@
/**
* Shared approval logic of the two front-ends: the `/approve` command and the
* `request_approval` model tool.
*
* `runApproval` owns everything except the question itself: the upstream
* artifact checks (design needs proposal/design/builder-check, plan needs an
* approved design and exactly one plan whose **Spec:** names the change) and
* the section write (`parseApproval` -> merge -> `serializeApproval`). The
* confirmation is injected as `ask(questionText, detail) -> Promise<boolean>`,
* so no model-facing caller can write `approval.md` without the owner: the
* command asks through `userQuestions.ask`, the tool does the same on behalf
* of the model, and this module never writes without a `true` answer.
*
* `now` is injected for deterministic tests; it returns a Date, so the written
* `at` is a UTC ISO timestamp exactly as before the extraction.
*
* No DSH runtime is imported here (only Node builtins and `./gates.js`).
*/
import { mkdirSync, readFileSync, readdirSync, statSync, writeFileSync } from 'node:fs'
import { dirname, join, relative, resolve } from 'node:path'
import {
APPROVAL_FILE,
CHANGE_DIR,
PLANS_DIR,
extractSpec,
hasDesignApproval,
hashPlan,
parseApproval,
resolveChangeFromSpec,
serializeApproval,
} from './gates.js'
/** Design approval requires all three change artifacts, as before. */
export const DESIGN_ARTIFACTS = ['proposal.md', 'design.md', 'builder-check.md']
/** Change ids are directory names: keep them one safe path segment. */
export const CHANGE_ID = /^[A-Za-z0-9][A-Za-z0-9._-]*$/u
export const KINDS = ['design', 'plan']
function isFile(path) {
try {
return statSync(path).isFile()
} catch {
return false
}
}
function readText(path) {
try {
return readFileSync(path, 'utf8')
} catch {
return null
}
}
function refusal(text) {
return { approved: false, message: text }
}
/** Merge one section into approval.md, leaving every other section as is. */
function writeApproval(root, changeId, mutate) {
const path = join(root, CHANGE_DIR, changeId, APPROVAL_FILE)
const next = mutate(parseApproval(readText(path)))
mkdirSync(dirname(path), { recursive: true })
writeFileSync(path, serializeApproval(next), 'utf8')
return path
}
/** Existing plans under docs/superpowers/plans/ whose **Spec:** is `<id>`. */
function plansForChange(root, changeId) {
const plansDir = join(root, PLANS_DIR)
let names
try {
names = readdirSync(plansDir)
} catch {
return []
}
const found = []
for (const name of [...names].sort()) {
if (!name.endsWith('.md')) continue
const path = join(plansDir, name)
if (!isFile(path)) continue
const content = readText(path)
if (content === null) continue
if (resolveChangeFromSpec(extractSpec(content)) === changeId) found.push(path)
}
return found
}
function approvalText(root, path) {
return relative(root, path).split('\\').join('/')
}
/** Ask the injected confirmation; anything but `true` is not an approval. */
async function confirmed(ask, questionText, detail) {
const answer = await ask(questionText, detail)
return answer === true
}
async function approveDesign({ root, changeId, ask, now }) {
const changeDir = join(root, CHANGE_DIR, changeId)
const missing = DESIGN_ARTIFACTS.filter((name) => !isFile(join(changeDir, name)))
if (missing.length > 0) {
const listed = missing.map((name) => `openspec/changes/${changeId}/${name}`).join(', ')
return refusal(`design approval requires the change artifacts; missing: ${listed}.`)
}
const detail = `Change: openspec/changes/${changeId} (proposal.md, design.md, builder-check.md).`
if (!(await confirmed(ask, `Approve ${changeId} design?`, detail))) {
return refusal('not confirmed')
}
const at = new Date(now()).toISOString()
const path = writeApproval(root, changeId, (approval) => ({
...approval,
design: { by: 'owner', at },
}))
return {
approved: true,
message: `Approved ${changeId} design at ${at}; wrote ${approvalText(root, path)}.`,
}
}
async function approvePlan({ root, changeId, ask, now }) {
const approval = parseApproval(readText(join(root, CHANGE_DIR, changeId, APPROVAL_FILE)))
if (!hasDesignApproval(approval)) {
return refusal(`design not approved; run /approve ${changeId} design`)
}
const plans = plansForChange(root, changeId)
if (plans.length === 0) {
return refusal(
`no plan for ${changeId} found under ${PLANS_DIR.split('\\').join('/')}/ with **Spec:** openspec/changes/${changeId}.`,
)
}
if (plans.length > 1) {
const listed = plans.map((path) => approvalText(root, path)).join(', ')
return refusal(`several plans reference ${changeId}: ${listed}. Keep exactly one.`)
}
const [planPath] = plans
const sha256 = hashPlan(planPath)
if (sha256 === '') {
return refusal(`cannot read plan ${approvalText(root, planPath)}.`)
}
const detail = `Plan: ${approvalText(root, planPath)} (sha256 ${sha256}).`
if (!(await confirmed(ask, `Approve ${changeId} plan?`, detail))) {
return refusal('not confirmed')
}
const at = new Date(now()).toISOString()
const path = writeApproval(root, changeId, (current) => ({
...current,
plan: { by: 'owner', at, sha256 },
}))
return {
approved: true,
message: `Approved ${changeId} plan at ${at} (sha256 ${sha256}); wrote ${approvalText(root, path)}.`,
}
}
/**
* Approve `<changeId> <kind>` and write the marker.
*
* Returns `{ approved, message }`; every refusal is `approved: false` with a
* human-readable reason. The change id and kind are re-validated here (not
* only in the front-ends): the id becomes a path segment.
*/
export async function runApproval({ repoRoot, changeId, kind, ask, now = () => new Date() }) {
if (typeof changeId !== 'string' || !CHANGE_ID.test(changeId)) {
return refusal(`Invalid change id "${String(changeId)}".`)
}
if (!KINDS.includes(kind)) {
return refusal(`Unknown kind "${String(kind)}"; expected design or plan.`)
}
if (typeof ask !== 'function') {
return refusal('no confirmation function; the approval requires the owner')
}
const root = resolve(repoRoot)
if (kind === 'design') return approveDesign({ root, changeId, ask, now })
return approvePlan({ root, changeId, ask, now })
}

View File

@ -14,6 +14,7 @@
"command-role.js",
"command-check.js",
"command-approve.js",
"tool-approve.js",
"lib",
"dsh-tools.md",
"cordis.patch.yml",

144
dsh/bundle/tool-approve.js Normal file
View File

@ -0,0 +1,144 @@
/**
* `request_approval` — the model-driven front-end of the approval gate.
*
* The owner no longer has to type `/approve <change-id> design|plan`: the model
* calls this tool, the owner only confirms in the userQuestions dialog, and the
* approval marker is still written by bundle code (`lib/approval.js`), never by
* the model. The gates keep their invariant: `approval.md` is written only by
* the harness after an explicit owner confirmation.
*
* The definition is hand-built plain JSON — no `@deepseek-ai/*` imports (the
* bundle is installed from a local directory, so DSH internals are not
* resolvable). `parameters` is object-rooted raw JSON Schema; DSH performs no
* argument validation, so `execute` validates the args itself and always
* returns the lossless `{ approved, message }` shape.
*
* `userQuestions` is resolved lazily (injected as `getUserQuestions`) and never
* declared in `inject`: a missing service must refuse this tool, not park the
* whole bundle. Likewise `getWorkingDirectory` is a service factory, because
* the repo root depends on the receiving agent's session.
*/
import { resolve } from 'node:path'
import { CHANGE_ID, KINDS, runApproval } from './lib/approval.js'
const APPROVE_LABEL = 'Approve'
const CANCEL_LABEL = 'Cancel'
const USAGE =
"usage: request_approval { change_id: '<id>', kind: 'design' | 'plan' }"
/** Session working directory of the receiving agent; process.cwd() fallback. */
function repoRootOf(getWorkingDirectory, agent) {
try {
const workingDirectory = getWorkingDirectory?.()
const cwd = workingDirectory?.get?.(agent?.session)
if (typeof cwd === 'string' && cwd !== '') return resolve(cwd)
} catch {
// Fall through to the process directory.
}
return process.cwd()
}
function refusal(message) {
return { approved: false, message }
}
/**
* Build the tool definition.
*
* @param {{
* getWorkingDirectory?: () => { get(session: unknown): string } | undefined,
* getUserQuestions?: () => { ask(request: unknown): Promise<unknown> } | null,
* logger?: { warn?: Function },
* now?: () => Date,
* }} deps
*/
export function createApproveTool({ getWorkingDirectory, getUserQuestions, logger, now = () => new Date() } = {}) {
return {
name: 'request_approval',
description:
'Ask the owner to approve an OpenSpec change design or an implementation plan. The approval marker is written by the harness only after the owner confirms.',
parameters: {
type: 'object',
properties: {
change_id: {
type: 'string',
description: 'Change id under openspec/changes/<id>/',
},
kind: {
type: 'string',
enum: ['design', 'plan'],
description: 'What is being approved',
},
},
required: ['change_id', 'kind'],
additionalProperties: false,
},
output: {
schema: {
type: 'object',
properties: {
approved: { type: 'boolean' },
message: { type: 'string' },
},
required: ['approved', 'message'],
additionalProperties: false,
},
render(args, value) {
return [{ type: 'text', text: value.message }]
},
},
async execute(args, exec = {}) {
try {
const changeId = typeof args?.change_id === 'string' ? args.change_id.trim() : ''
const kind = args?.kind
if (changeId === '') {
return refusal(`Missing change_id. ${USAGE}`)
}
if (!CHANGE_ID.test(changeId)) {
return refusal(`Invalid change id "${changeId}". ${USAGE}`)
}
if (!KINDS.includes(kind)) {
return refusal(`Unknown kind "${String(kind)}". ${USAGE}`)
}
const userQuestions = getUserQuestions?.() ?? null
if (typeof userQuestions?.ask !== 'function') {
return refusal(
'userQuestions service unavailable; request_approval requires the owner dialog (use /approve).',
)
}
// The ask implementation of this front-end: mirrors the /approve
// dialog, but a `selected[0] === 'Approve'` answer is the only yes.
const ask = async (questionText) => {
const answer = await userQuestions.ask({
questions: [
{
id: 'approve',
question: questionText,
header: 'Approval gate',
options: [{ label: APPROVE_LABEL }, { label: CANCEL_LABEL }],
},
],
...(exec.agent ? { agent: exec.agent } : {}),
signal: exec.signal,
})
return answer?.answers?.[0]?.selected?.[0] === APPROVE_LABEL
}
return await runApproval({
repoRoot: repoRootOf(getWorkingDirectory, exec.agent),
changeId,
kind,
ask,
now,
})
} catch (error) {
logger?.warn?.('[dsh-harness] request_approval failed: %s', error?.message ?? error)
return refusal(String(error?.message ?? error))
}
},
}
}

View File

@ -5,6 +5,7 @@ import { join } from 'node:path'
import test from 'node:test'
import { registerApproveCommand } from '../bundle/command-approve.js'
import { hashPlan, parseApproval, serializeApproval } from '../bundle/lib/gates.js'
import { createApproveTool } from '../bundle/tool-approve.js'
import { cleanup, tempDir, write } from './helpers.mjs'
const PLAN_TEXT = [
@ -347,3 +348,218 @@ test('approve falls back to process.cwd() when the session cwd lookup throws (T3
cleanup(tmp)
}
})
// `request_approval` (bounded #77): the model drives the approval gate, the
// owner only confirms in the userQuestions dialog, and the marker is still
// written by bundle code through the same lib/approval.js as `/approve`.
const FIXED_ISO = '2026-10-25T12:00:00.000Z'
/**
* Build the tool with fakes: `getWorkingDirectory` is the DSH service factory
* (`workingDirectory.get(session)`), `getUserQuestions` resolves lazily, and
* `now` is pinned so the written `at` is exact.
*/
function setupTool(repo, { answer, missingService = false, at = FIXED_ISO } = {}) {
const calls = []
const getUserQuestions = missingService
? () => null
: () => ({
async ask(request) {
calls.push(request)
return answer
},
})
const tool = createApproveTool({
getWorkingDirectory: () => ({ get() { return repo } }),
getUserQuestions,
logger: { warn() {} },
now: () => new Date(at),
})
return { tool, calls, run: (args, exec = {}) => tool.execute(args, exec) }
}
test('request_approval design: owner confirms, section is written by the harness', async () => {
const { tmp, repo } = repoFixture()
try {
const harness = setupTool(repo, { answer: approveAnswer('design') })
assert.equal(harness.tool.name, 'request_approval')
assert.match(harness.tool.description, /written by the harness only after the owner confirms/u)
assert.deepEqual(harness.tool.parameters.required, ['change_id', 'kind'])
assert.deepEqual(harness.tool.parameters.properties.kind.enum, ['design', 'plan'])
assert.deepEqual(harness.tool.output.render({}, { approved: true, message: 'ok' }), [
{ type: 'text', text: 'ok' },
])
const result = await harness.run({ change_id: 'demo', kind: 'design' }, { agent: { session: 's1' } })
assert.equal(result.approved, true)
assert.equal(result.message, `Approved demo design at ${FIXED_ISO}; wrote openspec/changes/demo/approval.md.`)
const question = harness.calls[0].questions[0]
assert.equal(question.question, 'Approve demo design?')
assert.equal(question.header, 'Approval gate')
assert.deepEqual(question.options.map((option) => option.label), ['Approve', 'Cancel'])
assert.equal(harness.calls[0].agent.session, 's1', 'the receiving agent is forwarded')
const parsed = parseApproval(readFileSync(approvalPath(repo), 'utf8'))
assert.equal(parsed.design.by, 'owner')
assert.equal(parsed.design.at, FIXED_ISO)
} finally {
cleanup(tmp)
}
})
test('request_approval design: cancel writes nothing', async () => {
const { tmp, repo } = repoFixture()
try {
const harness = setupTool(repo, {
answer: { answers: [{ id: 'approve', selected: ['Cancel'], custom: undefined }] },
})
const result = await harness.run({ change_id: 'demo', kind: 'design' })
assert.equal(result.approved, false)
assert.match(result.message, /not confirmed/u)
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
} finally {
cleanup(tmp)
}
})
test('request_approval refuses when the userQuestions service is missing', async () => {
const { tmp, repo } = repoFixture()
try {
const harness = setupTool(repo, { missingService: true })
const result = await harness.run({ change_id: 'demo', kind: 'design' })
assert.equal(result.approved, false)
assert.match(result.message, /userQuestions service unavailable/u)
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
} finally {
cleanup(tmp)
}
})
test('request_approval refuses an unknown kind with usage', async () => {
const { tmp, repo } = repoFixture()
try {
const harness = setupTool(repo, { answer: approveAnswer('design') })
const result = await harness.run({ change_id: 'demo', kind: 'review' })
assert.equal(result.approved, false)
assert.match(result.message, /usage: request_approval/u)
assert.equal(harness.calls.length, 0, 'the owner is not asked')
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
} finally {
cleanup(tmp)
}
})
test('request_approval refuses an unsafe change id', async () => {
const { tmp, repo } = repoFixture()
try {
const harness = setupTool(repo, { answer: approveAnswer('design') })
const result = await harness.run({ change_id: '../x', kind: 'design' })
assert.equal(result.approved, false)
assert.match(result.message, /Invalid change id "\.\.\/x"/u)
assert.equal(harness.calls.length, 0, 'the owner is not asked')
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
} finally {
cleanup(tmp)
}
})
test('request_approval plan writes the sha256 of the plan', async () => {
const { tmp, repo, planPath } = repoFixture()
try {
const design = { by: 'aep', at: '2026-10-25T10:00:00+04:00' }
write(approvalPath(repo), serializeApproval({ design }))
const harness = setupTool(repo, { answer: approveAnswer('plan') })
const result = await harness.run({ change_id: 'demo', kind: 'plan' })
assert.equal(result.approved, true)
assert.equal(
result.message,
`Approved demo plan at ${FIXED_ISO} (sha256 ${shaOf(planPath)}); wrote openspec/changes/demo/approval.md.`,
)
const parsed = parseApproval(readFileSync(approvalPath(repo), 'utf8'))
assert.equal(parsed.plan.by, 'owner')
assert.equal(parsed.plan.at, FIXED_ISO)
assert.equal(parsed.plan.sha256, shaOf(planPath))
assert.deepEqual(parsed.design, design, 'the design section is untouched')
} finally {
cleanup(tmp)
}
})
test('request_approval design refuses without builder-check.md (prerequisites hold)', async () => {
const { tmp, repo } = repoFixture({ builderCheck: false })
try {
const harness = setupTool(repo, { answer: approveAnswer('design') })
const result = await harness.run({ change_id: 'demo', kind: 'design' })
assert.equal(result.approved, false)
assert.match(result.message, /builder-check\.md/u)
assert.equal(harness.calls.length, 0, 'the owner is not asked before the prerequisites pass')
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
} finally {
cleanup(tmp)
}
})
test('request_approval falls back to process.cwd() when the session cwd lookup throws', async () => {
const { tmp, repo } = repoFixture()
const previous = process.cwd()
try {
process.chdir(repo)
const tool = createApproveTool({
getWorkingDirectory: () => ({
get() {
throw new Error('cannot get property "session" without inject')
},
}),
getUserQuestions: () => ({ async ask() { return approveAnswer('design') } }),
logger: { warn() {} },
now: () => new Date(FIXED_ISO),
})
const result = await tool.execute({ change_id: 'demo', kind: 'design' }, { agent: { session: 's1' } })
assert.equal(result.approved, true)
assert.equal(existsSync(approvalPath(repo)), true, 'approval.md is written into process.cwd()')
} finally {
process.chdir(previous)
cleanup(tmp)
}
})
// Refactor guard (bounded #77): `/approve` still reports a failing
// userQuestions service as a refusal with the service's reason, and writes
// nothing. The shared runApproval only writes after a `true` answer.
test('approve reports a userQuestions failure and writes nothing', async () => {
const { tmp, repo } = repoFixture()
try {
const captured = {}
const commands = { register(definition) { Object.assign(captured, definition) } }
registerApproveCommand({
commands,
userQuestions: {
async ask() {
throw new Error('dialog closed')
},
},
workingDirectory: { get() { return repo } },
logger: { warn() {} },
})
const result = await captured.handler({ rawInput: 'demo design' })
assert.equal(result.kind, 'error')
assert.equal(result.text, 'Refused: userQuestions failed: dialog closed')
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
} finally {
cleanup(tmp)
}
})

View File

@ -3,6 +3,7 @@ import { existsSync, readFileSync, readlinkSync, lstatSync } from 'node:fs'
import { join } from 'node:path'
import { fileURLToPath } from 'node:url'
import test from 'node:test'
import { apply } from '../bundle/index.js'
const dshDir = fileURLToPath(new URL('..', import.meta.url))
const bundleDir = join(dshDir, 'bundle')
@ -15,6 +16,7 @@ test('package declares a DSH bundle patch and the runtime files', () => {
assert.equal(pkg.exports?.['.'], './index.js')
assert.ok(pkg.files.includes('index.js'))
assert.ok(pkg.files.includes('command-approve.js'))
assert.ok(pkg.files.includes('tool-approve.js'))
assert.ok(pkg.files.includes('lib'))
assert.ok(pkg.files.includes('hooks'))
assert.ok(existsSync(join(bundleDir, 'cordis.patch.yml')))
@ -58,9 +60,11 @@ test('runtime files have no external @deepseek-ai imports', () => {
'command-role.js',
'command-check.js',
'command-approve.js',
'tool-approve.js',
'lib/message.js',
'lib/guards.js',
'lib/gates.js',
'lib/approval.js',
'lib/bootstrap.js',
'lib/role.js',
'lib/legacy.js',
@ -70,3 +74,24 @@ test('runtime files have no external @deepseek-ai imports', () => {
assert.doesNotMatch(source, /from ['"]@deepseek-ai\//, `${file} must not import DSH internals`)
}
})
test('apply registers the request_approval tool when tools.register exists', () => {
const registered = []
const ctx = {
tools: {
guard() {},
register(definition) {
registered.push(definition)
},
},
workingDirectory: { get() { return process.cwd() } },
logger: { info() {}, warn() {}, error() {} },
get() { return { ask: async () => ({ answers: [] }) } },
}
apply(ctx)
assert.equal(registered.length, 1, 'one tool is registered')
assert.equal(registered[0].name, 'request_approval')
assert.deepEqual(registered[0].parameters.required, ['change_id', 'kind'])
})

View File

@ -23,7 +23,7 @@ ensure_bundle_hooks() {
ensure_bundle_hooks
for f in dsh/bundle/index.js dsh/bundle/command-role.js dsh/bundle/command-check.js dsh/bundle/command-approve.js dsh/bundle/lib/*.js dsh/tests/*.mjs; do
for f in dsh/bundle/index.js dsh/bundle/command-role.js dsh/bundle/command-check.js dsh/bundle/command-approve.js dsh/bundle/tool-approve.js dsh/bundle/lib/*.js dsh/tests/*.mjs; do
node --check "$f"
done

View File

@ -16,9 +16,11 @@
builder-check субагента, скопированный в change контроллером; он MUST быть записан
после builder-check и до передачи дизайна владельцу.
`openspec/changes/<id>/approval.md` SHALL быть маркером утверждения владельцем;
его единственный писатель — команда `/approve` (D-1), инструменты модели писать
его MUST NOT (гейт C ниже).
`openspec/changes/<id>/approval.md` SHALL быть маркером утверждения владельцем.
Писать его MUST только код бандла — команда `/approve` или инструмент
`request_approval` (D-1), и оба MUST писать только после подтверждения владельца.
Прямые записи модели (`write`, `edit`, `multi_edit`, `str_replace_editor`, `bash`)
MUST NOT создавать или менять этот файл (гейт C ниже, без изменений).
Формат `approval.md` — yaml с секциями `design:` (поля `by`, `at`) и `plan:`
(поля `by`, `at`, `sha256`); `sha256` — hex-хэш sha256 содержимого файла плана на
@ -42,11 +44,14 @@ builder-check субагента, скопированный в change конт
### Requirement: Команда /approve
`/approve <change-id> design|plan` SHALL быть единственным писателем
`approval.md` и MUST регистрироваться при `enabled` и `approvalGates: true`,
независимо от флагов `roleCommand` и `checkCommand`: иначе гейты остаются без
команды-разблокировки (T3.2). `change-id` MUST быть одним безопасным сегментом
пути; небезопасный id или неизвестный вид — usage-отказ без записи.
`/approve <change-id> design|plan` SHALL быть ручным писателем `approval.md` —
одним из двух писателей бандла, наравне с инструментом `request_approval`
(следующее требование); оба пишут только после подтверждения владельца, прямые
записи модели — MUST NOT (гейт C). Команда MUST регистрироваться при `enabled` и
`approvalGates: true`, независимо от флагов `roleCommand` и `checkCommand`: иначе
гейты остаются без команды-разблокировки (T3.2). `change-id` MUST быть одним
безопасным сегментом пути; небезопасный id или неизвестный вид — usage-отказ без
записи.
Вид `design` SHALL требовать существования `proposal.md`, `design.md` и
`builder-check.md` в `openspec/changes/<id>/`; при отсутствии файла команда
@ -93,6 +98,71 @@ MUST отказать с перечислением отсутствующих
- **THEN** команда отказывает с `userQuestions service unavailable; /approve
requires it` и `approval.md` не пишется
### Requirement: Инструмент request_approval
Инструмент модели `request_approval` SHALL быть вторым писателем `approval.md`:
модель инициирует утверждение, владелец только подтверждает в диалоге, а маркер
пишет код бандла (`lib/approval.js`) — с теми же проверками предпосылок и той же
записью, что у `/approve`. Инструмент MUST регистрироваться при `enabled` и
`approvalGates: true`, если в контексте доступен `ctx.tools.register`;
`userQuestions` MUST резолвиться лениво (не через `inject`), чтобы отсутствие
сервиса не парковало весь бандл.
Параметры инструмента — `change_id` (строка) и `kind` (`design` | `plan`).
Валидация MUST быть собственной, в инструменте: пустой или небезопасный
`change_id` и неизвестный `kind` — usage-отказ без записи и без вопроса
владельцу; проверки предпосылок и отказы те же, что у `/approve` (design —
proposal/design/builder-check, plan — утверждённый дизайн, ровно один план,
sha256), и любой отказ MUST NOT писать `approval.md`. Подтверждением SHALL
считаться только ответ `answers[0].selected[0] === 'Approve'`; Cancel,
отсутствие ответа и ошибка диалога — отказ `not confirmed` без записи. При
отсутствии сервиса `userQuestions` инструмент MUST отказать (`userQuestions
service unavailable`), а не писать.
#### Scenario: Модель просит утвердить дизайн
- **WHEN** модель вызывает `request_approval` с `{change_id: <id>, kind: design}`,
предпосылки выполнены, а владелец выбрал Approve
- **THEN** в `approval.md` пишется design-секция (`by: owner`, `at` в ISO 8601
UTC), инструмент возвращает `{approved: true, message}`
#### Scenario: Модель просит утвердить план
- **WHEN** модель вызывает `request_approval` с `{change_id: <id>, kind: plan}`,
дизайн утверждён, план единственный, владелец выбрал Approve
- **THEN** plan-секция содержит `by: owner`, `at` в ISO 8601 и sha256 текущего
содержимого плана
#### Scenario: Владелец отменил утверждение
- **WHEN** в диалоге `request_approval` владелец выбрал Cancel
- **THEN** инструмент возвращает `{approved: false, message: 'not confirmed'}`, а
`approval.md` не меняется
#### Scenario: Некорректные аргументы request_approval
- **WHEN** модель передала неизвестный `kind` или небезопасный `change_id`
(например, `../x`)
- **THEN** инструмент возвращает usage-отказ без записи, владелец не спрашивается
#### Scenario: Отсутствует builder-check при утверждении дизайна моделью
- **WHEN** `kind: design`, а `builder-check.md` в change нет
- **THEN** инструмент отказывает с перечислением отсутствующего файла, владелец не
спрашивается, `approval.md` не создаётся
#### Scenario: request_approval plan без утверждённого дизайна
- **WHEN** `kind: plan`, а design-секции нет
- **THEN** инструмент отказывает `design not approved; run /approve <id> design`,
владелец не спрашивается, `approval.md` не меняется
#### Scenario: request_approval без сервиса вопросов
- **WHEN** `userQuestions` отсутствует
- **THEN** инструмент возвращает `{approved: false}` с `userQuestions service
unavailable`, владелец не спрашивается, `approval.md` не пишется
### Requirement: Гейт A — план до утверждения дизайна
Для write/edit/multi_edit/`str_replace_editor` с целевым путём под
@ -231,9 +301,12 @@ fail closed. Фрагмент `edit` проверяется по своему `n
### Requirement: Гейт C и bash-обход
`approval.md` MUST писаться только командой `/approve`: любой write/edit с целью,
чей basename — `approval.md`, и любая bash-цель с таким именем MUST получать
отказ `approval.md is written only by /approve` (D-1).
`approval.md` MUST писаться только кодом бандла — командой `/approve` или
инструментом `request_approval`: любой write/edit с целью, чей basename —
`approval.md`, и любая bash-цель с таким именем MUST получать отказ `approval.md
is written only by /approve` (D-1). Deny-текст не меняется: он адресован прямым
записям модели (`write`/`edit`/`bash`), а `request_approval` пишет маркер кодом
бандла и под этот запрет не попадает.
Bash-обход MUST быть закрыт (D-8): если команда содержит грубую подстроку
`docs/superpowers/plans`, `approval.md` или `.superpowers/sdd`, её файловые цели