feat(dsh): /approve — единственный писатель approval.md с подтверждением владельца (#59)
This commit is contained in:
Andrey Epifancev
parent
8744575271
commit
c255a24b94
264
dsh/bundle/command-approve.js
Normal file
264
dsh/bundle/command-approve.js
Normal file
@ -0,0 +1,264 @@
|
||||
/**
|
||||
* 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.
|
||||
*
|
||||
* Mirrors `command-role.js`: object-form `commands.register`, the same
|
||||
* `{ 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'
|
||||
|
||||
const USAGE = [
|
||||
'Usage: /approve <change-id> design|plan',
|
||||
' design — record the owner approval of the change design',
|
||||
' 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
|
||||
}
|
||||
}
|
||||
|
||||
function refusal(text) {
|
||||
return { kind: 'error', text: `Refused: ${text}` }
|
||||
}
|
||||
|
||||
function usage(text) {
|
||||
return { kind: 'error', text: `${text}\n${USAGE}` }
|
||||
}
|
||||
|
||||
/**
|
||||
* Read the answer of `userQuestions.ask`.
|
||||
*
|
||||
* The DSH UserQuestionService resolves to `{ answers: [{ id, selected }] }`;
|
||||
* the plan/brief contract also allows `{ approved }` / `{ confirmed }` (and a
|
||||
* bare `true`) so the command stays testable without the runtime. Anything
|
||||
* else — a skipped question, a cancel, a timeout, null — is not an approval.
|
||||
*/
|
||||
function isApproved(answer) {
|
||||
if (answer === true) return true
|
||||
if (answer === null || typeof answer !== 'object') return false
|
||||
|
||||
const flag = answer.approved ?? answer.confirmed
|
||||
if (flag !== undefined) return Boolean(flag)
|
||||
|
||||
const items = Array.isArray(answer.answers) ? answer.answers : []
|
||||
return items.some(
|
||||
(item) => item !== null && typeof item === 'object' && Array.isArray(item.selected) && item.selected.includes(APPROVE_LABEL),
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
async function askOwner(userQuestions, { changeId, kind, detail, agent, signal }) {
|
||||
if (typeof userQuestions?.ask !== 'function') {
|
||||
return { ok: false, text: 'userQuestions service unavailable; /approve requires it' }
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
let answer
|
||||
try {
|
||||
answer = await userQuestions.ask(request)
|
||||
} catch (error) {
|
||||
return { ok: false, text: `userQuestions failed: ${String(error?.message ?? error)}` }
|
||||
}
|
||||
|
||||
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)}.`,
|
||||
}
|
||||
}
|
||||
|
||||
export function registerApproveCommand({ commands, userQuestions, repoRoot, logger }) {
|
||||
commands.register({
|
||||
name: 'approve',
|
||||
description: 'Утвердить design или plan change (единственный писатель approval.md)',
|
||||
input: { hint: '<change-id> design|plan' },
|
||||
handler: async ({ agent, rawInput, signal } = {}) => {
|
||||
const parts =
|
||||
typeof rawInput === 'string' ? rawInput.trim().split(/\s+/u).filter((part) => part !== '') : []
|
||||
|
||||
if (parts.length !== 2) {
|
||||
return usage('Missing arguments: expected <change-id> design|plan.')
|
||||
}
|
||||
|
||||
const [changeId, kind] = parts
|
||||
if (!CHANGE_ID.test(changeId)) {
|
||||
return usage(`Invalid change id "${changeId}".`)
|
||||
}
|
||||
if (kind !== 'design' && kind !== 'plan') {
|
||||
return usage(`Unknown kind "${kind}".`)
|
||||
}
|
||||
|
||||
const root = resolve(repoRoot || process.cwd())
|
||||
try {
|
||||
if (kind === 'design') {
|
||||
return await approveDesign({ root, changeId, userQuestions, agent, signal })
|
||||
}
|
||||
return await approvePlan({ root, changeId, userQuestions, agent, signal })
|
||||
} catch (error) {
|
||||
logger?.warn?.('[dsh-harness] /approve failed: %s', error?.message ?? error)
|
||||
return refusal(`/approve failed: ${String(error?.message ?? error)}`)
|
||||
}
|
||||
},
|
||||
})
|
||||
}
|
||||
223
dsh/tests/approve.test.mjs
Normal file
223
dsh/tests/approve.test.mjs
Normal file
@ -0,0 +1,223 @@
|
||||
import assert from 'node:assert/strict'
|
||||
import { createHash } from 'node:crypto'
|
||||
import { existsSync, readFileSync } from 'node:fs'
|
||||
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 { cleanup, tempDir, write } from './helpers.mjs'
|
||||
|
||||
const PLAN_TEXT = [
|
||||
'# Plan X',
|
||||
'',
|
||||
'**Spec:** `openspec/changes/demo/`',
|
||||
'',
|
||||
'## Task 1',
|
||||
'',
|
||||
'Do the work.',
|
||||
'',
|
||||
].join('\n')
|
||||
|
||||
const ISO = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/u
|
||||
const APPROVE = 'Approve'
|
||||
|
||||
/** A throwaway repository with the design artifacts and one demo plan. */
|
||||
function repoFixture({ builderCheck = true } = {}) {
|
||||
const tmp = tempDir('dsh-approve-')
|
||||
const repo = join(tmp, 'repo')
|
||||
write(join(repo, 'openspec', 'changes', 'demo', 'proposal.md'), '# Proposal\n')
|
||||
write(join(repo, 'openspec', 'changes', 'demo', 'design.md'), '# Design\n')
|
||||
if (builderCheck) write(join(repo, 'openspec', 'changes', 'demo', 'builder-check.md'), '# Check\n')
|
||||
const planPath = join(repo, 'docs', 'superpowers', 'plans', 'plan-x.md')
|
||||
write(planPath, PLAN_TEXT)
|
||||
return { tmp, repo, planPath }
|
||||
}
|
||||
|
||||
function approvalPath(repo) {
|
||||
return join(repo, 'openspec', 'changes', 'demo', 'approval.md')
|
||||
}
|
||||
|
||||
/** An ask() answer batch shaped like the DSH UserQuestionService result. */
|
||||
function approveAnswer(kind = 'design') {
|
||||
return { answers: [{ id: `approve-${kind}`, selected: [APPROVE], custom: undefined }] }
|
||||
}
|
||||
|
||||
/**
|
||||
* Wire the command against a fake `commands.register` that captures the
|
||||
* registered definition, and a scripted `userQuestions.ask`.
|
||||
*/
|
||||
function setup(repo, { answer, missingService = false } = {}) {
|
||||
const captured = {}
|
||||
const commands = {
|
||||
register(definition) {
|
||||
Object.assign(captured, definition)
|
||||
},
|
||||
}
|
||||
const calls = []
|
||||
let userQuestions
|
||||
if (!missingService) {
|
||||
userQuestions = {
|
||||
async ask(request) {
|
||||
calls.push(request)
|
||||
return answer
|
||||
},
|
||||
}
|
||||
}
|
||||
registerApproveCommand({
|
||||
commands,
|
||||
userQuestions,
|
||||
repoRoot: repo,
|
||||
logger: { warn() {} },
|
||||
})
|
||||
return { captured, calls, run: (rawInput) => captured.handler({ rawInput }) }
|
||||
}
|
||||
|
||||
function shaOf(path) {
|
||||
return createHash('sha256').update(readFileSync(path)).digest('hex')
|
||||
}
|
||||
|
||||
test('approve design refuses without builder-check.md', async () => {
|
||||
const { tmp, repo } = repoFixture({ builderCheck: false })
|
||||
try {
|
||||
const harness = setup(repo, { answer: approveAnswer('design') })
|
||||
const result = await harness.run('demo design')
|
||||
|
||||
assert.match(result.text, /builder-check\.md/u)
|
||||
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
|
||||
assert.equal(harness.calls.length, 0, 'the owner is not asked when artifacts are missing')
|
||||
} finally {
|
||||
cleanup(tmp)
|
||||
}
|
||||
})
|
||||
|
||||
test('approve design writes design section (by owner, at ISO)', async () => {
|
||||
const { tmp, repo } = repoFixture()
|
||||
try {
|
||||
const existingPlan = { by: 'aep', at: '2026-10-25T11:30:00+04:00', sha256: 'a'.repeat(64) }
|
||||
write(approvalPath(repo), serializeApproval({ plan: existingPlan }))
|
||||
|
||||
const harness = setup(repo, { answer: approveAnswer('design') })
|
||||
const result = await harness.run('demo design')
|
||||
|
||||
assert.equal(result.kind, 'success')
|
||||
const request = harness.calls[0].questions[0]
|
||||
assert.equal(request.question, 'Approve demo design?')
|
||||
|
||||
const parsed = parseApproval(readFileSync(approvalPath(repo), 'utf8'))
|
||||
assert.equal(parsed.design.by, 'owner')
|
||||
assert.match(parsed.design.at, ISO)
|
||||
assert.equal(Number.isNaN(Date.parse(parsed.design.at)), false)
|
||||
assert.deepEqual(parsed.plan, existingPlan, 'the pre-existing plan section is untouched')
|
||||
} finally {
|
||||
cleanup(tmp)
|
||||
}
|
||||
})
|
||||
|
||||
test('approve design refuses without confirmation', async () => {
|
||||
const { tmp, repo } = repoFixture()
|
||||
try {
|
||||
const harness = setup(repo, {
|
||||
answer: { answers: [{ id: 'approve-design', selected: ['Cancel'] }] },
|
||||
})
|
||||
const result = await harness.run('demo design')
|
||||
|
||||
assert.match(result.text, /not confirmed/u)
|
||||
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
|
||||
} finally {
|
||||
cleanup(tmp)
|
||||
}
|
||||
})
|
||||
|
||||
test('approve plan refuses without design section', async () => {
|
||||
const { tmp, repo } = repoFixture()
|
||||
try {
|
||||
const harness = setup(repo, { answer: approveAnswer('plan') })
|
||||
const result = await harness.run('demo plan')
|
||||
|
||||
assert.match(result.text, /design not approved/u)
|
||||
assert.match(result.text, /\/approve demo design/u)
|
||||
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
|
||||
assert.equal(harness.calls.length, 0, 'the owner is not asked before the design gate passes')
|
||||
} finally {
|
||||
cleanup(tmp)
|
||||
}
|
||||
})
|
||||
|
||||
test('approve plan refuses when two plans reference the change', async () => {
|
||||
const { tmp, repo } = repoFixture()
|
||||
try {
|
||||
write(join(repo, 'docs', 'superpowers', 'plans', 'plan-y.md'), PLAN_TEXT)
|
||||
write(
|
||||
approvalPath(repo),
|
||||
serializeApproval({ design: { by: 'aep', at: '2026-10-25T10:00:00+04:00' } }),
|
||||
)
|
||||
const before = readFileSync(approvalPath(repo), 'utf8')
|
||||
|
||||
const harness = setup(repo, { answer: approveAnswer('plan') })
|
||||
const result = await harness.run('demo plan')
|
||||
|
||||
assert.match(result.text, /plan-x\.md/u)
|
||||
assert.match(result.text, /plan-y\.md/u)
|
||||
assert.equal(readFileSync(approvalPath(repo), 'utf8'), before, 'approval.md is untouched')
|
||||
assert.equal(harness.calls.length, 0, 'the owner is not asked when the plan is ambiguous')
|
||||
} finally {
|
||||
cleanup(tmp)
|
||||
}
|
||||
})
|
||||
|
||||
test('approve plan writes plan section with 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 = setup(repo, { answer: approveAnswer('plan') })
|
||||
const result = await harness.run('demo plan')
|
||||
|
||||
assert.equal(result.kind, 'success')
|
||||
const request = harness.calls[0].questions[0]
|
||||
assert.equal(request.question, 'Approve demo plan?')
|
||||
|
||||
const parsed = parseApproval(readFileSync(approvalPath(repo), 'utf8'))
|
||||
assert.equal(parsed.plan.by, 'owner')
|
||||
assert.match(parsed.plan.at, ISO)
|
||||
assert.equal(parsed.plan.sha256, shaOf(planPath))
|
||||
assert.equal(parsed.plan.sha256, hashPlan(planPath))
|
||||
assert.deepEqual(parsed.design, design, 'the design section is untouched')
|
||||
} finally {
|
||||
cleanup(tmp)
|
||||
}
|
||||
})
|
||||
|
||||
test('approve refuses when userQuestions is missing', async () => {
|
||||
const { tmp, repo } = repoFixture()
|
||||
try {
|
||||
const harness = setup(repo, { missingService: true })
|
||||
const result = await harness.run('demo design')
|
||||
|
||||
assert.match(result.text, /userQuestions service unavailable/u)
|
||||
assert.match(result.text, /\/approve requires it/u)
|
||||
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
|
||||
} finally {
|
||||
cleanup(tmp)
|
||||
}
|
||||
})
|
||||
|
||||
test('approve with unknown kind returns usage', async () => {
|
||||
const { tmp, repo } = repoFixture()
|
||||
try {
|
||||
const harness = setup(repo, { answer: approveAnswer() })
|
||||
|
||||
const unknown = await harness.run('demo review')
|
||||
assert.match(unknown.text, /Usage: \/approve <change-id> design\|plan/u)
|
||||
assert.equal(unknown.kind, 'error')
|
||||
|
||||
const missing = await harness.run('demo')
|
||||
assert.match(missing.text, /Usage: \/approve <change-id> design\|plan/u)
|
||||
|
||||
assert.equal(harness.calls.length, 0, 'nothing is asked')
|
||||
assert.equal(existsSync(approvalPath(repo)), false, 'no approval.md is written')
|
||||
} finally {
|
||||
cleanup(tmp)
|
||||
}
|
||||
})
|
||||
Loading…
x
Reference in New Issue
Block a user