From 6bbf8b301fd9346dbe1110f2d327b299bc1cd8c7 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:01:56 -0300 Subject: [PATCH 1/7] test: require each test contract to name its gate runner Every contract in scripts/config/test-contracts.json still listed node-24 after #88 moved 16 of them to Bun, because the registry check only verified that toolchains is a sorted string array (#96). test-contracts.mjs now derives the runner of each suite from how the gate runs it (node()/nodeOk() or bun()/bunOk() in scripts/ci.mjs, `bun test` for the unit files, and the descriptor call sites for plugin suites) and requires the matching token: node- from the workflow's node-version and bun- from packageManager. The registry now lists bun-1 for the Bun suites and bun-1 plus node-24 for repo-plan-cli, whose fake gh runs on Bun. Red baseline: before the registry change the check failed for exactly the 16 Bun suites and passed for repo-plan-cli. Refs #96 --- scripts/config/test-contracts.json | 34 +++---- scripts/tests/test-contracts.mjs | 140 ++++++++++++++++++++++++++--- 2 files changed, 143 insertions(+), 31 deletions(-) diff --git a/scripts/config/test-contracts.json b/scripts/config/test-contracts.json index e89f09e3..b3a085f0 100644 --- a/scripts/config/test-contracts.json +++ b/scripts/config/test-contracts.json @@ -15,7 +15,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -34,7 +34,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -53,7 +53,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -72,7 +72,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -91,7 +91,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -110,7 +110,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1", "node-24"], "release_role": "gate", "replaces": [], "skips": [] @@ -129,7 +129,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -148,7 +148,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -167,7 +167,7 @@ "expected_min": 7 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -186,7 +186,7 @@ "expected_min": 4 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -205,7 +205,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -224,7 +224,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -243,7 +243,7 @@ "expected_min": 11 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -262,7 +262,7 @@ "expected_min": 1 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -281,7 +281,7 @@ "expected_min": 8 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -300,7 +300,7 @@ "expected_min": 4 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] @@ -319,7 +319,7 @@ "expected_min": 9 }, "platforms": ["linux", "macos"], - "toolchains": ["node-24"], + "toolchains": ["bun-1"], "release_role": "gate", "replaces": [], "skips": [] diff --git a/scripts/tests/test-contracts.mjs b/scripts/tests/test-contracts.mjs index 4958ebda..94dee445 100755 --- a/scripts/tests/test-contracts.mjs +++ b/scripts/tests/test-contracts.mjs @@ -3,6 +3,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; +import { parseDocument } from 'yaml'; import { PLUGINS } from '../lib/plugins.mjs'; const HERE = path.dirname(fileURLToPath(import.meta.url)); @@ -140,8 +141,12 @@ function slug(value) { .toLowerCase(); } -function descriptor({ id, suite, kind, selector, selectedCount, executed }) { - return { id, suite, kind, selector, selectedCount, executed }; +function descriptor({ id, suite, kind, selector, selectedCount, executed, runners, runnerSource }) { + return { id, suite, kind, selector, selectedCount, executed, runners, runnerSource }; +} + +function distinctRunners(invocations = []) { + return [...new Set(invocations.map(({ runner }) => runner))].sort(); } function unitDescriptors(packageJson) { @@ -155,6 +160,7 @@ function unitDescriptors(packageJson) { assert.equal(typeof unitCommand, 'string', 'package.json must declare scripts.test:unit'); // Only Bun's test runner registers the files' test() calls; `bun ` would run one file as a plain script. const executesUnitGlob = /^bun test\b/u.test(unitCommand) && unitCommand.includes('./scripts/tests/unit/*.test.mjs'); + const unitRunner = unitCommand.trim().split(/\s+/u)[0]; return unitFiles.map((file) => { const suite = `scripts/tests/unit/${file}`; const source = fs.readFileSync(path.join(REPO, suite), 'utf8'); @@ -166,6 +172,8 @@ function unitDescriptors(packageJson) { selector: suite, selectedCount, executed: executesUnitGlob, + runners: [unitRunner], + runnerSource: 'package.json scripts.test:unit', }); }); } @@ -206,23 +214,59 @@ function executableCodePositions(source) { return positions; } +// A plugin descriptor suite reaches the runner through gatePlugin's parameter, sometimes by way of a loop variable or +// an argument array, so follow the nearest preceding binding back to the descriptor field it reads. +function pluginDescriptorField(ciSource, codePositions, expression, position, pluginParameter) { + const [root, field] = expression.split('.'); + if (root === pluginParameter) return field ?? null; + const bindingPattern = new RegExp( + String.raw`\b(?:const\s+${root}\s*=\s*\[\s*|for\s*\(\s*const\s+${root}\s+of\s+)(?[A-Za-z_][\w.]*)`, + 'gu', + ); + let binding = null; + for (const candidate of ciSource.slice(0, position).matchAll(bindingPattern)) { + if (codePositions[candidate.index] === 1) binding = candidate; + } + if (binding === null) return null; + return pluginDescriptorField(ciSource, codePositions, binding.groups.source, binding.index, pluginParameter); +} + function gateInvocationArguments(ciSource) { const bySuite = new Map(); + const byPluginField = new Map(); const codePositions = executableCodePositions(ciSource); - const invocationPattern = /\b(?:node|bun)(?:Ok)?\(\[\s*['"](?[^'"]+\.mjs)['"](?[^[]*?)\]\)/gu; + const pluginParameter = /\bfunction gatePlugin\((?[A-Za-z_]\w*)\)/u.exec(ciSource)?.groups.name; + assert.ok(pluginParameter, 'scripts/ci.mjs must declare gatePlugin() to gate plugin descriptor suites'); + const invocationPattern = + /\b(?node|bun)(?:Ok)?\((?:\[\s*(?:['"](?[^'"]+\.mjs)['"]|(?[A-Za-z_][\w.]*))(?[^[]*?)\]|(?[A-Za-z_]\w*))\)/gu; for (const match of ciSource.matchAll(invocationPattern)) { if (codePositions[match.index] !== 1) continue; + const { runner, suite, expression, variable } = match.groups; + if (suite === undefined) { + const field = pluginDescriptorField( + ciSource, + codePositions, + expression ?? variable, + match.index, + pluginParameter, + ); + if (field === null) continue; + const invocations = byPluginField.get(field) ?? []; + invocations.push({ runner, args: [] }); + byPluginField.set(field, invocations); + continue; + } const args = [...match.groups.args.matchAll(/,\s*(['"])([^'"]*)\1/gu)].map((argument) => argument[2]); - const invocations = bySuite.get(match.groups.suite) ?? []; - invocations.push(args); - bySuite.set(match.groups.suite, invocations); + const invocations = bySuite.get(suite) ?? []; + invocations.push({ runner, args }); + bySuite.set(suite, invocations); } - return bySuite; + return { bySuite, byPluginField }; } function gateSelection(suite, invocations = []) { - const distinct = [...new Map(invocations.map((args) => [JSON.stringify(args), args])).values()].sort((left, right) => - JSON.stringify(left).localeCompare(JSON.stringify(right)), + const distinct = [...new Map(invocations.map(({ args }) => [JSON.stringify(args), args])).values()].sort( + (left, right) => JSON.stringify(left).localeCompare(JSON.stringify(right)), ); if (distinct.length === 0) return { selector: suite, selectedCount: 1 }; if (distinct.length === 1) return { selector: [suite, ...distinct[0]].join(' '), selectedCount: 1 }; @@ -239,8 +283,7 @@ function gateSelection(suite, invocations = []) { return { selector, selectedCount: distinct.length }; } -function repoSuiteDescriptors(ciSource) { - const invocations = gateInvocationArguments(ciSource); +function repoSuiteDescriptors({ bySuite: invocations }) { // The registry certifies commands the gate executes; a path in prose or diagnostics only names a suite. const paths = [...invocations.keys()].filter( (suite) => @@ -258,6 +301,8 @@ function repoSuiteDescriptors(ciSource) { selector: selectors.get(suite) ?? selection.selector, selectedCount: selection.selectedCount, executed: true, + runners: distinctRunners(invocations.get(suite)), + runnerSource: 'scripts/ci.mjs', }); }); } @@ -272,7 +317,12 @@ function commandSelector(suite, args = []) { return [suite, ...args].join(' '); } -function pluginDescriptors() { +function pluginDescriptors({ byPluginField }) { + const runnersOf = (field) => { + const runners = distinctRunners(byPluginField.get(field)); + assert.ok(runners.length > 0, `scripts/ci.mjs gatePlugin() never runs the plugin descriptor field ${field}`); + return runners; + }; const discovered = []; for (const plugin of PLUGINS) { if (plugin.selftest) { @@ -284,6 +334,8 @@ function pluginDescriptors() { selector: plugin.selftest, selectedCount: 1, executed: true, + runners: runnersOf('selftest'), + runnerSource: 'scripts/ci.mjs', }), ); } @@ -296,6 +348,8 @@ function pluginDescriptors() { selector: plugin.distributionContract, selectedCount: 1, executed: true, + runners: runnersOf('distributionContract'), + runnerSource: 'scripts/ci.mjs', }), ); } @@ -308,6 +362,8 @@ function pluginDescriptors() { selector: commandSelector(check.path, check.args), selectedCount: 1, executed: true, + runners: runnersOf('sourceChecks'), + runnerSource: 'scripts/ci.mjs', }), ); } @@ -320,6 +376,8 @@ function pluginDescriptors() { selector: suite, selectedCount: 1, executed: true, + runners: runnersOf('releaseContracts'), + runnerSource: 'scripts/ci.mjs', }), ); } @@ -337,15 +395,52 @@ function assertSetEqual(actual, expected, label) { ); } +// The registry's toolchain tokens must follow the gate: CI pins Node in the workflow and Bun in package.json, so a +// suite whose runner changes, or a runtime major bump, makes every affected contract name the new runner. +function nodeToolchain() { + const workflowPath = '.github/workflows/ci.yml'; + const document = parseDocument(fs.readFileSync(path.join(REPO, workflowPath), 'utf8')); + assert.deepEqual(document.errors, [], `${workflowPath} must parse as YAML`); + const versions = new Set(); + for (const [jobId, job] of Object.entries(document.toJS()?.jobs ?? {})) { + for (const step of job?.steps ?? []) { + if (typeof step?.uses !== 'string' || !step.uses.startsWith('actions/setup-node@')) continue; + const version = step.with?.['node-version']; + assert.ok( + version !== undefined && version !== null && String(version).length > 0, + `${workflowPath} job ${jobId} setup-node step must set node-version`, + ); + versions.add(String(version)); + } + } + assert.ok(versions.size > 0, `${workflowPath} must set node-version in a setup-node step`); + assert.equal( + versions.size, + 1, + `${workflowPath} setup-node steps disagree on node-version: ${[...versions].sort().join(', ')}`, + ); + const [version] = versions; + const major = /^v?(?\d+)(?:\.|$)/u.exec(version)?.groups.major; + assert.ok(major, `${workflowPath} node-version must name a numeric major version: ${version}`); + return `node-${major}`; +} + +function bunToolchain(packageJson) { + const major = /^bun@(?\d+)\./u.exec(packageJson.packageManager ?? '')?.groups.major; + assert.ok(major, `package.json packageManager must pin bun@..: ${packageJson.packageManager}`); + return `bun-${major}`; +} + const registry = readJson('scripts/config/test-contracts.json'); const packageJson = readJson('package.json'); const ciSource = fs.readFileSync(path.join(REPO, 'scripts/ci.mjs'), 'utf8'); validateRegistry(registry); +const gateInvocations = gateInvocationArguments(ciSource); const discoveredDescriptors = [ ...unitDescriptors(packageJson), - ...repoSuiteDescriptors(ciSource), - ...pluginDescriptors(), + ...repoSuiteDescriptors(gateInvocations), + ...pluginDescriptors(gateInvocations), ]; const descriptorById = new Map(); for (const discovered of discoveredDescriptors) { @@ -359,6 +454,11 @@ assertSetEqual(registered, discovered, 'discovered and registered sets differ'); const selected = new Set(); const executed = new Set(); +const toolchainByRunner = new Map([ + ['node', nodeToolchain()], + ['bun', bunToolchain(packageJson)], +]); +const staleToolchains = []; for (const contract of registry.contracts) { const evidence = descriptorById.get(contract.id); assert.ok(evidence, `${contract.id} has no discovered selection evidence`); @@ -371,11 +471,23 @@ for (const contract of registry.contracts) { `${contract.id} selected ${evidence.selectedCount}, below expected minimum ${contract.selection.expected_min}`, ); assert.ok(fs.existsSync(path.join(REPO, contract.owner.suite)), `${contract.id} owner suite does not exist`); + assert.ok(evidence.runners.length > 0, `${contract.id} has no discovered runner`); + for (const runner of evidence.runners) { + const toolchain = toolchainByRunner.get(runner); + assert.ok(toolchain, `${contract.id} runs with ${runner}, which has no known toolchain token`); + if (!contract.toolchains.includes(toolchain)) { + staleToolchains.push( + `${contract.id}.toolchains must list ${toolchain}: ${evidence.runnerSource} runs ${evidence.suite} with ${runner}`, + ); + } + } selected.add(contract.id); if (evidence.executed) executed.add(contract.id); } assert.ok(selected.size > 0, 'selected set must not be empty'); for (const id of selected) assert.ok(registered.has(id), `selected contract is not registered: ${id}`); assertSetEqual(executed, selected, 'executed and selected sets differ'); +// Collected rather than asserted per row so one run names every stale contract. +assert.deepEqual(staleToolchains, [], `contract toolchains omit the gate runner:\n${staleToolchains.join('\n')}`); console.log(`PASS test_contracts contracts=${registered.size} discovered=${discovered.size} selected=${selected.size}`); From 347f0a58dc6ded16b3ff5eeda4ccacf67b6e828c Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:25:49 -0300 Subject: [PATCH 2/7] fix(plan-lifecycle): wait for GitHub's close before a partial-merge reopen DocksDocks/docks#90: the manager reopened a plan issue one second after the close that a partial pull request's merge triggered. GitHub left the issue CLOSED with reason REOPENED, a state it refuses to change (also in the web page), so plan.mjs read the plan as unreadable for good. The contract now runs `reopen --pr ` after a partial merge. The helper reads the merged pull request; when it does not close the plan, it writes nothing. Otherwise it polls until the plan reads CLOSED with a close event from that pull request, waits a full 10 s (the wait that avoided the state in #90), re-reads owner, state, and close event, and reopens in one GraphQL request that also posts the contract comment and claims an unassigned plan. A plan already open again, a close by someone else, a change during the wait, and the stuck state each stop with their own message, and `show` advises on the stuck state. The contract records the fallback: keep step results in a comment and ask GitHub Support. The spec cases came first (red baseline: 13 failed on the unknown command, 48 passed) and stayed unchanged; a test-only --import hook skips the waits and logs them, so the 10 s settle is asserted without slowing the suite. Closes #90 Refs #96 --- .../skills/productivity/plan-manager/SKILL.md | 8 +- .../plan-manager/references/plan-contract.md | 26 +- .../plan-manager/scripts/plan.mjs | 173 ++++++++++- scripts/tests/fixtures/gh-stub/gh | 176 ++++++++++- scripts/tests/plan-cli/harness.mjs | 71 ++++- scripts/tests/plan-cli/reopen.test.mjs | 277 ++++++++++++++++++ scripts/tests/plan-cli/skip-waits.mjs | 11 + 7 files changed, 721 insertions(+), 21 deletions(-) create mode 100644 scripts/tests/plan-cli/reopen.test.mjs create mode 100644 scripts/tests/plan-cli/skip-waits.mjs diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md index 86ec04cc..022f25a1 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-10-23" - content_hash: "37cfc75fba999189e56e877a1cb3ad02be4e26c5e3f289bd83e1d3e3f08a427b" + content_hash: "9033244710f826b6c8c48909fe7e5ba40173fb7688b81a17cfbf2e1c0f8b716d" --- # Plan Manager @@ -145,8 +145,8 @@ After code review passes, follow the contract's Landing and archive proof: commit and push the reviewed bytes, open or update the pull request, wait for checks, ask afresh for `Merge now` or `Leave pull request open`, and merge only on `Merge now`. The `Merge now` option states the teardown, as the contract's -Checks and merge says. After a partial merge, reopen the issue as the contract -states. Archive only after the landing merge, when every step is terminal. +Checks and merge says. After a partial merge, run `reopen --pr ` as the +contract states. Archive only after the landing merge, when every step is terminal. After archive, run the contract's Teardown. Without `Merge now`, run Teardown's exit without a merge. Report Kept resources in both cases. @@ -167,7 +167,7 @@ BAD: Merge because checks passed earlier. GOOD: Verify the reviewed head and obtain a fresh Merge now answer. BAD: Merge a partial pull request and trust `Refs #` to keep the issue open. -GOOD: Say that the merge closes the issue, merge on `Merge now`, reopen it, and confirm `show` reads ongoing. +GOOD: Say that the merge closes the issue, merge on `Merge now`, run `reopen --pr `, and confirm `show` reads ongoing. BAD: Run `git branch -D` after `git branch -d` refuses. GOOD: Keep the branch and list it under Kept resources. diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md index 98fd84b3..f2a1842b 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md @@ -210,6 +210,7 @@ prefixed with `#`. The header strip is `# · · · <url>`. | `list [--status <s>]` | `<status>\t#<n>\t<title>`; open first, then closed, each in ascending number order; prints advice when it reaches 500 plans | | `archive <n>` | `plan #<n> finished (closed by <pr-url>)`; removes stale phase labels | | `retire <n> --reason <t>` | `plan #<n> retired`; closes as not planned and removes phase labels | +| `reopen <n> --pr <pr>` | `plan #<n> reopened: <status>`; `plan #<n> is open: <status>` when the plan is already open again; `pull request #<pr> does not close plan #<n>; nothing to reopen` when that pull request does not close the plan | With `show --body`, only the body goes to stdout; both metadata lines go to stderr. Render a body for the user only when they name the plan and ask to see it. @@ -248,7 +249,10 @@ directory, a Windows drive, or a network share. Its advice is `body contains an absolute machine path: <path>`, once per distinct path. URL routes and portable system paths, such as `/usr/bin/env` or a bare `/tmp`, get no advice. `show` also advises when a plan is closed as completed while a -step is not terminal, and names the closer. `list` advises when it reaches 500 +step is not terminal, and names the closer. For an issue that GitHub left +closed with reason `REOPENED`, `show` advises +`GitHub left this issue closed with reason REOPENED and refuses every state change; ask GitHub Support to repair it, and record step results in an issue comment until then`. +The status of that issue stays `unreadable`. `list` advises when it reaches 500 plans. Advice is not approval. Refusals exit 1. These stderr strings are exact, with angle brackets replaced: @@ -280,6 +284,12 @@ retire requires a single-line --reason cannot retire a <status> plan blocked status requires --reason as single-line text reserved label namespace: <label> +pull request #<pr> is not merged +GitHub has not closed plan #<n> for pull request #<pr> yet; run reopen again once it has +plan #<n> was closed by <closer> after pull request #<pr> merged; reopen it by hand only if that close was a mistake +plan #<n> changed during the settle wait; run reopen again +GitHub left plan #<n> closed with reason REOPENED and refuses every state change; ask GitHub Support to repair it +plan issue state differs after reopen ``` GitHub call failures exit 1 and print one of these messages on stderr. `edit`, @@ -335,10 +345,20 @@ For a partial pull request: `gh pr view <pr> --repo <owner/repo> --json closingIssuesReferences`. If the plan issue is listed, say in the merge question that the merge closes the issue and that you will reopen it. -2. After the merge, run `show <n>`. If the status is `finished`, run - `gh issue reopen <n> --repo <owner/repo> --comment "Reopened: partial pull request #<pr> closed this plan through its linked branch. Work continues."`. +2. After the merge, run `reopen <n> --pr <pr>`. The helper waits until GitHub + records the close by that pull request, then waits 10 s more. It reopens the + plan with the comment + `Reopened: partial pull request #<pr> closed this plan through its linked branch. Work continues.`, + claims an unassigned plan in the same write, and prints the status. Never + reopen the issue by hand right after the merge. 3. Run `show <n>` again. Continue only when the status is `ongoing`. +GitHub can leave an issue closed with reason `REOPENED`. It then refuses every +state change on the issue, also in the web page (DocksDocks/docks#90). Do not +retry the reopen. Record the remaining step results in one issue comment, +report the plan as stuck, and ask the repository owner to contact GitHub +Support. After GitHub repairs the issue, continue normally. + ### Checks and merge Wait up to five minutes for checks to appear. Then wait for every check to diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs b/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs index c171bb75..483ca88f 100755 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs @@ -28,6 +28,12 @@ const PLAN_LIST_ARGS = ',labels:["plan"],states:[OPEN,CLOSED],orderBy:{field:CRE const PLAN_LIST_FIELDS = 'number title state stateReason labels(first:100){nodes{name}}'; const GH_MAX_BUFFER_MIB = 256; const PLAN_LIST_LIMIT = 500; +// `reopen` reads the plan up to REOPEN_READS times until GitHub records the close by the merged pull request. Between +// reads it pauses for the next entry of REOPEN_PAUSES_MS, and the last entry repeats: about 60 s in all. Once the +// close shows, it waits REOPEN_SETTLE_MS before it reads the plan again and writes (see sleep). +const REOPEN_PAUSES_MS = [250, 500, 1000, 2000]; +const REOPEN_READS = 34; +const REOPEN_SETTLE_MS = 10_000; // A machine path points into a user or root home, a mount, a named temporary or runtime directory, // a Windows drive, or a network share. URL routes, slash prose, and portable system paths such as // /usr/bin/env or a bare /tmp do not match (#71). The match runs to the next space or delimiter. @@ -92,7 +98,7 @@ function parseJson(output, command) { const ISSUE_LABEL_FIELDS = 'name id'; const ASSIGNEE_FIELDS = '... on Actor{login}'; const LABELED_EVENT_FIELDS = '... on LabeledEvent{label{name}}'; -const CLOSED_EVENT_FIELDS = `... on ClosedEvent{closer{__typename ... on PullRequest{${PULL_REQUEST_FIELDS} body} ... on Commit{oid associatedPullRequests(first:100){${PULL_REQUEST_CONNECTION}}}}}`; +const CLOSED_EVENT_FIELDS = `... on ClosedEvent{createdAt closer{__typename ... on PullRequest{${PULL_REQUEST_FIELDS} body} ... on Commit{oid associatedPullRequests(first:100){${PULL_REQUEST_CONNECTION}}}}}`; function issueConnectionSelection(field, nodeFields, extraArgs = '', after = false) { return `${field}(first:100${after ? ',after:$after' : ''}${extraArgs}){nodes{${nodeFields}} ${PAGE_INFO}}`; @@ -267,10 +273,14 @@ function readSnapshot( fail('gh api graphql returned malformed latest closure'); if (readsClosure) { if (!Array.isArray(issue.latestClosure?.nodes)) fail('gh api graphql returned malformed latest closure'); - const closer = issue.latestClosure.nodes.at(-1)?.closer; + const latest = issue.latestClosure.nodes.at(-1); + const closer = latest?.closer; if ( issue.latestClosure.nodes.length > 1 || - (issue.latestClosure.nodes.length && (closer === undefined || (closer !== null && !closer.__typename))) + (issue.latestClosure.nodes.length && + (closer === undefined || + (closer !== null && !closer.__typename) || + !Number.isFinite(Date.parse(latest.createdAt)))) ) fail('gh api graphql returned malformed latest closure'); if (closer?.__typename === 'PullRequest') pullRequestNodes([closer]); @@ -279,12 +289,13 @@ function readSnapshot( if (archive) issue.associatedPullRequests = pullRequestNodes(associatedNodes(closer)); } issue.closer = closer; + issue.closedEventAt = latest?.createdAt; } return { issue, login, viewerId, labelIds }; } -function parseIssueNumber(value) { +function parseIssueNumber(value, kind = 'plan issue') { const match = /^#?([1-9]\d*)$/.exec(String(value ?? '')); - if (!match) fail(`invalid plan issue: ${value ?? '(missing)'}`); + if (!match) fail(`invalid ${kind}: ${value ?? '(missing)'}`); return Number(match[1]); } function workStarted(issue, recordStatus) { @@ -442,6 +453,19 @@ export function statusFromIssue(issue) { return 'unreadable'; } } +// A reopen that races GitHub's close can leave an issue closed with reason REOPENED, and GitHub then refuses every +// state change on it, also in the web page (#90). The derived status stays `unreadable`. +const STUCK_ADVICE = + 'GitHub left this issue closed with reason REOPENED and refuses every state change; ask GitHub Support to repair it, and record step results in an issue comment until then'; +function stuckClosed(issue) { + return String(issue.state).toUpperCase() === 'CLOSED' && String(issue.stateReason).toUpperCase() === 'REOPENED'; +} +function refuseStuck(issue) { + if (stuckClosed(issue)) + fail( + `GitHub left plan #${issue.number} closed with reason REOPENED and refuses every state change; ask GitHub Support to repair it`, + ); +} function labelsToRemove(issue, target) { const names = (issue.labels ?? []).map((label) => (typeof label === 'string' ? label : label.name)); return names.filter((label) => token(label).startsWith('plan:') && (!target || token(label) !== `plan:${target}`)); @@ -836,6 +860,7 @@ function showPlan(args) { `plan is closed as completed while step ${unfinished.id} is ${unfinished.status}; closed by ${describeCloser(issue.closer)}; reopen the issue if work remains`, ] : []; + if (stuckClosed(issue)) extra.push(STUCK_ADVICE); printAdvice(issue.body, status, extra); } } @@ -1052,6 +1077,141 @@ function archivePlan(args, retired = false) { : `plan #${issue.number} finished (closed by ${closer.url ?? `https://github.com/${repository.nameWithOwner}/pull/${closer.number}`})`, ); } +// Block the process for `ms` milliseconds. A reopen 1 s after GitHub closed a plan for a merged pull request raced +// that close and left the issue stuck closed with reason REOPENED (#90). So `reopen` pauses between its reads until +// it sees the close, then waits REOPEN_SETTLE_MS for the close to settle before it writes. +function sleep(ms) { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +} +// Pull request `number`: whether and when it merged, and whether it closes plan `planNumber` of this repository. +function readPullRequest(number, planNumber) { + const references = (after) => + issueConnectionSelection('closingIssuesReferences', 'number repository{nameWithOwner}', '', after); + const query = `query($owner:String!,$name:String!,$pr:Int!){repository(owner:$owner,name:$name){nameWithOwner pullRequest(number:$pr){merged mergedAt ${references(false)}}}}`; + const repo = graphql(query, { pr: number }).repository; + const pullRequest = repo?.pullRequest; + if ( + typeof repo?.nameWithOwner !== 'string' || + typeof pullRequest?.merged !== 'boolean' || + (pullRequest.merged && !Number.isFinite(Date.parse(pullRequest.mergedAt))) + ) + fail('gh api graphql returned malformed pull request'); + const pageQuery = `query($owner:String!,$name:String!,$pr:Int!,$after:String!){repository(owner:$owner,name:$name){pullRequest(number:$pr){${references(true)}}}}`; + const closed = connectionNodes( + pullRequest.closingIssuesReferences, + 'closing issue references', + (after) => graphql(pageQuery, { pr: number, after }).repository?.pullRequest?.closingIssuesReferences, + ); + if (closed.some((issue) => !Number.isInteger(issue.number) || typeof issue.repository?.nameWithOwner !== 'string')) + fail('gh api graphql returned malformed closing issue references'); + return { + number, + merged: pullRequest.merged, + mergedAt: pullRequest.mergedAt, + closesPlan: closed.some( + (issue) => issue.number === planNumber && issue.repository.nameWithOwner === repo.nameWithOwner, + ), + }; +} +// Whether the latest close of the plan came from the merged pull request, directly or through a commit associated +// with it; false while that close predates the merge or the plan has none. Refuses a stuck plan and a close by +// another closer after the merge. +function closedByPullRequest(issue, pullRequest) { + refuseStuck(issue); + if (issue.closedEventAt === undefined || Date.parse(issue.closedEventAt) < Date.parse(pullRequest.mergedAt)) + return false; + const { closer } = issue; + const isPullRequest = (candidate) => + candidate.number === pullRequest.number && candidate.repository.nameWithOwner === repository.nameWithOwner; + const fromPullRequest = + closer?.__typename === 'PullRequest' + ? isPullRequest(closer) + : closer?.__typename === 'Commit' && pullRequestNodes(associatedNodes(closer)).some(isPullRequest); + if (!fromPullRequest) + fail( + `plan #${issue.number} was closed by ${describeCloser(closer)} after pull request #${pullRequest.number} merged; reopen it by hand only if that close was a mistake`, + ); + return true; +} +// One write request reopens the plan, posts the contract comment, and claims an unassigned plan. Its last field +// returns the stored state, which shows whether GitHub left the plan stuck. +function writeReopen(issue, pr) { + const { id } = preWriteIssue(issue.number); + const claims = issue.claimantId !== undefined; + const stored = '{... on Issue{state stateReason}}'; + const fields = [ + 'reopenIssue(input:{issueId:$issue}){clientMutationId}', + `addComment(input:{subjectId:$issue,body:$body}){${claims ? 'clientMutationId' : `subject${stored}`}}`, + ]; + if (claims) + fields.push(`addAssigneesToAssignable(input:{assignableId:$issue,assigneeIds:$assignees}){assignable${stored}}`); + let data; + try { + data = graphqlData([ + '-f', + `query=mutation($issue:ID!,$body:String!${claims ? ',$assignees:[ID!]!' : ''}){${fields.join(' ')}}`, + '-f', + `issue=${id}`, + '-f', + `body=Reopened: partial pull request #${pr} closed this plan through its linked branch. Work continues.`, + ...(claims ? ['-f', `assignees[]=${issue.claimantId}`] : []), + ]); + } catch (error) { + // GitHub answers the reopen of a stuck plan with "Could not reopen the issue." (#90); the request is not retried. + refuseStuck(readSnapshot(issue.number).issue); + throw error; + } + const payload = claims ? data.addAssigneesToAssignable?.assignable : data.addComment?.subject; + if (typeof payload?.state !== 'string' || (payload.stateReason !== null && typeof payload.stateReason !== 'string')) + fail('gh api graphql returned malformed stored plan issue'); + const reopened = { ...issue, state: payload.state, stateReason: payload.stateReason }; + refuseStuck(reopened); + if (String(reopened.state).toUpperCase() === 'CLOSED') fail('plan issue state differs after reopen'); + console.log(`plan #${issue.number} reopened: ${statusFromIssue(reopened)}`); +} +// The reopen after a partial pull request closed the plan through its linked branch (#90). +function reopenPlan(args) { + const [value, ...flags] = args, + pr = parseOptions(flags, new Set(['--pr']))['--pr']; + if (!value || !pr) fail('reopen requires an issue and --pr'); + const number = parseIssueNumber(value), + pullRequest = readPullRequest(parseIssueNumber(pr, 'pull request'), number); + if (!pullRequest.merged) fail(`pull request #${pullRequest.number} is not merged`); + if (!pullRequest.closesPlan) { + console.log(`pull request #${pullRequest.number} does not close plan #${number}; nothing to reopen`); + return; + } + // Each read is a snapshot for the write: it checks ownership and decides the claim. + const read = () => readPlanIssue(number, true, { closure: true }); + const isClosed = ({ issue }) => String(issue.state).toUpperCase() === 'CLOSED'; + let observed = read(); + for (let reads = 1; !closedByPullRequest(observed.issue, pullRequest); reads += 1) { + if (reads === REOPEN_READS) + fail( + `GitHub has not closed plan #${number} for pull request #${pullRequest.number} yet; run reopen again once it has`, + ); + sleep(REOPEN_PAUSES_MS[Math.min(reads, REOPEN_PAUSES_MS.length) - 1]); + observed = read(); + } + // A plan that reads open was reopened after that close. A closed plan gets the settle wait and one more read, + // and the write runs only when that read still shows the same close as completed. + let current = observed; + if (isClosed(observed)) { + sleep(REOPEN_SETTLE_MS); + current = read(); + const sameClose = + closedByPullRequest(current.issue, pullRequest) && + current.issue.closedEventAt === observed.issue.closedEventAt && + describeCloser(current.issue.closer) === describeCloser(observed.issue.closer); + if (isClosed(current) && (current.status !== 'finished' || !sameClose)) + fail(`plan #${number} changed during the settle wait; run reopen again`); + } + if (!isClosed(current)) { + console.log(`plan #${number} is open: ${current.status}`); + return; + } + writeReopen(current.issue, pullRequest.number); +} function main([command, ...args]) { const commands = { new: createPlan, @@ -1063,9 +1223,10 @@ function main([command, ...args]) { list: listPlans, archive: archivePlan, retire: (values) => archivePlan(values, true), + reopen: reopenPlan, }; if (!Object.hasOwn(commands, command)) - fail('usage: plan.mjs <new|show|export|edit|status|step|list|archive|retire> ...'); + fail('usage: plan.mjs <new|show|export|edit|status|step|list|archive|retire|reopen> ...'); repository = undefined; commands[command](args); } diff --git a/scripts/tests/fixtures/gh-stub/gh b/scripts/tests/fixtures/gh-stub/gh index e0356619..62ae8be3 100755 --- a/scripts/tests/fixtures/gh-stub/gh +++ b/scripts/tests/fixtures/gh-stub/gh @@ -240,6 +240,21 @@ function graphqlSelections(query, field) { return selections; } +// The fields that `... on <type>{...}` selects at its own level, without the text of nested selections. +function ownFields(query, type) { + const opened = new RegExp(`\\.\\.\\.\\s*on\\s+${type}\\s*\\{`).exec(query); + if (!opened) return ''; + let own = ''; + let depth = 1; + for (let index = opened.index + opened[0].length; index < query.length && depth > 0; index += 1) { + const char = query[index]; + if (char === '{') depth += 1; + else if (char === '}') depth -= 1; + else if (depth === 1) own += char; + } + return own; +} + function graphqlPage(nodes, selection, variables, allowed) { for (const name of selection.args.keys()) { if (!allowed.includes(name)) die(`Unknown GraphQL argument: "${name}"`); @@ -261,6 +276,31 @@ function observeIssue(issue) { } } +// A scheduled change lands once the issue has served `afterReads` GraphQL reads, so the next request that +// reads or writes the issue sees it: a close that GitHub records a while after a merge, or a change that +// someone makes while the helper waits. A `closedEvent` becomes the latest ClosedEvent of the timeline. +function applyDueChanges(issue) { + const reads = issue.graphqlReads ?? 0; + const due = (issue.pendingChanges ?? []).filter((change) => (change.afterReads ?? 0) <= reads); + if (due.length === 0) return; + issue.pendingChanges = issue.pendingChanges.filter((change) => !due.includes(change)); + for (const change of due) { + const { closedEvent, ...fields } = change; + delete fields.afterReads; + Object.assign(issue, fields); + if (closedEvent) { + issue.timelineItems ??= []; + issue.timelineItems.push({ __typename: 'ClosedEvent', ...closedEvent }); + } + touch(issue); + } +} + +function readIssue(issue) { + applyDueChanges(issue); + issue.graphqlReads = (issue.graphqlReads ?? 0) + 1; +} + function respondGraphql(data, stage) { const response = { data }; const error = state.graphqlError; @@ -285,29 +325,65 @@ function respondGraphql(data, stage) { process.exit(0); } +// gh prints a GraphQL response that holds errors, reports the messages on stderr, and exits 1. +function failGraphql(data, errors) { + emit(JSON.stringify({ data, errors })); + save(); + console.error(`gh: ${errors.map((error) => error.message).join('\n')}`); + process.exit(1); +} + +// The issue fields that a mutation payload can select, rendered as the issue stores them. +const STORED_SCALARS = ['number', 'title', 'body', 'state', 'stateReason', 'url', 'createdAt', 'updatedAt']; + // A mutation payload shows the issue as stored right after its own field ran, limited to what the field selects. function mutationPayload(field, object, issue, variables) { const payload = { clientMutationId: null }; const opened = new RegExp(`\\b${object}\\s*\\{`).exec(field.selection); if (!opened) return payload; const selection = field.selection.slice(opened.index); - const stored = /\bbody\b/.test(selection) ? { body: issue.body } : {}; + const stored = {}; + for (const name of STORED_SCALARS) { + if (new RegExp(`\\b${name}\\b`).test(selection)) stored[name] = issue[name]; + } for (const page of graphqlSelections(selection, 'labels')) { stored[page.alias] = graphqlPage(labels(issue.labels), page, variables, ['first', 'after']); } - for (const page of graphqlSelections(selection, 'assignees')) { - stored[page.alias] = graphqlPage(assignees(issue.assignees), page, variables, ['first', 'after']); + for (const connection of ['assignees', 'assignedActors']) { + for (const page of graphqlSelections(selection, connection)) { + stored[page.alias] = graphqlPage(assignees(issue.assignees), page, variables, ['first', 'after']); + } + } + for (const page of graphqlSelections(selection, 'comments')) { + const comments = (issue.comments ?? []).map((comment) => commentJson(comment, issue)); + stored[page.alias] = graphqlPage(comments, page, variables, ['first', 'after']); } payload[object] = stored; return payload; } +function requireInput(field, input, keys) { + for (const key of Object.keys(input)) if (!keys.includes(key)) die(`unsupported ${field.name} input field: ${key}`); + for (const key of keys) if (!Object.hasOwn(input, key)) die(`${field.name} requires input field ${key}`); +} + // GitHub runs the top-level fields of a mutation serially, in document order. updateIssue replaces the body -// and the assignee list that its input names; the label mutations add or remove labels by node id. +// and the assignee list that its input names; the label mutations add or remove labels by node id; +// reopenIssue, addComment, and addAssigneesToAssignable reopen, comment as the viewer, and add assignees. A +// field that fails resolves to null with an error, and the later fields still run. function respondMutation(query, variables) { // The fault flags that faulted `gh issue edit` fault the write request before any field runs. if (state.issueEditError) die(state.issueEditError); - const pattern = /(?:(\w+)\s*:\s*)?\b(updateIssue|addLabelsToLabelable|removeLabelsFromLabelable)\s*\(([^)]*)\)/g; + for (const issue of state.issues) applyDueChanges(issue); + const mutationNames = [ + 'updateIssue', + 'addLabelsToLabelable', + 'removeLabelsFromLabelable', + 'reopenIssue', + 'addComment', + 'addAssigneesToAssignable', + ]; + const pattern = new RegExp(String.raw`(?:(\w+)\s*:\s*)?\b(${mutationNames.join('|')})\s*\(([^)]*)\)`, 'g'); const matches = [...query.matchAll(pattern)]; if (matches.length === 0) die(`unsupported GraphQL mutation: ${query}`); const fields = matches.map((match, index) => ({ @@ -323,6 +399,7 @@ function respondMutation(query, variables) { } const same = (a, b) => a.toLowerCase() === b.toLowerCase(); const data = {}; + const errors = []; for (const field of fields) { const { input } = field; if (!input || typeof input !== 'object' || Array.isArray(input)) die(`${field.name} requires an input object`); @@ -342,6 +419,53 @@ function respondMutation(query, variables) { data[field.alias] = mutationPayload(field, 'issue', issue, variables); continue; } + if (field.name === 'reopenIssue') { + requireInput(field, input, ['issueId']); + const issue = issueByNodeId(input.issueId); + // GitHub refuses every state change of an issue that it left closed with reason REOPENED (#90). + if (issue.state === 'CLOSED' && issue.stateReason === 'REOPENED') { + data[field.alias] = null; + errors.push({ path: [field.alias], message: 'Could not reopen the issue.' }); + continue; + } + if (issue.state === 'CLOSED') { + // reopenLeavesClosed reproduces #90: a reopen that races GitHub's close leaves the issue stuck. + if (!state.reopenLeavesClosed) issue.state = 'OPEN'; + issue.stateReason = 'REOPENED'; + touch(issue); + } + data[field.alias] = mutationPayload(field, 'issue', issue, variables); + continue; + } + if (field.name === 'addComment') { + requireInput(field, input, ['subjectId', 'body']); + const issue = issueByNodeId(input.subjectId); + if (typeof input.body !== 'string' || !input.body) die('AddCommentInput.body must be a non-empty string'); + touch(issue); + state.nextComment ??= 1; + const comment = { + id: state.nextComment++, + body: input.body, + author: state.viewerLogin, + createdAt: issue.updatedAt, + }; + issue.comments ??= []; + issue.comments.push(comment); + data[field.alias] = { + ...mutationPayload(field, 'subject', issue, variables), + ...(/\bcommentEdge\s*\{/.test(field.selection) && { commentEdge: { node: commentJson(comment, issue) } }), + }; + continue; + } + if (field.name === 'addAssigneesToAssignable') { + requireInput(field, input, ['assignableId', 'assigneeIds']); + const issue = issueByNodeId(input.assignableId); + const logins = nodeIds(input.assigneeIds, 'assigneeIds').map(loginByNodeId); + issue.assignees = [...new Set([...(issue.assignees ?? []), ...logins])]; + touch(issue); + data[field.alias] = mutationPayload(field, 'assignable', issue, variables); + continue; + } const issue = issueByNodeId(input.labelableId); const names = nodeIds(input.labelIds, 'labelIds').map(labelNameByNodeId); if (field.name === 'addLabelsToLabelable') { @@ -356,6 +480,7 @@ function respondMutation(query, variables) { touch(issue); data[field.alias] = mutationPayload(field, 'labelable', issue, variables); } + if (errors.length) failGraphql(data, errors); respondGraphql(data, 'mutation'); } @@ -517,6 +642,7 @@ if (group === 'api' && command === 'graphql') { const requestedNumber = graphqlValue(issueSelection[0].args.get('number'), variables); const issue = issueByNumber(Number(requestedNumber)); if (stage === 'snapshot') observeIssue(issue); + readIssue(issue); const payload = { number: issue.number, title: issue.title, @@ -572,7 +698,10 @@ if (group === 'api' && command === 'graphql') { }), }; } - payload[selection.alias] = { nodes: latest ? [{ closer: renderedCloser }] : [] }; + // GitHub stamps every close; a fixture close without a time reads as the issue's last update. + const stamped = /\bcreatedAt\b/.test(ownFields(query, 'ClosedEvent')); + const node = { ...(stamped && { createdAt: latest?.createdAt ?? issue.updatedAt }), closer: renderedCloser }; + payload[selection.alias] = { nodes: latest ? [node] : [] }; } else die(`unsupported timeline item types: ${types ?? '(missing)'}`); } repository.issue = payload; @@ -627,6 +756,41 @@ if (group === 'api' && command === 'graphql') { const stored = Object.keys(state.labels).find((label) => label.toLowerCase() === name?.toLowerCase()); repository[selection.alias] = stored ? { name: stored, id: labelNodeId(stored) } : null; } + // Repository.pullRequest: a merged pull request still lists the issues that it closes (checked on #95). + for (const selection of graphqlSelections(query, 'pullRequest')) { + for (const argument of selection.args.keys()) { + if (argument !== 'number') die(`Unknown GraphQL argument: "${argument}"`); + } + const number = graphqlValue(selection.args.get('number'), variables); + const pullRequest = (state.prs ?? []).find( + (entry) => entry.number === number && entry.repository === state.repo.nameWithOwner, + ); + if (!pullRequest) { + failGraphql({ repository: { ...repository, [selection.alias]: null } }, [ + { + type: 'NOT_FOUND', + path: ['repository', selection.alias], + message: `Could not resolve to a PullRequest with the number of ${number}.`, + }, + ]); + } + const node = { + number, + url: pullRequest.url ?? `https://github.com/${state.repo.nameWithOwner}/pull/${number}`, + state: pullRequest.state, + merged: pullRequest.state === 'MERGED', + mergedAt: pullRequest.mergedAt ?? null, + baseRefName: pullRequest.baseRefName ?? null, + }; + for (const page of graphqlSelections(query, 'closingIssuesReferences')) { + const references = (pullRequest.closingIssuesReferences ?? []).map((issueNumber) => ({ + number: issueNumber, + repository: { nameWithOwner: state.repo.nameWithOwner }, + })); + node[page.alias] = graphqlPage(references, page, variables, ['first', 'after']); + } + repository[selection.alias] = node; + } const viewer = { login: state.viewerLogin ?? '', id: state.viewerLogin ? userNodeId(state.viewerLogin) : null }; respondGraphql({ ...(query.includes('viewer') && { viewer }), repository }, stage); } diff --git a/scripts/tests/plan-cli/harness.mjs b/scripts/tests/plan-cli/harness.mjs index 282ac7d8..967e3738 100644 --- a/scripts/tests/plan-cli/harness.mjs +++ b/scripts/tests/plan-cli/harness.mjs @@ -11,6 +11,11 @@ const GH_STUB_DIR = path.join(ROOT, 'scripts/tests/fixtures/gh-stub'); // The state a repository reaches after its first `new`: every plan label exists as the fake gh // `label create` stores it. A case that depends on label metadata replaces this map itself. const PLAN_LABELS = ['plan', 'plan:drafting', 'plan:planned', 'plan:ongoing', 'plan:blocked']; +// The cases load this hook through `--import` so the waits of `reopen` return at once and get recorded. +export const SKIP_WAITS = new URL('./skip-waits.mjs', import.meta.url).href; +// A partial pull request merges at MERGED_AT, and GitHub records its close 2 s later, as it did for #89. +const MERGED_AT = '2026-08-21T00:00:00Z'; +const CLOSED_AT = '2026-08-21T00:00:02Z'; export function expectSuccess(result, label) { assert.equal(result.status, 0, `${label}: ${result.stderr}`); @@ -62,6 +67,11 @@ export function body(status = 'planned') { ].join('\n'); } +// The change GitHub makes when `closer` closes a plan as completed, for a scheduled change of the fake gh. +export function closedBy(closer, createdAt = CLOSED_AT) { + return { state: 'CLOSED', stateReason: 'COMPLETED', closedEvent: { closer, createdAt } }; +} + // Each test gets its own scratch repository, fake gh state, and XDG_STATE_HOME, so tests share no // issue numbers, labels, or exports and the runner can execute case files in parallel. export function planCliContext(t) { @@ -76,6 +86,7 @@ export function planCliContext(t) { fs.rmSync(stateHome, { recursive: true, force: true }); }); const statePath = path.join(scratch, 'gh-state.json'); + const waitLog = path.join(scratch, 'waits.log'); const initialized = spawnSync('git', ['init', '--quiet'], { cwd: scratch, encoding: 'utf8' }); assert.equal(initialized.status, 0, initialized.stderr); const committed = spawnSync( @@ -118,16 +129,28 @@ export function planCliContext(t) { PATH: `${GH_STUB_DIR}${path.delimiter}${process.env.PATH ?? ''}`, GH_STUB_STATE: statePath, XDG_STATE_HOME: stateHome, + PLAN_CLI_WAIT_LOG: waitLog, }; - function run(...args) { - return spawnSync(process.execPath, [PLAN_CLI, ...args], { + // Node flags such as `['--import', SKIP_WAITS]` apply to this one helper process. + function runWith(nodeFlags, ...args) { + return spawnSync(process.execPath, [...nodeFlags, PLAN_CLI, ...args], { cwd: scratch, encoding: 'utf8', env: childEnv, }); } + function run(...args) { + return runWith([], ...args); + } + + // The timeouts that the skip-waits hook received, in call order. + function waits() { + if (!fs.existsSync(waitLog)) return []; + return fs.readFileSync(waitLog, 'utf8').split('\n').filter(Boolean).map(Number); + } + function loadState() { return JSON.parse(fs.readFileSync(statePath, 'utf8')); } @@ -146,6 +169,21 @@ export function planCliContext(t) { updateState((state) => update(state.issues.find((entry) => entry.number === number))); } + // The gh calls that write: GraphQL mutations and every non-api command other than `view` and `list`. + function writes() { + return loadState().calls.filter(([group, command, ...rest]) => { + if (group !== 'api') return !['view', 'list'].includes(command); + return command === 'graphql' && rest.some((argument) => /^query=\s*mutation\b/.test(argument)); + }); + } + + // Each change lands once the fake gh has served `afterReads` GraphQL reads of the issue. + function schedule(number, ...changes) { + updateIssue(number, (entry) => { + entry.pendingChanges = [...(entry.pendingChanges ?? []), ...changes]; + }); + } + function addIssueComment(number, body, author = 'plan-agent') { updateState((state) => { const entry = state.issues.find((candidate) => candidate.number === number); @@ -208,9 +246,37 @@ export function planCliContext(t) { return { number, closer }; } + // An ongoing plan after the merge of a partial pull request that lists it in closingIssuesReferences. + // The plan still reads OPEN: a case schedules the close that GitHub records a while after the merge. + function partialMerge(name, { assignees = [], merged = true, closes = true } = {}) { + const number = createPlan(name, body('in-flight')); + const pr = 100 + number; + updateState((state) => { + const entry = state.issues.find((candidate) => candidate.number === number); + entry.labels = ['plan', 'plan:ongoing']; + entry.assignees = assignees; + state.prs.push({ + number: pr, + url: `https://github.com/DocksDocks/fixture/pull/${pr}`, + state: merged ? 'MERGED' : 'OPEN', + mergedAt: merged ? MERGED_AT : null, + baseRefName: 'main', + repository: 'DocksDocks/fixture', + closingIssuesReferences: closes ? [number] : [], + }); + // The call log then holds only what the case itself runs. + state.calls = []; + }); + return { number, pr, closer: { __typename: 'PullRequest', number: pr } }; + } + return { scratch, run, + runWith, + waits, + writes, + schedule, loadState, updateState, issue, @@ -220,5 +286,6 @@ export function planCliContext(t) { exported, edit, closedPlan, + partialMerge, }; } diff --git a/scripts/tests/plan-cli/reopen.test.mjs b/scripts/tests/plan-cli/reopen.test.mjs new file mode 100644 index 00000000..44cb1d50 --- /dev/null +++ b/scripts/tests/plan-cli/reopen.test.mjs @@ -0,0 +1,277 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { closedBy, expectSuccess, planCliContext, refuse, SKIP_WAITS } from './harness.mjs'; + +// The fake gh counts the GraphQL reads of each issue and lands a scheduled change once the plan has served +// `afterReads` of them. These cases expect `reopen` to read the plan once per poll, the first read being +// poll 1, and once more after the settle wait, right before its single write request. +const SETTLE_WAIT = 10_000; +const ONGOING = { state: 'OPEN', stateReason: null, labels: ['plan', 'plan:ongoing'], assignees: [], comments: [] }; + +function reopenComment(pr) { + return `Reopened: partial pull request #${pr} closed this plan through its linked branch. Work continues.`; +} + +function stuckRefusal(number) { + return `GitHub left plan #${number} closed with reason REOPENED and refuses every state change; ask GitHub Support to repair it`; +} + +// Every reopen loads the skip-waits hook, so its pauses and its settle wait return at once and get recorded. +function reopen({ runWith }, plan) { + return runWith(['--import', SKIP_WAITS], 'reopen', String(plan.number), '--pr', String(plan.pr)); +} + +// The plan fields that a reopen may change; a comment reduces to its author and body. +function stored({ issue }, number) { + const { state, stateReason, labels, assignees, comments } = issue(number); + return { state, stateReason, labels, assignees, comments: comments.map(({ author, body }) => ({ author, body })) }; +} + +test('reopen waits for the close by the pull request, then reopens with the contract comment and a claim', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('reopen after the close'); + // GitHub records the close on the third read, so the helper pauses 250 ms and 500 ms before it sees it. + context.schedule(plan.number, { afterReads: 2, ...closedBy(plan.closer) }); + const result = reopen(context, plan); + expectSuccess(result, 'reopen'); + assert.equal(result.stdout, `plan #${plan.number} reopened: ongoing\n`); + assert.equal(result.stderr, ''); + assert.deepEqual(context.waits(), [250, 500, SETTLE_WAIT]); + assert.deepEqual(stored(context, plan.number), { + ...ONGOING, + stateReason: 'REOPENED', + assignees: ['plan-agent'], + comments: [{ author: 'plan-agent', body: reopenComment(plan.pr) }], + }); + assert.equal(context.writes().length, 1, 'the reopen, the comment, and the claim land in one request'); +}); + +test('reopen accepts the close by a commit of the merged pull request and leaves an owned plan with its owner', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('commit close', { assignees: ['plan-agent'] }); + const oid = 'c0ffee0000000000000000000000000000000001'; + context.updateState((state) => { + state.commits = [{ oid, associatedPullRequests: [{ number: plan.pr }] }]; + }); + context.schedule(plan.number, { afterReads: 0, ...closedBy({ __typename: 'Commit', oid }) }); + const result = reopen(context, plan); + expectSuccess(result, 'reopen after a commit close'); + assert.equal(result.stdout, `plan #${plan.number} reopened: ongoing\n`); + assert.deepEqual(context.waits(), [SETTLE_WAIT]); + assert.deepEqual(stored(context, plan.number), { + ...ONGOING, + stateReason: 'REOPENED', + assignees: ['plan-agent'], + comments: [{ author: 'plan-agent', body: reopenComment(plan.pr) }], + }); + const writes = context.writes(); + assert.equal(writes.length, 1); + assert.ok( + !writes[0].some((argument) => argument.includes('addAssigneesToAssignable')), + 'an owned plan needs no claim', + ); +}); + +test('reopen writes nothing when the merged pull request does not close the plan', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('not closed by the pull request', { closes: false }); + const result = reopen(context, plan); + expectSuccess(result, 'reopen without a closing reference'); + assert.equal(result.stdout, `pull request #${plan.pr} does not close plan #${plan.number}; nothing to reopen\n`); + assert.equal(result.stderr, ''); + assert.deepEqual(stored(context, plan.number), ONGOING); + assert.deepEqual(context.writes(), []); + assert.deepEqual(context.waits(), []); +}); + +test('reopen leaves a retired plan untouched when the merged pull request does not close it', (t) => { + const context = planCliContext(t); + // The shape of probe issue #94: retired, unlabelled, owned by the viewer, and not closed by #95. + const plan = context.partialMerge('retired probe', { assignees: ['plan-agent'], closes: false }); + context.updateIssue(plan.number, (entry) => { + entry.state = 'CLOSED'; + entry.stateReason = 'NOT_PLANNED'; + entry.labels = []; + }); + const result = reopen(context, plan); + expectSuccess(result, 'reopen a retired plan'); + assert.equal(result.stdout, `pull request #${plan.pr} does not close plan #${plan.number}; nothing to reopen\n`); + assert.deepEqual(stored(context, plan.number), { + ...ONGOING, + state: 'CLOSED', + stateReason: 'NOT_PLANNED', + labels: [], + assignees: ['plan-agent'], + }); + assert.deepEqual(context.writes(), []); +}); + +test('reopen refuses a pull request that has not merged', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('unmerged pull request', { merged: false }); + refuse(reopen(context, plan), `pull request #${plan.pr} is not merged`); + assert.deepEqual(stored(context, plan.number), ONGOING); + assert.deepEqual(context.writes(), []); +}); + +test('reopen refuses a plan that another closer closed after the pull request merged', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('closed by another pull request'); + const other = { + __typename: 'PullRequest', + number: plan.pr + 1, + url: `https://github.com/DocksDocks/fixture/pull/${plan.pr + 1}`, + state: 'MERGED', + mergedAt: '2026-08-21T00:00:05Z', + baseRefName: 'main', + repository: { nameWithOwner: 'DocksDocks/fixture' }, + }; + context.schedule(plan.number, { afterReads: 0, ...closedBy(other, '2026-08-21T00:00:06Z') }); + refuse( + reopen(context, plan), + `plan #${plan.number} was closed by ${other.url} after pull request #${plan.pr} merged; reopen it by hand only if that close was a mistake`, + ); + assert.deepEqual(stored(context, plan.number), { ...ONGOING, state: 'CLOSED', stateReason: 'COMPLETED' }); + assert.deepEqual(context.writes(), []); +}); + +test('reopen refuses a plan that GitHub left closed with reason REOPENED, and show advises GitHub Support', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('stuck before the reopen'); + // A hand reopen raced GitHub's close, as in #90. + context.schedule(plan.number, { afterReads: 0, ...closedBy(plan.closer), stateReason: 'REOPENED' }); + refuse(reopen(context, plan), stuckRefusal(plan.number)); + assert.deepEqual(stored(context, plan.number), { ...ONGOING, state: 'CLOSED', stateReason: 'REOPENED' }); + assert.deepEqual(context.writes(), []); + const shown = context.run('show', String(plan.number)); + expectSuccess(shown, 'show a stuck plan'); + const lines = shown.stdout.split('\n'); + assert.equal( + lines[0], + `#${plan.number} · unreadable · stuck before the reopen · https://github.com/DocksDocks/fixture/issues/${plan.number}`, + ); + assert.ok( + lines.includes( + 'advice: GitHub left this issue closed with reason REOPENED and refuses every state change; ask GitHub Support to repair it, and record step results in an issue comment until then', + ), + shown.stdout, + ); +}); + +test('reopen refuses when GitHub leaves the plan closed with reason REOPENED after the reopen', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('stuck after the reopen'); + context.schedule(plan.number, { afterReads: 0, ...closedBy(plan.closer) }); + context.updateState((state) => { + state.reopenLeavesClosed = true; + }); + refuse(reopen(context, plan), stuckRefusal(plan.number)); + // The one request ran every field: the comment and the claim landed, and only the state stuck. + assert.deepEqual(stored(context, plan.number), { + ...ONGOING, + state: 'CLOSED', + stateReason: 'REOPENED', + assignees: ['plan-agent'], + comments: [{ author: 'plan-agent', body: reopenComment(plan.pr) }], + }); + assert.equal(context.writes().length, 1); +}); + +test('reopen refuses with the stuck state when GitHub answers the reopen with an error', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('reopen answered with an error'); + // Another reopen races GitHub's close after the helper's second read, so the issue is stuck at the write. + context.schedule( + plan.number, + { afterReads: 0, ...closedBy(plan.closer) }, + { afterReads: 2, stateReason: 'REOPENED' }, + ); + refuse(reopen(context, plan), stuckRefusal(plan.number)); + const { state, stateReason } = context.issue(plan.number); + assert.deepEqual([state, stateReason], ['CLOSED', 'REOPENED']); + assert.equal(context.writes().length, 1, 'the helper does not retry the reopen'); +}); + +test('reopen prints the open status and writes nothing for a plan reopened after the pull request closed it', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('already reopened'); + context.schedule(plan.number, { afterReads: 0, ...closedBy(plan.closer), state: 'OPEN', stateReason: 'REOPENED' }); + const result = reopen(context, plan); + expectSuccess(result, 'reopen an open plan'); + assert.equal(result.stdout, `plan #${plan.number} is open: ongoing\n`); + assert.equal(result.stderr, ''); + assert.deepEqual(stored(context, plan.number), { ...ONGOING, stateReason: 'REOPENED' }); + assert.deepEqual(context.writes(), []); +}); + +test('reopen prints the open status and writes nothing when someone reopens the plan during the settle wait', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('reopened during the settle wait'); + context.schedule( + plan.number, + { afterReads: 0, ...closedBy(plan.closer) }, + { afterReads: 1, state: 'OPEN', stateReason: 'REOPENED' }, + ); + const result = reopen(context, plan); + expectSuccess(result, 'reopen a plan reopened meanwhile'); + assert.equal(result.stdout, `plan #${plan.number} is open: ongoing\n`); + assert.equal(result.stderr, ''); + assert.deepEqual(context.waits(), [SETTLE_WAIT]); + assert.deepEqual(stored(context, plan.number), { ...ONGOING, stateReason: 'REOPENED' }); + assert.deepEqual(context.writes(), []); +}); + +test('reopen refuses after 34 reads when GitHub never closes the plan for the merged pull request', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('never closed'); + // The latest close is older than this merge: a commit that an earlier partial pull request merged, and that + // this pull request also contains, closed the plan before someone reopened it. + const earlier = plan.pr - 1; + const oid = 'c0ffee0000000000000000000000000000000002'; + context.updateState((state) => { + state.prs.push({ + number: earlier, + url: `https://github.com/DocksDocks/fixture/pull/${earlier}`, + state: 'MERGED', + mergedAt: '2026-08-20T22:00:00Z', + baseRefName: 'main', + repository: 'DocksDocks/fixture', + closingIssuesReferences: [plan.number], + }); + state.commits = [{ oid, associatedPullRequests: [{ number: earlier }, { number: plan.pr }] }]; + }); + context.updateIssue(plan.number, (entry) => { + entry.stateReason = 'REOPENED'; + entry.timelineItems = [ + { __typename: 'ClosedEvent', closer: { __typename: 'Commit', oid }, createdAt: '2026-08-20T22:00:02Z' }, + ]; + }); + refuse( + reopen(context, plan), + `GitHub has not closed plan #${plan.number} for pull request #${plan.pr} yet; run reopen again once it has`, + ); + assert.equal(context.issue(plan.number).graphqlReads, 34); + // The pause starts at 250 ms and doubles up to 2 s between reads; no settle wait follows. + assert.deepEqual(context.waits(), [250, 500, 1000, ...Array(30).fill(2000)]); + assert.deepEqual(stored(context, plan.number), { ...ONGOING, stateReason: 'REOPENED' }); + assert.deepEqual(context.writes(), []); +}); + +test('reopen refuses a plan that another login claims during the settle wait', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('claimed during the settle wait'); + context.schedule( + plan.number, + { afterReads: 0, ...closedBy(plan.closer) }, + { afterReads: 1, assignees: ['other-agent'] }, + ); + refuse(reopen(context, plan), `plan #${plan.number} is owned by other-agent`); + assert.deepEqual(context.waits(), [SETTLE_WAIT]); + assert.deepEqual(stored(context, plan.number), { + ...ONGOING, + state: 'CLOSED', + stateReason: 'COMPLETED', + assignees: ['other-agent'], + }); + assert.deepEqual(context.writes(), []); +}); diff --git a/scripts/tests/plan-cli/skip-waits.mjs b/scripts/tests/plan-cli/skip-waits.mjs new file mode 100644 index 00000000..430062ec --- /dev/null +++ b/scripts/tests/plan-cli/skip-waits.mjs @@ -0,0 +1,11 @@ +// The plan-CLI cases load this hook through `--import`, so the waits of `reopen` return at once and the +// suite stays fast; the shipped helper keeps its real waits. Each skipped timeout is appended to the file +// that PLAN_CLI_WAIT_LOG names, so a case can check which pauses and settle wait the helper asked for. +import fs from 'node:fs'; + +const log = process.env.PLAN_CLI_WAIT_LOG; + +Atomics.wait = (_typedArray, _index, _value, timeout) => { + if (log) fs.appendFileSync(log, `${timeout}\n`); + return 'timed-out'; +}; From 8542d798faf7e0daa0c9131b98620fba595861e5 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:40:59 -0300 Subject: [PATCH 3/7] feat(docks): add report-skill-issue and a report section in every skill Agents that hit a wrong, outdated, or unsafe instruction in a shipped skill had no documented way to tell this repository (#96). #90 showed both the value of such a report and its risk: it named a private repository and a private plan title in a public issue. The new docks skill report-skill-issue drafts one reproducible issue for DocksDocks/docks: it confirms the problem is in the skill, records `<plugin>:<skill>`, the plugin version, the skill's updated date and hash prefix, the runtime, and the exact instruction, searches for an existing report first, and files with `gh issue create --body-file` only after the user approves the exact text (a run that cannot ask prints the draft and a prefilled URL). Private names, secrets, and machine paths stay out; a security problem never goes public. Every shipped skill of both plugins now ends with one byte-identical "Report a problem with this skill" section that points to it; without docks installed, an agent only drafts for the user. plugins/docks/skills/AGENTS.md owns the rule and its verify command. Checks: the 30 sections hash to one value; the skill scores 15; a trigger check routed 9 of 9 report requests to it and 9 of 9 near misses elsewhere; with the skill, three fresh agents closed the gaps that agents without it showed (search, template, approval, redaction, security kept private). Refs #96 --- README.md | 2 +- plugins/docks/README.md | 2 +- plugins/docks/skills/AGENTS.md | 4 + .../skills/engineering/accessibility/SKILL.md | 8 +- .../skills/engineering/code-clarity/SKILL.md | 8 +- .../engineering/code-guardrails/SKILL.md | 8 +- .../skills/engineering/code-review/SKILL.md | 8 +- .../engineering/commit-discipline/SKILL.md | 8 +- .../engineering/dep-vuln-workflow/SKILL.md | 8 +- .../engineering/design-tokenization/SKILL.md | 8 +- .../skills/engineering/fix-workflow/SKILL.md | 8 +- .../engineering/lint-no-suppressions/SKILL.md | 8 +- .../make-interfaces-feel-better/SKILL.md | 8 +- .../react-component-patterns/SKILL.md | 8 +- .../skills/engineering/refactor/SKILL.md | 8 +- .../skills/engineering/security/SKILL.md | 8 +- .../docks/skills/engineering/solid/SKILL.md | 8 +- .../skills/engineering/tdd-workflow/SKILL.md | 8 +- .../skills/engineering/test-coverage/SKILL.md | 8 +- .../type-safety-discipline/SKILL.md | 8 +- .../productivity/agent-first-setup/SKILL.md | 8 +- .../skills/productivity/context-tree/SKILL.md | 8 +- .../productivity/multi-tool-bridge/SKILL.md | 8 +- .../productivity/report-skill-issue/SKILL.md | 240 ++++++++++++++++++ .../skills/productivity/scaffold/SKILL.md | 8 +- .../skill-agent-pipeline/SKILL.md | 8 +- .../productivity/skill-maintenance/SKILL.md | 8 +- .../skills/productivity/write-skill/SKILL.md | 6 +- .../skills/productivity/zoom-out/SKILL.md | 8 +- plugins/plan-lifecycle/skills/AGENTS.md | 6 + .../skills/productivity/plan-manager/SKILL.md | 6 +- .../productivity/plan-reviewer/SKILL.md | 8 +- .../productivity/plan-workspace/SKILL.md | 8 +- .../skills/productivity/yolo/SKILL.md | 8 +- 34 files changed, 424 insertions(+), 58 deletions(-) create mode 100644 plugins/docks/skills/productivity/report-skill-issue/SKILL.md diff --git a/README.md b/README.md index 4795d99b..c051fad3 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ Auto-trigger on matching tasks (all `user-invocable: false` except `make-interfa | `solid` | Generic SOLID for TS/Python/Go modules - strategy maps, discriminated unions, fat-interface splits, dependency injection | | `type-safety-discipline` | Branded/newtype IDs, discriminated unions, parse-don't-validate - TS primary; references for Rust/Kotlin/Python | -The `productivity/` category contains `agent-first-setup`, `context-tree`, `multi-tool-bridge`, `scaffold`, `skill-agent-pipeline`, `skill-maintenance`, `write-skill`, and `zoom-out`. +The `productivity/` category contains `agent-first-setup`, `context-tree`, `multi-tool-bridge`, `report-skill-issue`, `scaffold`, `skill-agent-pipeline`, `skill-maintenance`, `write-skill`, and `zoom-out`. ### Plan lifecycle (the `plan-lifecycle` plugin) diff --git a/plugins/docks/README.md b/plugins/docks/README.md index 896e061c..0ffcf41d 100644 --- a/plugins/docks/README.md +++ b/plugins/docks/README.md @@ -58,7 +58,7 @@ Auto-trigger on matching tasks (all `user-invocable: false` except `make-interfa | `solid` | Generic SOLID for TS/Python/Go modules — strategy maps, discriminated unions, fat-interface splits, dependency injection | | `type-safety-discipline` | Branded/newtype IDs, discriminated unions, parse-don't-validate — TS primary; references for Rust/Kotlin/Python | -Plus `agent-first-setup`, `context-tree`, `multi-tool-bridge`, `scaffold`, `skill-maintenance`, `write-skill`, and `zoom-out` under `productivity/`. +Plus `agent-first-setup`, `context-tree`, `multi-tool-bridge`, `report-skill-issue`, `scaffold`, `skill-maintenance`, `write-skill`, and `zoom-out` under `productivity/`. ### Plan lifecycle (companion plugin) diff --git a/plugins/docks/skills/AGENTS.md b/plugins/docks/skills/AGENTS.md index 690be190..ef5ae1d8 100644 --- a/plugins/docks/skills/AGENTS.md +++ b/plugins/docks/skills/AGENTS.md @@ -102,6 +102,10 @@ byte-identical absent-lifecycle prerequisite paragraph; no check compares the copies. `plugins/plan-lifecycle/skills/AGENTS.md` (Fail-loud routing) owns the rule and the command that finds every copy. +## Report section (every shipped skill) + +Every shipped skill of the docks and plan-lifecycle plugins ends with one byte-identical `## Report a problem with this skill` section, so an agent that uses a skill sees where and how to report a problem with it. The section points to the docks `report-skill-issue` skill. For a runtime without docks, it allows only a draft for the user, because the publication safeguards live in that skill. A new skill copies the section unchanged; a change to the section changes every copy in the same commit. Verify: `for f in plugins/docks/skills/*/*/SKILL.md plugins/plan-lifecycle/skills/*/*/SKILL.md; do sed -n '/^## Report a problem with this skill$/,$p' "$f" | sha256sum; done | sort | uniq -c` must print one line whose count equals the number of shipped skills. + ## Cross-tool wording (Claude Code + Codex) Skills run in both runtimes; phrase for both. Sources: the Claude Code skills docs (`## Sources`) and the openai/codex source. diff --git a/plugins/docks/skills/engineering/accessibility/SKILL.md b/plugins/docks/skills/engineering/accessibility/SKILL.md index e5062b0d..5f02d513 100644 --- a/plugins/docks/skills/engineering/accessibility/SKILL.md +++ b/plugins/docks/skills/engineering/accessibility/SKILL.md @@ -4,8 +4,8 @@ description: "Use when making UI accessible: focus trap + restore-on-close in mo user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "dbca450826300d1e4d88fd6331e04919988a6d33b77a59074cd569f940a98104" + updated: "2026-10-23" + content_hash: "5ef4ee5bbf865ad959c5c41fcec507c056878b618fab21a212750a29610a9c44" --- # Accessibility @@ -212,3 +212,7 @@ Contrast criteria (1.4.x) audits route to `design-tokenization`. - APG patterns index: https://www.w3.org/WAI/ARIA/apg/patterns/ · Read Me First: https://www.w3.org/WAI/ARIA/apg/practices/read-me-first/ - WCAG 2.2 changes: https://www.w3.org/WAI/standards-guidelines/wcag/new-in-22/ - Companion skills: `design-tokenization` (contrast/tokens), `make-interfaces-feel-better` (motion values, hit areas), `react-component-patterns` (component APIs) + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/code-clarity/SKILL.md b/plugins/docks/skills/engineering/code-clarity/SKILL.md index 2106510f..af180527 100644 --- a/plugins/docks/skills/engineering/code-clarity/SKILL.md +++ b/plugins/docks/skills/engineering/code-clarity/SKILL.md @@ -4,8 +4,8 @@ description: "Use when code is hard to understand without narration; improving n user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-24" - content_hash: "1dafc5e1aed48b9f9a95c837935aeea0c9845f20e104ecdc2bada93115cdfcf8" + updated: "2026-10-23" + content_hash: "1e0bd7d6d6e2c5e7bd59e09b9c672a842f11ec8a1120cb944af4418ec34b8297" --- # Code Clarity @@ -192,3 +192,7 @@ shorter files, more abstractions, or a higher raw test count. - `tdd-workflow`: a new behavior contract should be written test-first. - `solid`: a demonstrated mixed change axis, fat Interface, growing dispatch, or concrete dependency needs a structural pattern. + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/code-guardrails/SKILL.md b/plugins/docks/skills/engineering/code-guardrails/SKILL.md index 4dea9e3a..b0cb27fb 100644 --- a/plugins/docks/skills/engineering/code-guardrails/SKILL.md +++ b/plugins/docks/skills/engineering/code-guardrails/SKILL.md @@ -4,8 +4,8 @@ description: "Use when setting up strict linter rules for TypeScript (oxlint, an user-invocable: true metadata: pattern: tool-wrapper - updated: "2026-09-24" - content_hash: "d7581cc74f6c4adf10f22114724c3294dd5e419cdf43242b311a132241619ca1" + updated: "2026-10-23" + content_hash: "edb40fc49d952999cc51e8dcb7515e66ff7c17d813bee9a3388a1d71464a019a" --- # Code Guardrails @@ -238,3 +238,7 @@ Then list the tools you did not install and the questions for the user. - Run the two checks in "Find scattered values" again. Each remaining repeated value must have a reason in the report. - Run the project tests. A moved constant must not change behavior. + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/code-review/SKILL.md b/plugins/docks/skills/engineering/code-review/SKILL.md index 57674547..67878a89 100644 --- a/plugins/docks/skills/engineering/code-review/SKILL.md +++ b/plugins/docks/skills/engineering/code-review/SKILL.md @@ -4,8 +4,8 @@ description: Use when reviewing code for bugs, security vulnerabilities (OWASP T user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-24" - content_hash: "b79b459375711df7545e89d5b3745f1f387279f4a025c74565fec5d2bbb8d83c" + updated: "2026-10-23" + content_hash: "6957128707464720648cd7207b6cdaa6fceba4bcf22c4a176d79b07a24ed5351" --- # Code Review @@ -211,3 +211,7 @@ MEDIUM · Maintainability · src/api/orders.ts:45-62 - Pairs with: the `security` skill for OWASP Top 10 coverage with adversarial perspective; the `refactor` skill for SOLID and dead-code analysis at scale; `lint-no-suppressions` if your fix would otherwise involve silencing a linter rule - Per-axis finding catalogs: `references/security.md`, `references/perf.md`, `references/maintainability.md` - OWASP Top 10 vocabulary: https://owasp.org/Top10/ + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/commit-discipline/SKILL.md b/plugins/docks/skills/engineering/commit-discipline/SKILL.md index a31acb86..887dbb44 100644 --- a/plugins/docks/skills/engineering/commit-discipline/SKILL.md +++ b/plugins/docks/skills/engineering/commit-discipline/SKILL.md @@ -4,8 +4,8 @@ description: "Use when splitting work into small reviewable/atomic commits, deci user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "7b5fb2ca75a7ab21734bb0f35dfb418f67cf849b8ecd388fd4b6d6e20f0af21f" + updated: "2026-10-23" + content_hash: "157a2715927032a78b2af3dba171e5255a49b11d7c4edd69d7159502871f359b" --- # Commit & PR Discipline @@ -252,3 +252,7 @@ git diff @{upstream}.. --stat # size sanity: is this one reviewable unit? - `git-rebase(1)` — `--autosquash`, `rebase.autoSquash`, `--exec`, `--update-refs`; `git-commit(1)` — `--fixup=amend:`/`reword:` (re-verify against your installed Git's man pages). + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/dep-vuln-workflow/SKILL.md b/plugins/docks/skills/engineering/dep-vuln-workflow/SKILL.md index 5f11ffd2..7b26a366 100644 --- a/plugins/docks/skills/engineering/dep-vuln-workflow/SKILL.md +++ b/plugins/docks/skills/engineering/dep-vuln-workflow/SKILL.md @@ -4,8 +4,8 @@ description: Use when running bun/pnpm/npm/yarn audit, pip-audit, cargo audit, o user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "8655db8483888b6d0e6b85fbb49abef8df212ca148bdfc7d387da3b6248c92ab" + updated: "2026-10-23" + content_hash: "4da60cafda174ac480d497a05983897f90d616f8dc7dea73bd3cc804ce995ab4" --- # Dependency Vulnerability & Upgrade Workflow @@ -132,3 +132,7 @@ The lockfile (`bun.lock` / `pnpm-lock.yaml` / `package-lock.json` / `yarn.lock` - RustSec Advisory DB: https://rustsec.org/advisories/ - PyPA Advisory DB: https://github.com/pypa/advisory-database - Go Vulnerability DB: https://pkg.go.dev/vuln/ + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/design-tokenization/SKILL.md b/plugins/docks/skills/engineering/design-tokenization/SKILL.md index 6d0d7136..197bdb23 100644 --- a/plugins/docks/skills/engineering/design-tokenization/SKILL.md +++ b/plugins/docks/skills/engineering/design-tokenization/SKILL.md @@ -4,8 +4,8 @@ description: "Use when working with colors, Tailwind classes, CSS variables, dar user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "df13bc6b4ad0edc5bdc9ce7a35bae32c80db8ef6a3d07a5760ff5ed5adccbb1d" + updated: "2026-10-23" + content_hash: "3c341f36e34da1d5c1d065b2caab992a96916ad6531f6c24b2cb12c606019161" --- # Design Tokenization @@ -177,3 +177,7 @@ Four-step procedure. Don't skip the audit — proposing token names without seei - shadcn/ui design tokens: https://ui.shadcn.com/docs/theming (uses the `*-foreground` convention) - Brandfetch / logo.dev — verify official hex before adding a brand token - Companion skills: `accessibility` (semantics, keyboard behavior, and ARIA), `make-interfaces-feel-better` (visual polish), `lint-no-suppressions` (when CI greps feel "annoying" — fix the violation, don't disable) + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/fix-workflow/SKILL.md b/plugins/docks/skills/engineering/fix-workflow/SKILL.md index 22b0ffe2..174fc57b 100644 --- a/plugins/docks/skills/engineering/fix-workflow/SKILL.md +++ b/plugins/docks/skills/engineering/fix-workflow/SKILL.md @@ -4,8 +4,8 @@ description: Use when fixing a specific bug, security finding, performance regre user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-24" - content_hash: "655b1d447db265b60d69707a5a43ca2092d34de790fb79d9d4cf6307e4402915" + updated: "2026-10-23" + content_hash: "e1d85256c77c5a5255502ea2bc8ecec68b5910e04df672ee3d2f85ab7eaaacfa" --- # Fix Workflow @@ -195,3 +195,7 @@ After all approved fixes land, run the full verification sweep (tests + lint + t - Per-finding-type templates: `references/security-fix-templates.md`, `references/perf-fix-templates.md`, `references/bug-fix-templates.md` - Feedback-loop construction (Step 0): `references/feedback-loops.md` - Framing for feedback-loops.md adapted from Matt Pocock's `diagnosing-bugs` skill (formerly `diagnose`; MIT): https://github.com/mattpocock/skills/blob/main/skills/engineering/diagnosing-bugs/SKILL.md — the original text was adapted from the `diagnose` version pinned at https://github.com/mattpocock/skills/blob/694fa30311e02c2639942308513555e61ee84a6f/skills/engineering/diagnose/SKILL.md + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/lint-no-suppressions/SKILL.md b/plugins/docks/skills/engineering/lint-no-suppressions/SKILL.md index 9c440ce4..0d175cbf 100644 --- a/plugins/docks/skills/engineering/lint-no-suppressions/SKILL.md +++ b/plugins/docks/skills/engineering/lint-no-suppressions/SKILL.md @@ -4,8 +4,8 @@ description: "Use when a linter or type-checker flags an error; when tempted to user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "f84f8db840dac60cb80898571ddb7a00f63ca056bde41931a8e55887675208fe" + updated: "2026-10-23" + content_hash: "b3715bdf00626f49c21f0ca95630e708afe95485495990f451fb75d192e926e2" --- # Never Suppress Lint / Type Errors @@ -102,3 +102,7 @@ CI must enforce the suppression block too. Client-side hooks are bypassable with - ruff rule reference: https://docs.astral.sh/ruff/rules/ - clippy lint list: https://rust-lang.github.io/rust-clippy/master/ - golangci-lint `nolintlint` (set `require-explanation: true` to require a reason on `//nolint:`): https://golangci-lint.run/docs/linters/configuration/#nolintlint + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/make-interfaces-feel-better/SKILL.md b/plugins/docks/skills/engineering/make-interfaces-feel-better/SKILL.md index 0ece5fdc..c70f730b 100644 --- a/plugins/docks/skills/engineering/make-interfaces-feel-better/SKILL.md +++ b/plugins/docks/skills/engineering/make-interfaces-feel-better/SKILL.md @@ -4,7 +4,7 @@ description: "Use when building or reviewing UI components; fixing positioning t user-invocable: true metadata: pattern: upstream-adapted - updated: "2026-08-25" + updated: "2026-10-23" upstream: source: https://github.com/jakubkrehel/make-interfaces-feel-better license: MIT @@ -15,7 +15,7 @@ metadata: - "2026-08-25: SKILL.md description and icons.md route accessibility semantics, keyboard, and ARIA work to accessibility" - "2026-08-25: SKILL.md and surfaces.md distinguish WCAG 2.2 Target Size (Minimum) AA 24×24px from 44×44px touch and 40×40px dense-desktop usability heuristics" - "2026-08-25: animations.md, surfaces.md, and typography.md add Contents tables required by docks reference hygiene" - content_hash: "7d94c2ea34fa0ed414aabda7aea4ee9484f364a37e93b5e6912a5c8030d0bcbe" + content_hash: "aff027c6dc1155504e6119e8c1ed271b57ac3bd68a6ccd1cada55f7db08ba894" --- # Details that make interfaces feel better @@ -199,3 +199,7 @@ After the findings: 2. **Verdict**: `Block` if any `HIGH` finding remains, `Needs changes` if only `MEDIUM` or `LOW` findings remain, and `Approve` only when no actionable findings remain. List every unverified check beside the verdict. When there are no findings, omit the findings table, state "No actionable interface-polish findings", report verification and rejected candidates, and end with `Approve`. + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/react-component-patterns/SKILL.md b/plugins/docks/skills/engineering/react-component-patterns/SKILL.md index ebedaaad..eda9c7d2 100644 --- a/plugins/docks/skills/engineering/react-component-patterns/SKILL.md +++ b/plugins/docks/skills/engineering/react-component-patterns/SKILL.md @@ -9,8 +9,8 @@ paths: - "**/*.js" metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "192403e00c8c0e97c8f6f7c0e83160842b639908692b3adb824e2267e160188b" + updated: "2026-10-23" + content_hash: "5ab177592d79aafa89439210627d3892305ce699cebd106e35b7e37599591ff9" --- # React Component Patterns @@ -116,3 +116,7 @@ This is the single home of the derived-state example. The full anti-pattern → - `useSyncExternalStore`: https://react.dev/reference/react/useSyncExternalStore - Radix UI Slot source: https://github.com/radix-ui/primitives/blob/main/packages/react/slot/src/slot.tsx - TanStack Table headless docs: https://tanstack.com/table/latest/docs/introduction + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/refactor/SKILL.md b/plugins/docks/skills/engineering/refactor/SKILL.md index 16ad39d3..063528bf 100644 --- a/plugins/docks/skills/engineering/refactor/SKILL.md +++ b/plugins/docks/skills/engineering/refactor/SKILL.md @@ -4,8 +4,8 @@ description: "Use when auditing a codebase for structural issues — dead code, user-invocable: true metadata: pattern: pipeline - updated: "2026-09-24" - content_hash: "051d96108813dc7a20b04b7a27c346e913680099028044d94ba60255534dae16" + updated: "2026-10-23" + content_hash: "adecb5d2163cfbdc270efe3b9cd2cf1feaeafa7e92b894fea3e0792f7eb8969e" --- # Refactor (cross-tool pipeline) @@ -150,3 +150,7 @@ No content loss outside the planned diff: every deletion must be a planned dead- | Flagging modernization from memory | Ships a backwards "fix" (e.g. Next.js `proxy.ts`) | Verify against current docs for the installed version | | Resolving one SOLID violation but adding another | Net-negative refactor ships | Phase 8 re-checks all 5 principles; revert on any new violation | | Assuming a GitHub plan issue is available in a repository with no GitHub remote | The report cannot be filed | Stop and report the missing GitHub remote; do not create a plan file | + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/security/SKILL.md b/plugins/docks/skills/engineering/security/SKILL.md index 17439a4a..8790361d 100644 --- a/plugins/docks/skills/engineering/security/SKILL.md +++ b/plugins/docks/skills/engineering/security/SKILL.md @@ -4,8 +4,8 @@ description: "Use when running a security audit on a codebase — OWASP Top 10, user-invocable: true metadata: pattern: pipeline - updated: "2026-09-23" - content_hash: "11afe1a4f5938ce937e511ad0261f1e370483595191271ccfb96cd6a31e81bcb" + updated: "2026-10-23" + content_hash: "dd88c3cd2db8450e1c418b7652318877677763ccf9bcea87b8c22742bcb34a8d" --- # Security Audit (cross-tool pipeline) @@ -132,3 +132,7 @@ Do not edit source from inside this audit pipeline; requested remediation begins | Skipping synthesis because the scanners "already found everything" | Duplicate, mis-severitied, unreproducible findings ship | Always run Phase 3 — challenge, dedupe, drop unreproducible | | Assuming a GitHub plan issue is available in a repository with no GitHub remote | The report cannot be filed | STOP and report the missing GitHub-backed plan lifecycle; never write a fallback plan file | | Trusting a library API from memory in a suggested fix | A wrong security fix is worse than none | Verify the API against current docs before recommending it | + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/solid/SKILL.md b/plugins/docks/skills/engineering/solid/SKILL.md index 8bcaed33..18e66556 100644 --- a/plugins/docks/skills/engineering/solid/SKILL.md +++ b/plugins/docks/skills/engineering/solid/SKILL.md @@ -4,8 +4,8 @@ description: Use when designing a module / service / class with multiple concern user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "21967495ba4496b81eba6325a7515c887010dbcfc132941b47712f6b739b5a77" + updated: "2026-10-23" + content_hash: "1a8f9e40c7f0d5ed0c837b5f6b318add3d49ab768d6b4657e33ea18b112b4e54" --- # SOLID — Single Responsibility, Open/Closed, Liskov, Interface Segregation, Dependency Inversion @@ -153,3 +153,7 @@ In codebases without a DI container, **function arguments are the abstraction**. - Maintenance twin: the `refactor` skill's `references/solid-analyzer.md` carries a condensed copy of this per-principle rubric and of the TypeScript class-justification gate (canonical deep form in `type-safety-discipline`) — sync the twins in the same commit when either changes. - Uncle Bob's original SOLID essays: https://blog.cleancoder.com/uncle-bob/2014/05/08/SingleReponsibilityPrinciple.html - Depth/seam vocabulary, the three tests, dependency categories, and "replace, don't layer" adapted from Matt Pocock's `codebase-design` skill (MIT; the vocabulary was first part of his `improve-codebase-architecture` skill): https://github.com/mattpocock/skills/blob/main/skills/engineering/codebase-design/SKILL.md + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/tdd-workflow/SKILL.md b/plugins/docks/skills/engineering/tdd-workflow/SKILL.md index 0e9c8ae3..514f9168 100644 --- a/plugins/docks/skills/engineering/tdd-workflow/SKILL.md +++ b/plugins/docks/skills/engineering/tdd-workflow/SKILL.md @@ -4,8 +4,8 @@ description: Use when the user asks for TDD, test-first, "write the test first t user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "238323e1eddc5ccf0ee61f3b63ba451a9b854894200740c003819cfad7c3370d" + updated: "2026-10-23" + content_hash: "c6571cd172d00288928b8badaafde610652cca70ea4ed2f62397ea17a5a87483" --- # Test-Driven Development Workflow @@ -153,3 +153,7 @@ Skip Phase 2 and you cannot distinguish "test passes because feature works" from - Kent Beck, "Test-Driven Development By Example" — the canonical sequence - Cursor agent best-practices (test-first recipe): https://cursor.com/blog/agent-best-practices — blog URLs drift; re-verify the link resolves before citing it - Agentic best-practice applied (model-agnostic): linter-loop 3-strike — stop after the third failed attempt on the same test; the diagnosis is more often wrong than the code + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/test-coverage/SKILL.md b/plugins/docks/skills/engineering/test-coverage/SKILL.md index b7e0a088..8f2dc646 100644 --- a/plugins/docks/skills/engineering/test-coverage/SKILL.md +++ b/plugins/docks/skills/engineering/test-coverage/SKILL.md @@ -4,8 +4,8 @@ description: Use when writing tests for code that ALREADY EXISTS — adding cove user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-09-23" - content_hash: "f6c76c235a29f24268143102cfbb2edd0b344410dc14aa5f977b70981e33d76e" + updated: "2026-10-23" + content_hash: "fed3c74e671dbf24548ea2cc5949cacd500c9a0c497b68e21ea598ee8691668f" --- # Test Coverage Generation @@ -171,3 +171,7 @@ The skills share the lint-loop 3-strike rule and the discipline of running tests - Companion skill: **tdd-workflow** — for test-first development (different ordering, tests as spec) - Per-framework conventions: `references/jest-vitest.md`, `references/pytest.md`, `references/cargo-test.md`, `references/go-test.md`, `references/junit.md` - Project conventions: always read one existing test file before writing the first new one — that's the project's actual style guide + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/engineering/type-safety-discipline/SKILL.md b/plugins/docks/skills/engineering/type-safety-discipline/SKILL.md index abcffa0c..b1000adc 100644 --- a/plugins/docks/skills/engineering/type-safety-discipline/SKILL.md +++ b/plugins/docks/skills/engineering/type-safety-discipline/SKILL.md @@ -11,8 +11,8 @@ paths: - "**/*.py" metadata: pattern: tool-wrapper - updated: "2026-09-24" - content_hash: "6f68d95ea02cdca7a598bf0766ad605909283a7e7a5e14e53e577c7cc38ca03d" + updated: "2026-10-23" + content_hash: "064d110536a72180f3440b9f466e6328c22ed5a1418acf29a7c6137c6c94e615" --- # Type-Safety Discipline @@ -308,3 +308,7 @@ Maintenance twin: this three-case gate is restated in `references/typescript-cla - Rust Book — newtype pattern: https://doc.rust-lang.org/book/ch20-03-advanced-types.html - Kotlin value classes: https://kotlinlang.org/docs/inline-classes.html - Python `typing.NewType`: https://docs.python.org/3/library/typing.html#newtype + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/agent-first-setup/SKILL.md b/plugins/docks/skills/productivity/agent-first-setup/SKILL.md index f47287e6..dbb25995 100644 --- a/plugins/docks/skills/productivity/agent-first-setup/SKILL.md +++ b/plugins/docks/skills/productivity/agent-first-setup/SKILL.md @@ -4,8 +4,8 @@ description: "Use when making a repository agent-first in one pass — root AGEN user-invocable: true metadata: pattern: pipeline - updated: "2026-09-24" - content_hash: "3ff20cb985f33d796cf288319a1a52bd59ef0dbde94bc3435e9f36b43852b072" + updated: "2026-10-23" + content_hash: "34a8a3ebebbb12d223958f9db7c87c0e9f5c44bbeaadbffd068b38da0066c7de" --- # Agent-First Setup @@ -265,3 +265,7 @@ GOOD — STOP at the BROKEN line; the report shows the step that failed. - Codex AGENTS.md discovery (project root down to the working directory): https://developers.openai.com/codex/guides/agents-md - Claude Code memory and native AGENTS.md support: https://code.claude.com/docs/en/memory - agentskills.io specification: https://agentskills.io/specification + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/context-tree/SKILL.md b/plugins/docks/skills/productivity/context-tree/SKILL.md index b0edca27..92830ea8 100644 --- a/plugins/docks/skills/productivity/context-tree/SKILL.md +++ b/plugins/docks/skills/productivity/context-tree/SKILL.md @@ -4,8 +4,8 @@ description: "Use when a repo's root AGENTS.md grew too large and per-area conve user-invocable: true metadata: pattern: meta-skill - updated: "2026-09-24" - content_hash: "97e3beb7f8b6de1d49c59b26c862cc5100cad6fef84de4992720223b9e24d680" + updated: "2026-10-23" + content_hash: "82b38008dd692113158159672e844c46d6dce3c62e1d616f16fb1805c342ccaa" --- # Context Tree — lazy per-folder AGENTS.md @@ -177,3 +177,7 @@ Any `LOST SECTION` / `NET SHRINK` line ⇒ restore root from `"$SOURCE_BEFORE"`, - [`references/conflict-resolution.md`](references/conflict-resolution.md) — existing-file detection, drift/audit logic, merge-vs-overwrite, no-op refresh. - [`references/data-preservation.md`](references/data-preservation.md) — the section-inventory algorithm, per-section relocation table, two-phase write, and the verbatim verification snippet (self-contained; references stay one level deep). - Companion: `plan-manager` (create, review, execute, and finish a durable plan) · `skill-maintenance` (the update-only-when-meaning-changed discipline the refresh op mirrors) · `multi-tool-bridge` (owns every legacy CLAUDE.md write: migrate and delete; same split discipline). + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/multi-tool-bridge/SKILL.md b/plugins/docks/skills/productivity/multi-tool-bridge/SKILL.md index 20844b6b..e03c24c2 100644 --- a/plugins/docks/skills/productivity/multi-tool-bridge/SKILL.md +++ b/plugins/docks/skills/productivity/multi-tool-bridge/SKILL.md @@ -4,8 +4,8 @@ description: Use when setting up multi-tool agent compatibility in a project (Co user-invocable: true metadata: pattern: tool-wrapper - updated: "2026-09-24" - content_hash: "c222893261d33a3ade616bb0cc7367cc0cb998885c696969f45ec193f7940a32" + updated: "2026-10-23" + content_hash: "cd3c4a37157329fe656c8d70c7964edee087c4417db46d392742e9ffe01046a2" --- # Multi-Tool Agent Bridge @@ -298,3 +298,7 @@ Final report (markdown): - `references/agents-md-template.md` — the AGENTS.md scaffold (agents.md-spec-compliant) with its durable-fact fill rules, the `## Context tree` routing section (rows owned by `context-tree`), and the stale-tolerance line - `references/claude-md-classification.md` — keyword rules and section-by-section heuristics for splitting legacy CLAUDE.md content between AGENTS.md and `.claude/rules/`, including nested destinations and the `paths:` frontmatter format - Companion: when porting Claude subagents to Codex (`.claude/agents/*.md` → `.codex/agents/*.toml`), use the `skill-agent-pipeline` skill — its Phase 5 drafts every agent in BOTH formats; format mismatch and model-name translation make symlinks unsuitable. Out of scope here. + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/report-skill-issue/SKILL.md b/plugins/docks/skills/productivity/report-skill-issue/SKILL.md new file mode 100644 index 00000000..6a9ad258 --- /dev/null +++ b/plugins/docks/skills/productivity/report-skill-issue/SKILL.md @@ -0,0 +1,240 @@ +--- +name: report-skill-issue +description: "Use when reporting a problem with a docks or plan-lifecycle skill, reviewer agent, hook, or script: a wrong, outdated, or unsafe instruction, or a shipped script that fails. Drafts a redacted, reproducible issue for DocksDocks/docks and files it with gh only after the user approves the exact text. Not for fixing the skill in place (use write-skill or skill-maintenance) or bugs in the user's own code (use fix-workflow). A security problem never becomes a public issue." +user-invocable: true +metadata: + pattern: tool-wrapper + updated: "2026-10-23" + content_hash: "b47f863afd7b99bac74a711c9e03c5c88e64975a8df0d218cd841dd223d5eb3b" +--- + +# Report a Skill Issue + +The docks and plan-lifecycle plugins come from the public GitHub repository DocksDocks/docks. When one of their skills, reviewer agents, hooks, or scripts gives a wrong, outdated, or unsafe instruction, or a shipped script fails, this skill turns what you saw into one issue there. A useful report lets a maintainer reproduce the problem without access to the user's project. It identifies the exact skill text, quotes the instruction, and shows what happened. It contains nothing private, because anyone can read it. + +<constraint> +DocksDocks/docks is a public repository. Anyone can read its issues and comments, and search engines index them. File no issue and post no comment until the user has read and approved the exact title and body that you will send. Show the draft, ask for approval, and end your turn. Do not run `gh issue create` or `gh issue comment` in the turn that shows the draft. If the user changes the draft, show the new text and get approval again. An approval that the user gave before they saw the final text does not count. In a run where you cannot ask the user (a headless run, a scheduled job, or a subagent with no way to reach the user), print the draft and the prefilled URL as your final message and stop. The URL files nothing by itself: the user reads the draft and files it from the form. Give no prefilled URL for a security problem. File nothing in that run. +</constraint> + +<constraint> +Keep private data out of the title and the body. Leave out secret values and tokens (name the variable only, for example `GH_TOKEN`), names of private repositories, organizations, customers, and people, internal hostnames and URLs, absolute machine paths (write a path relative to the project or the plugin root), proprietary code or data, and titles of private plans and issues. Describe each one generically instead, for example "a private repository" or "a plan issue in a private repository". A security problem is a skill, agent, hook, or script that exposes secrets, runs unsafe commands, or weakens access control. A security problem never goes into a public issue or comment, not even in redacted form. Give the user the draft and tell them to send it privately to the repository owner. If a credential was exposed, tell the user to rotate it now, whether or not they send the report. +</constraint> + +## Scope + +Report problems in what ships with the docks or plan-lifecycle plugins: skills, reviewer agents, hooks, and scripts. A shipped skill sits at `<plugin-root>/skills/<category>/<skill>/SKILL.md`. The plugin root holds `.claude-plugin/plugin.json` or `.codex-plugin/plugin.json`, and its `name` is `docks` or `plan-lifecycle`. + +| The problem is in | Do this | +|---|---| +| A shipped skill, reviewer agent, hook, or script | Use this skill | +| A shipped item, and it exposes secrets, runs unsafe commands, or weakens access control | Write a private draft (see "Security problems"); file no issue | +| The user's own project code | Not a skill problem; use `fix-workflow` | +| Your own run: you skipped a step, changed a command, or acted on an assumption that the skill does not state | Not a report; correct the run and tell the user | +| A skill from another source: another plugin, a project-local skill in `.agents/skills/` or `.claude/skills/`, or a personal skill | Not a DocksDocks report; tell the user which source owns it | +| The skill text in a checkout of DocksDocks/docks that you were asked to change | Edit it with `write-skill` or `skill-maintenance`; no issue | + +## Workflow + +The goal is one approved, redacted, reproducible report. Each stage below states what it must establish; choose the commands that fit the runtime. Do every stage in order, also in a text-only run. Never skip the search in stage 3, because a duplicate report splits the evidence across two issues. + +### 1. Confirm that the problem is in the skill + +Re-read the instruction or the file that failed. Copy the instruction exactly, with the heading that it sits under. For a script, note its path relative to the plugin root and the command that failed. Then rule out the other causes: + +- The user's environment: a missing tool, an old CLI, a permission or network failure, or a project setting that the skill does not cover. +- Your own deviation: compare what you did with the quoted text, word by word. + +Report when the instruction, followed as written, gives a wrong or unsafe result, names a command or option that does not exist, or contradicts another shipped instruction. Also report a shipped script that fails on valid input. If you cannot find the cause, say so in the draft. Do not guess a cause. + +### 2. Identify the exact skill text + +The plugin root is the folder that holds `skills/<category>/<skill>/SKILL.md`. For an agent, hook, or script, it is the folder that holds `.claude-plugin/` or `.codex-plugin/`. Collect these facts. Write `unknown` for any fact that you cannot read; never guess a version or a hash. + +| Fact | Where to read it | +|---|---| +| `<plugin>:<skill>` | `name` in the plugin root's `plugin.json`, and `name` in the skill's frontmatter. For an agent, hook, or script, use its path relative to the plugin root | +| Plugin version | `version` in `.claude-plugin/plugin.json` or `.codex-plugin/plugin.json` in the plugin root | +| Skill metadata | `metadata.updated` and the first 12 characters of `metadata.content_hash` in the skill's frontmatter | +| Runtime | Claude Code, Codex, OpenCode, VS Code Copilot, or omp, and its version (for example from its `--version` output) | +| Model | The model name, if the runtime shows it | + +```bash +# Run in the plugin root. +grep -m1 '"name"' .claude-plugin/plugin.json .codex-plugin/plugin.json +grep -m1 '"version"' .claude-plugin/plugin.json .codex-plugin/plugin.json +grep -E '^ (updated|content_hash):' skills/<category>/<skill>/SKILL.md +``` + +The version and the hash tell the maintainer which text you read. Release tags have the form `<plugin>--v<version>` (`gh release list --repo DocksDocks/docks` lists them). If the user's version is older than the latest release, say so in the draft. + +### 3. Search for an existing report + +Search open and closed issues with the skill name and two or three words from the symptom. Use only public words, because the query goes to GitHub. + +```bash +gh issue list --repo DocksDocks/docks --state all --search "<skill name> <key words>" +gh issue view <n> --repo DocksDocks/docks --comments +``` + +| Search result | Action | +|---|---| +| An open issue describes the same problem | Draft a comment that adds only new facts: another version, another runtime, or a new reproduction | +| A closed issue fixed it in a later plugin version | File nothing; tell the user to update the plugin | +| A closed issue matches, and the problem is still in the latest version | Draft a new issue that links the closed one | +| No match | Draft a new issue | + +### 4. Draft the report + +Fill the template below. Put one problem in each issue. If you found two problems, draft two reports and get approval for each. Keep the quote of the instruction exact. Trim command output to the lines that show the failure. Give times in UTC. Then check the draft against the redaction rule and the list in "Check before filing". + +### 5. Get approval + +Show the user, as the final message of your turn: + +- the target: a new issue in DocksDocks/docks, or a comment on issue #n there; +- the exact title and body; +- the command that you will run, and the GitHub account that it files under (`gh auth status` shows it). + +Ask the user to approve or change the text, then end your turn. A security problem stops before this step: give the private draft instead (see "Security problems"). + +### 6. File the approved text + +Write the approved body, unchanged, to a new temporary file. Run the scan from "Check before filing" on it. File it, remove the file, and give the user the URL that gh prints. + +```bash +body=$(mktemp) +# Write the approved body to "$body" with your file tool, then: +gh issue create --repo DocksDocks/docks --title "<approved title>" --body-file "$body" +# Or, for a comment on an existing issue: +gh issue comment <n> --repo DocksDocks/docks --body-file "$body" +rm -- "$body" +``` + +If your shell does not keep variables between commands, use the path that `mktemp` printed. Use `--body-file`, not `--body`, because the shell can change backticks, `$`, and quotes in inline text. Do not add labels or assignees; the maintainer triages the issue. + +### Without gh + +If `gh` is not installed, or `gh auth status` reports no login, do not install gh and do not ask for a token. Give the user a link that opens the new-issue form with the text filled in: + +`https://github.com/DocksDocks/docks/issues/new?title=<url-encoded title>&body=<url-encoded body>` + +Encode the exact title and body that you show, unchanged, as URL query components, for example with `encodeURIComponent` in Node. If the full URL is longer than about 6,000 characters, give the title and the body as text instead, for the user to paste at `https://github.com/DocksDocks/docks/issues/new`. For a comment on an existing issue, give the text and the issue URL, `https://github.com/DocksDocks/docks/issues/<n>`. Show the draft and the link in the same message. The user files the issue from the form. + +## Security problems + +A skill, agent, hook, or script that exposes secrets, runs unsafe commands, or weakens access control has a security problem. Do not file it, comment on it, or describe it in any public place, not even in redacted form. Instead: + +1. Tell the user to rotate every credential that was exposed, now. +2. Draft the report with every section of the template below, and put "Private security report" above the title. A private report needs the same facts as a public one. +3. Give the draft to the user, to send privately to the repository owner (the DocksDocks account on GitHub). A private channel is not an issue, a comment, a discussion, or a pull request. Give no prefilled issue URL. +4. If the repository accepts private vulnerability reports, the user can send the draft there instead. `gh api repos/DocksDocks/docks/private-vulnerability-reporting` prints `{"enabled":true}` when it does; the form is in the repository's Security tab. + +## Template + +The first line is the title. The body starts at `## Summary`. Write `none` in a section that has nothing to say; do not drop the section. + +```markdown +Title: <plugin>:<skill>: <symptom in one line> + +## Summary + +<Two or three sentences: what the skill told the agent to do, what went wrong, and the effect on the work.> + +## Skill + +- Skill: `<plugin>:<skill>` (or the agent, hook, or script path relative to the plugin root) +- Plugin version: <x.y.z or unknown> +- Skill metadata: updated <YYYY-MM-DD>, content_hash <first 12 characters> +- Runtime: <Claude Code, Codex, OpenCode, VS Code Copilot, or omp> <version or unknown> +- Model: <model name or unknown> + +## What the skill says + +Under "<heading>": + +> <the instruction, quoted exactly> + +## What happened + +<What the agent did when it followed the instruction as written. The exact error text and trimmed output in fenced blocks. A UTC timeline table when timing matters.> + +## Expected + +<What the instruction should have produced.> + +## How to reproduce + +1. <Minimal steps in a generic project, with no private names.> + +## Workaround + +<What worked instead, or none.> + +## Suggested change + +<Optional: the change to the skill text or script that would fix the problem.> +``` + +## BAD / GOOD + +```text +BAD title: plan-manager is broken +GOOD title: plan-lifecycle:plan-manager: reopen right after a partial merge can leave the plan issue closed with reason REOPENED + +BAD body: Seen in acme-corp/billing-api issue #123 (plan "Invoice export for Contoso"). +GOOD body: Seen in a plan issue in a private repository. + +BAD body: gh failed with token ghp_1a2b... for jdoe on build-01.corp.internal +GOOD body: gh failed with an authentication error (token from GH_TOKEN). + +BAD action: a public issue "docks:<skill>: prints the GH_TOKEN value to the log", value redacted +GOOD action: no public issue; a private draft for the repository owner, and advice to rotate the token +``` + +The good title names the plugin, the skill, and one observable symptom, so a maintainer can find it and a duplicate search can match it. + +## Gotchas + +| Gotcha | Do instead | +|---|---| +| A redacted secret still shows a prefix or a length (`ghp_1a2b...`) | Name the variable only (`GH_TOKEN`); show no part of the value | +| Full command output carries paths, hostnames, and user names | Trim to the failing lines; make paths relative to the project or the plugin root | +| The search query carries private words | Search with the skill name and words from the instruction only | +| A paraphrase in "What the skill says" | Quote the instruction exactly, with its heading | +| Two problems in one issue | One problem per issue, each with its own approval | +| The reproduction needs the user's private project | Reduce it to a generic project, or describe the minimal setup | +| A guessed version or hash | Read it from the files, or write `unknown` | +| Local times in the timeline | Convert every time to UTC | +| The temporary body file stays on disk | Remove the temporary file after filing | + +## Check before filing + +Check the draft before you show it, and again before you file it: + +- The title has the form `<plugin>:<skill>: <symptom>` and names one problem. +- Every template section is present; an empty section says `none`. +- "What the skill says" quotes the instruction exactly, with its heading. +- You searched for an existing report (stage 3). A draft that adds to an existing issue is a comment on that issue. +- Your message names the target, the command that you will run, and the GitHub account (stage 5). In a run where you cannot ask, it gives the prefilled URL instead. +- The title and the body contain no secret value or token, no private repository, organization, customer, or person name, no internal hostname, no absolute machine path, no proprietary code or data, and no private plan or issue title. +- The problem is not a security problem. If it is, give a private draft only. +- The user approved this exact title and body after they saw it. + +This scan prints nothing when the body file contains no machine path and no token-shaped string. Read the title for the same patterns. + +```bash +grep -nE '/home/[^/ ]+/|/Users/[^/ ]+/|[A-Za-z]:\\Users\\|gh[pousr]_[A-Za-z0-9]{20,}|github_pat_|AKIA[0-9A-Z]{16}|xox[abprs]-|BEGIN [A-Z ]*PRIVATE KEY' "$body" +``` + +The scan finds only patterns. It does not find a private name or a hostname; read the text for those. + +## Sources + +- `gh issue create`: <https://cli.github.com/manual/gh_issue_create> +- `gh issue list`: <https://cli.github.com/manual/gh_issue_list> +- `gh issue comment`: <https://cli.github.com/manual/gh_issue_comment> +- Creating an issue from a URL query: <https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/creating-an-issue> + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/scaffold/SKILL.md b/plugins/docks/skills/productivity/scaffold/SKILL.md index 2531b7e8..07644a7d 100644 --- a/plugins/docks/skills/productivity/scaffold/SKILL.md +++ b/plugins/docks/skills/productivity/scaffold/SKILL.md @@ -4,8 +4,8 @@ description: "Use when spinning up a new docks-style plugin project, or capturin user-invocable: true metadata: pattern: generative-skill - updated: "2026-09-24" - content_hash: "7bf6913ec04fb4f11b90c5e5b1a4758a596037249c3ce7e6536a03b75f5069a2" + updated: "2026-10-23" + content_hash: "0fbc2a1fc06e9cf2907c747227d679850414cf60e6bed143e7f6293db67b17b3" --- # Scaffold — capture a repo's shape, seed new projects from it @@ -135,3 +135,7 @@ plugins/acme-tools/.claude-plugin/plugin.json ← plugin_name = "acme - [`references/spec-schema.md`](references/spec-schema.md) — full `spec.yaml` schema: every field, `seed_from_skill` vs `template` vs `bundled_skills`, `default_from`, variable rules. - Companion skills: `context-tree` (the nodes seed mode writes) · `plan-workspace`, `plan-manager`, and `plan-reviewer` (the exact bundled plan owners) · `write-skill` (author new skills in the seeded project). + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/skill-agent-pipeline/SKILL.md b/plugins/docks/skills/productivity/skill-agent-pipeline/SKILL.md index e13e8943..09bb2910 100644 --- a/plugins/docks/skills/productivity/skill-agent-pipeline/SKILL.md +++ b/plugins/docks/skills/productivity/skill-agent-pipeline/SKILL.md @@ -4,8 +4,8 @@ description: "Use when bootstrapping or auditing a project's skills and agents user-invocable: true metadata: pattern: pipeline - updated: "2026-09-23" - content_hash: "cc2fb9a0de5b99c74d91bf1dd129e945af6f1e5e0210e71776e59e445f03f3ff" + updated: "2026-10-23" + content_hash: "bdbbd27ba61ed89960c67b81f87a9f8ba4b0151e842f94b8cd434ddee3e3a3c1" --- # Skills & Agents Pipeline (cross-tool) @@ -143,3 +143,7 @@ Any `LOST SECTION` / `NET SHRINK` line ⇒ restore from `/tmp/skill.before`, loc | Unquoted `description:` contains `: ` or `#` | Codex skips the skill with invalid YAML or silently truncated description | Quote every generated description | | Agent skill-references pointing at pre-split paths | Agents land with broken references | Phase 4–5 must reference Phase 3's proposed paths, not old ones | | Editing AGENTS.md / CLAUDE.md from this pipeline | Scope bleed; clobbers cross-tool config | Nested nodes and the routing table → `context-tree`; legacy CLAUDE.md, root `AGENTS.md`, symlinks → `multi-tool-bridge` | + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/skill-maintenance/SKILL.md b/plugins/docks/skills/productivity/skill-maintenance/SKILL.md index d6ac1777..aba631cd 100644 --- a/plugins/docks/skills/productivity/skill-maintenance/SKILL.md +++ b/plugins/docks/skills/productivity/skill-maintenance/SKILL.md @@ -4,8 +4,8 @@ description: "Use when project-local SKILL.md files need validation or refresh a user-invocable: false metadata: pattern: reviewer - updated: "2026-09-23" - content_hash: "7208f28d8a7da0f88b12ba010e9b772d3d011a1c555f4dbb0bb58c993ba4bdf4" + updated: "2026-10-23" + content_hash: "323204626f1f6a8a496618b0124f2dda83ea9ba322d2e325697bd094e1c719bd" --- # Skill Maintenance @@ -205,3 +205,7 @@ loader warning that is not fixed by quoting the description. Read `references/blind-comparison.md` when running the optional old-vs-new comparison. + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/write-skill/SKILL.md b/plugins/docks/skills/productivity/write-skill/SKILL.md index acfc2571..f65e3c4b 100644 --- a/plugins/docks/skills/productivity/write-skill/SKILL.md +++ b/plugins/docks/skills/productivity/write-skill/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: meta-skill updated: "2026-10-23" - content_hash: "637c4dae9c986682675dc37480598f3734c313271253f95f1423b3571609de4b" + content_hash: "bf462a01341249d67ae1065d80333c7cfb60e760deef4dfad7c80ca67e81e8ae" --- # Write a Skill (docks conventions) @@ -221,3 +221,7 @@ A skill that MOVES, SPLITS, or REWRITES existing files can drop content with no ## Source attribution Framing ("the description is the only thing your agent sees") adapted from Matt Pocock's `write-a-skill` (MIT; removed upstream in June 2026 and replaced by `writing-for-agents`, <https://github.com/mattpocock/skills/blob/main/skills/productivity/writing-for-agents/SKILL.md>). Last version: <https://github.com/mattpocock/skills/blob/ab7196a1584ac60688aeb63cf0b56a5114c4a6f3/skills/productivity/write-a-skill/SKILL.md>. Step 0, the baseline check, the trigger check (query set with `query` / `should_trigger`, held-back queries, intent categories over keyword lists), and the stop condition are paraphrased from Anthropic's `skill-creator` (Apache-2.0; <https://github.com/anthropics/skills/blob/main/skills/skill-creator/SKILL.md>). The assertion rules (evidence per pass, surface compliance fails, non-discriminating assertions flagged) are paraphrased from its grader prompt (Apache-2.0; <https://github.com/anthropics/skills/blob/main/skills/skill-creator/agents/grader.md>). Degrees-of-freedom and the near-miss negative idea adapted from Anthropic's skill authoring best practices (<https://platform.claude.com/docs/en/agents-and-tools/agent-skills/best-practices>) and its `skill-creator` plugin. Body / rubric / loop are docks-specific — `skill-creator` covers evals and benchmarking; this one covers the kit conventions neither generic skill knows. The markdown + frontmatter + progressive-disclosure shape itself is convergent prior art: Google's Open Knowledge Format (OKF v0.1, Apache-2.0 — markdown files with YAML frontmatter; <https://cloud.google.com/blog/products/data-analytics/how-the-open-knowledge-format-can-improve-data-sharing>, spec in <https://github.com/GoogleCloudPlatform/knowledge-catalog>) and Karpathy's LLM-Wiki pattern (raw sources / wiki / schema layers with Ingest–Query–Lint ops; <https://gist.github.com/karpathy/442a6bf555914893e9891c11519de94f>) standardize the same idea independently. + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/docks/skills/productivity/zoom-out/SKILL.md b/plugins/docks/skills/productivity/zoom-out/SKILL.md index 7b112253..a7839289 100644 --- a/plugins/docks/skills/productivity/zoom-out/SKILL.md +++ b/plugins/docks/skills/productivity/zoom-out/SKILL.md @@ -4,8 +4,8 @@ description: "Use when tunneling in code-level detail and you need a system-leve user-invocable: true metadata: pattern: micro-skill - updated: "2026-09-23" - content_hash: "81f77792c9967fb2e6b1704f481eab86229e1162e253fc55096186f85bf2cda5" + updated: "2026-10-23" + content_hash: "887fe22d0fc356ee1dd1cf426422dbebd2226fbb0d814ee29025502fff0fef2a" --- # Zoom Out @@ -93,3 +93,7 @@ your question restated: ## Source Pattern adapted from Matt Pocock's `zoom-out` skill (MIT; since removed upstream), pinned at its last version: <https://github.com/mattpocock/skills/blob/801a01cc7d265e06dd9dbcef5a4c471add05a0b3/skills/engineering/zoom-out/SKILL.md>. The upstream skill was a one-line prompt that relied on the project's domain glossary. This skill states the method inline, so it works without a glossary file. + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/plan-lifecycle/skills/AGENTS.md b/plugins/plan-lifecycle/skills/AGENTS.md index f2c2834a..d19b715d 100644 --- a/plugins/plan-lifecycle/skills/AGENTS.md +++ b/plugins/plan-lifecycle/skills/AGENTS.md @@ -84,6 +84,12 @@ prerequisite paragraph naming this plugin, so a runtime without compares the copies. Change the paragraph only in lockstep across every copy (verify: `grep -rl 'Prerequisite: .plan-lifecycle. must be installed' plugins/docks`). +## Report section + +Every skill here also ends with the shared report section, byte-identical +across both plugins. `plugins/docks/skills/AGENTS.md` (Report section) owns +the rule and its verify command. + ## Scoring and namespace Same rubric as every kit skill: `node diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md index 022f25a1..cf4aa610 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-10-23" - content_hash: "9033244710f826b6c8c48909fe7e5ba40173fb7688b81a17cfbf2e1c0f8b716d" + content_hash: "f1012e9255b604143194984ac7b6f482d506bf76b1722b44c3852b2416068bee" --- # Plan Manager @@ -172,3 +172,7 @@ GOOD: Say that the merge closes the issue, merge on `Merge now`, run `reopen <n> BAD: Run `git branch -D` after `git branch -d` refuses. GOOD: Keep the branch and list it under Kept resources. ``` + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/plan-lifecycle/skills/productivity/plan-reviewer/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-reviewer/SKILL.md index 8a4f11a5..87f42158 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-reviewer/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-reviewer/SKILL.md @@ -4,8 +4,8 @@ description: "Use when plan-manager needs a read-only pre-implementation review user-invocable: false metadata: pattern: tool-wrapper - updated: "2026-10-19" - content_hash: "52a28e27f3849805a5044e9c6f16c95320abca5f6ab4aa2f5a19c4645461b94f" + updated: "2026-10-23" + content_hash: "9d25c91145ddc43666e1374476ed27494e901fbb7e938105701bfd3532a62f7d" --- # Plan Reviewer @@ -85,3 +85,7 @@ verdict. Re-read each locator and confirm the proposed fix addresses a real defect. Trace load-bearing symbols to definitions and callers. Remove cosmetic preferences and duplicate findings. Confirm the output is one verdict block and that repository and external state remain unchanged. + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md index 83642e45..099f6f5e 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-workspace/SKILL.md @@ -4,8 +4,8 @@ description: "Use when bootstrapping, migrating, auditing, or explicitly refresh user-invocable: true metadata: pattern: tool-wrapper - updated: "2026-10-18" - content_hash: "666bb6231e8a3be9dc265b1d55c7c061bc1ee34c9a3d6739d982203f31768a77" + updated: "2026-10-23" + content_hash: "43e43336c020090188500830f3dec7b18929b47fa1bcc818cf6c478fecfb7c55" --- # Plans Workspace @@ -151,3 +151,7 @@ GOOD: Report drift and wait for an explicit refresh request. BAD: Parse a legacy plan to create a fresh issue. GOOD: Preserve the file and use only the user's restated title and goal. ``` + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. diff --git a/plugins/plan-lifecycle/skills/productivity/yolo/SKILL.md b/plugins/plan-lifecycle/skills/productivity/yolo/SKILL.md index 30fcf9e3..2a07b9d4 100644 --- a/plugins/plan-lifecycle/skills/productivity/yolo/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/yolo/SKILL.md @@ -5,8 +5,8 @@ user-invocable: true disable-model-invocation: true metadata: pattern: pipeline - updated: "2026-10-21" - content_hash: "127c523b2d0662e08d7ac64f8a11da0e173e2e56cfb605530c4491f5170abcea" + updated: "2026-10-23" + content_hash: "1aa42cf114945d7281e40403f64a0e52f5b8dc5f416f150657e732184df03562" --- # Yolo @@ -336,3 +336,7 @@ GOOD: Continue to the Yolo runs ceiling, then ask the one stalled-review questio BAD: Paste an error log that contains a token into the issue. GOOD: Remove the token and name its variable. ``` + +## Report a problem with this skill + +When this skill gives a wrong, outdated, or unsafe instruction, or a script that it ships fails, report the problem in the public DocksDocks/docks repository. Use the docks `report-skill-issue` skill for the report. If that skill is not installed, do not file anything: give the user a draft instead. The draft names this skill and its plugin version, quotes the instruction, and states what happened and what you expected. Leave out secrets and private names. From 093c13f0a383c88c42730207ce70e8b68e5534c4 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:46:10 -0300 Subject: [PATCH 4/7] fix(docks): build the report-skill-issue link with a script Two dry runs of the new skill encoded the prefilled issue link by hand: one link held different body text than the draft that the user saw, and one held an empty body. A user who approves one text could then file another. The link section now covers both cases that need it (no gh, and a run that cannot ask the user) and gives a Node and a Python command that encode the title and the body file. A repeat dry run on #90's facts produced a link whose body equals the shown draft byte for byte. Refs #96 --- .../productivity/report-skill-issue/SKILL.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/plugins/docks/skills/productivity/report-skill-issue/SKILL.md b/plugins/docks/skills/productivity/report-skill-issue/SKILL.md index 6a9ad258..e7515f95 100644 --- a/plugins/docks/skills/productivity/report-skill-issue/SKILL.md +++ b/plugins/docks/skills/productivity/report-skill-issue/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-10-23" - content_hash: "b47f863afd7b99bac74a711c9e03c5c88e64975a8df0d218cd841dd223d5eb3b" + content_hash: "e314a6864e8f7c6ea04a75c7c134c95283f3f1bd36b352022c7b19ce11cb9b15" --- # Report a Skill Issue @@ -112,13 +112,22 @@ rm -- "$body" If your shell does not keep variables between commands, use the path that `mktemp` printed. Use `--body-file`, not `--body`, because the shell can change backticks, `$`, and quotes in inline text. Do not add labels or assignees; the maintainer triages the issue. -### Without gh +### Without gh, or without a way to ask -If `gh` is not installed, or `gh auth status` reports no login, do not install gh and do not ask for a token. Give the user a link that opens the new-issue form with the text filled in: +Give a prefilled link when gh is missing or not logged in, or when the run cannot ask the user. Do not install gh, and do not ask for a token. The link opens the new-issue form with the text filled in: `https://github.com/DocksDocks/docks/issues/new?title=<url-encoded title>&body=<url-encoded body>` -Encode the exact title and body that you show, unchanged, as URL query components, for example with `encodeURIComponent` in Node. If the full URL is longer than about 6,000 characters, give the title and the body as text instead, for the user to paste at `https://github.com/DocksDocks/docks/issues/new`. For a comment on an existing issue, give the text and the issue URL, `https://github.com/DocksDocks/docks/issues/<n>`. Show the draft and the link in the same message. The user files the issue from the form. +Build the link with a script, not by hand, so that it holds the exact title and body that you show. Write the body to a temporary file as in stage 6, run one of these commands, and then remove the file: + +```bash +# Node +node -e 'const fs = require("fs"); const [t, f] = process.argv.slice(1); console.log("https://github.com/DocksDocks/docks/issues/new?title=" + encodeURIComponent(t) + "&body=" + encodeURIComponent(fs.readFileSync(f, "utf8")))' "<approved title>" "$body" +# Python +python3 -c 'import sys, urllib.parse as p; t, f = sys.argv[1:]; print("https://github.com/DocksDocks/docks/issues/new?title=" + p.quote(t, safe="") + "&body=" + p.quote(open(f, encoding="utf-8").read(), safe=""))' "<approved title>" "$body" +``` + +If the full URL is longer than about 6,000 characters, give the title and the body as text instead, for the user to paste at `https://github.com/DocksDocks/docks/issues/new`. For a comment on an existing issue, give the text and the issue URL, `https://github.com/DocksDocks/docks/issues/<n>`. Show the draft and the link in the same message. The user files the issue from the form. ## Security problems From 6a3ef0e8627f9732d6f7a13dc4e4b5f5f33c2381 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:51:53 -0300 Subject: [PATCH 5/7] fix(plan-lifecycle): refuse reopen for a plan closed before the merge GitHub cannot close an issue that is already closed. When a plan was closed before its partial pull request merged, `reopen` polled 34 times and then told the caller to "run reopen again once it has", so an agent that followed the message would retry forever. After the last read, a plan that still reads closed now gets a refusal that names the earlier closer and says to reopen by hand only if that close was a mistake. The helper still polls first, so a short lag between the issue state and its timeline cannot cause a false refusal. Found in self-review. Red baseline: the new case failed with the old timeout message; the other 13 reopen cases passed. Refs #96 --- .../skills/productivity/plan-manager/SKILL.md | 2 +- .../plan-manager/references/plan-contract.md | 1 + .../plan-manager/scripts/plan.mjs | 9 ++++++- scripts/tests/plan-cli/reopen.test.mjs | 24 +++++++++++++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md index cf4aa610..52aafe5b 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-10-23" - content_hash: "f1012e9255b604143194984ac7b6f482d506bf76b1722b44c3852b2416068bee" + content_hash: "7d8c5904f96e18cc5f4e2a00ea3674b58064ef46b4a94a0f4fdccf9f9e61c64b" --- # Plan Manager diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md index f2a1842b..e683d9cf 100644 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/references/plan-contract.md @@ -287,6 +287,7 @@ reserved label namespace: <label> pull request #<pr> is not merged GitHub has not closed plan #<n> for pull request #<pr> yet; run reopen again once it has plan #<n> was closed by <closer> after pull request #<pr> merged; reopen it by hand only if that close was a mistake +plan #<n> was closed by <closer> before pull request #<pr> merged; reopen it by hand only if that close was a mistake plan #<n> changed during the settle wait; run reopen again GitHub left plan #<n> closed with reason REOPENED and refuses every state change; ask GitHub Support to repair it plan issue state differs after reopen diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs b/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs index 483ca88f..d48b6c24 100755 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs @@ -1186,10 +1186,17 @@ function reopenPlan(args) { const isClosed = ({ issue }) => String(issue.state).toUpperCase() === 'CLOSED'; let observed = read(); for (let reads = 1; !closedByPullRequest(observed.issue, pullRequest); reads += 1) { - if (reads === REOPEN_READS) + if (reads === REOPEN_READS) { + // GitHub cannot close a closed issue, so a plan that still reads closed by an earlier close never gets the + // close from this merge; telling the caller to run reopen again would loop forever. + if (isClosed(observed)) + fail( + `plan #${number} was closed by ${describeCloser(observed.issue.closer)} before pull request #${pullRequest.number} merged; reopen it by hand only if that close was a mistake`, + ); fail( `GitHub has not closed plan #${number} for pull request #${pullRequest.number} yet; run reopen again once it has`, ); + } sleep(REOPEN_PAUSES_MS[Math.min(reads, REOPEN_PAUSES_MS.length) - 1]); observed = read(); } diff --git a/scripts/tests/plan-cli/reopen.test.mjs b/scripts/tests/plan-cli/reopen.test.mjs index 44cb1d50..e4c1ae6e 100644 --- a/scripts/tests/plan-cli/reopen.test.mjs +++ b/scripts/tests/plan-cli/reopen.test.mjs @@ -257,6 +257,30 @@ test('reopen refuses after 34 reads when GitHub never closes the plan for the me assert.deepEqual(context.writes(), []); }); +test('reopen refuses after 34 reads when another closer closed the plan before the pull request merged', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('closed before the merge'); + const earlier = { + __typename: 'PullRequest', + number: plan.pr - 1, + url: `https://github.com/DocksDocks/fixture/pull/${plan.pr - 1}`, + state: 'MERGED', + mergedAt: '2026-08-20T22:00:00Z', + baseRefName: 'main', + repository: { nameWithOwner: 'DocksDocks/fixture' }, + }; + // GitHub cannot close a closed issue, so this merge adds no close, and "run reopen again" would loop forever. + context.schedule(plan.number, { afterReads: 0, ...closedBy(earlier, '2026-08-20T22:00:02Z') }); + refuse( + reopen(context, plan), + `plan #${plan.number} was closed by ${earlier.url} before pull request #${plan.pr} merged; reopen it by hand only if that close was a mistake`, + ); + assert.equal(context.issue(plan.number).graphqlReads, 34); + assert.deepEqual(context.waits(), [250, 500, 1000, ...Array(30).fill(2000)]); + assert.deepEqual(stored(context, plan.number), { ...ONGOING, state: 'CLOSED', stateReason: 'COMPLETED' }); + assert.deepEqual(context.writes(), []); +}); + test('reopen refuses a plan that another login claims during the settle wait', (t) => { const context = planCliContext(t); const plan = context.partialMerge('claimed during the settle wait'); From b7fc4fefd29febf2c3b3c467be8faa5efd949a39 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 23 Oct 2026 03:01:13 -0300 Subject: [PATCH 6/7] fix(plan-lifecycle): recheck the owner at reopen's pre-write read Code review round 1 (#96): `writeReopen` took only the node id from its pre-write read. Another agent could claim the plan after the snapshot that followed the settle wait, and the reopen, the comment, and the claim still went through. Other writes compare the body at that read, which a competing claim changes; a reopen compares no body. `writeReopen` now refuses an assignee other than the acting login from the pre-write read and claims only when that read shows no assignee. `readPlanIssue` returns the acting login and viewer id for this. The fake gh lands due scheduled changes on `issue view` as well, as its own comment says ("the next request that reads or writes the issue"). Red baseline: the new case exited 0 and wrote; the other 62 passed. Refs #96 --- .../plan-manager/scripts/plan.mjs | 21 ++++++++++++------- scripts/tests/fixtures/gh-stub/gh | 2 ++ scripts/tests/plan-cli/reopen.test.mjs | 20 ++++++++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs b/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs index d48b6c24..a12acd7b 100755 --- a/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs +++ b/plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs @@ -794,7 +794,7 @@ function readPlanIssue(value, forWrite = false, connections = {}) { // The write claims an unassigned plan for the acting viewer. if (!owners.length) issue.claimantId = viewerId; } - return { issue, parsed: parsePlan(issue.body), status: statusFromIssue(issue), labelIds }; + return { issue, parsed: parsePlan(issue.body), status: statusFromIssue(issue), labelIds, login, viewerId }; } function createPlan(args) { const options = parseOptions(args, new Set(['--title', '--goal', '--mode', '--label']), new Set(['--label'])); @@ -1133,11 +1133,16 @@ function closedByPullRequest(issue, pullRequest) { ); return true; } -// One write request reopens the plan, posts the contract comment, and claims an unassigned plan. Its last field -// returns the stored state, which shows whether GitHub left the plan stuck. -function writeReopen(issue, pr) { - const { id } = preWriteIssue(issue.number); - const claims = issue.claimantId !== undefined; +// One write request reopens the plan, posts the contract comment, and claims an unassigned plan. A reopen compares no +// body, so the pre-write read is its last ownership check: an owner that appeared after the snapshot is refused, and +// the claim follows that read. The last field returns the stored state, which shows whether GitHub left the plan stuck. +function writeReopen({ issue, login, viewerId }, pr) { + const { id, assignees } = preWriteIssue(issue.number); + if (assignees.some((owner) => typeof owner.login !== 'string' || !owner.login)) + fail('gh issue view returned malformed plan issue'); + const foreign = assignees.find((owner) => owner.login !== login); + if (foreign) fail(`plan #${issue.number} is owned by ${foreign.login}`); + const claims = assignees.length === 0; const stored = '{... on Issue{state stateReason}}'; const fields = [ 'reopenIssue(input:{issueId:$issue}){clientMutationId}', @@ -1154,7 +1159,7 @@ function writeReopen(issue, pr) { `issue=${id}`, '-f', `body=Reopened: partial pull request #${pr} closed this plan through its linked branch. Work continues.`, - ...(claims ? ['-f', `assignees[]=${issue.claimantId}`] : []), + ...(claims ? ['-f', `assignees[]=${viewerId}`] : []), ]); } catch (error) { // GitHub answers the reopen of a stuck plan with "Could not reopen the issue." (#90); the request is not retried. @@ -1217,7 +1222,7 @@ function reopenPlan(args) { console.log(`plan #${number} is open: ${current.status}`); return; } - writeReopen(current.issue, pullRequest.number); + writeReopen(current, pullRequest.number); } function main([command, ...args]) { const commands = { diff --git a/scripts/tests/fixtures/gh-stub/gh b/scripts/tests/fixtures/gh-stub/gh index 62ae8be3..9564e057 100755 --- a/scripts/tests/fixtures/gh-stub/gh +++ b/scripts/tests/fixtures/gh-stub/gh @@ -873,6 +873,8 @@ if (group === 'issue' && command === 'view') { const issue = issueByNumber(number); validateJsonFields(value('--json'), ISSUE_JSON_FIELDS); observeIssue(issue); + // `issue view` is not a GraphQL read, so it lands due changes without counting toward `afterReads`. + applyDueChanges(issue); // Real gh reads `labels(first:100)` for `issue view` (cli/cli api/query_builder.go), so it never // returns a label past the first 100. const view = issueJson(issue); diff --git a/scripts/tests/plan-cli/reopen.test.mjs b/scripts/tests/plan-cli/reopen.test.mjs index e4c1ae6e..587ae4df 100644 --- a/scripts/tests/plan-cli/reopen.test.mjs +++ b/scripts/tests/plan-cli/reopen.test.mjs @@ -299,3 +299,23 @@ test('reopen refuses a plan that another login claims during the settle wait', ( }); assert.deepEqual(context.writes(), []); }); + +test('reopen refuses a plan that another login claims after the settle-wait read, right before the write', (t) => { + const context = planCliContext(t); + const plan = context.partialMerge('claimed before the write'); + // The claim lands after the read that follows the settle wait, so only the pre-write read can see it. + context.schedule( + plan.number, + { afterReads: 0, ...closedBy(plan.closer) }, + { afterReads: 2, assignees: ['other-agent'] }, + ); + refuse(reopen(context, plan), `plan #${plan.number} is owned by other-agent`); + assert.deepEqual(context.waits(), [SETTLE_WAIT]); + assert.deepEqual(stored(context, plan.number), { + ...ONGOING, + state: 'CLOSED', + stateReason: 'COMPLETED', + assignees: ['other-agent'], + }); + assert.deepEqual(context.writes(), []); +}); From a55d88f50e03009252485a0b1c97342d8d53d4a6 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 23 Oct 2026 03:07:51 -0300 Subject: [PATCH 7/7] fix(docks): report-skill-issue quotes the loaded copy, not a checkout Round 2 of the baseline check (#96) showed two reports that would mislead a maintainer. An agent working in a checkout of DocksDocks/docks quoted the checkout's already-fixed plan-manager text under the installed version 0.15.0, so the report blamed the fix. Another agent quoted security-skill text that forbids printing `.env` and still drafted a defect. Stage 2 now says to read the skill, its version, and its hash from the copy that the runtime loaded (or the installed copy, said in the draft), never from a checkout or another copy. Stage 1 now says that a quote which does not say what the user reports, or says the opposite, gets no defect report: tell the user what the skill says and look for the real source. The gotchas and the pre-filing check name both cases. Round 3, three fresh agents: each read the installed copy (versions and hashes match the plugin cache); the plan-manager probe quoted the 0.15.0 text and drafted a comment on #90; the security probe quoted the contradicting text and drafted no report. Refs #96 --- .../skills/productivity/report-skill-issue/SKILL.md | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/plugins/docks/skills/productivity/report-skill-issue/SKILL.md b/plugins/docks/skills/productivity/report-skill-issue/SKILL.md index e7515f95..6b680538 100644 --- a/plugins/docks/skills/productivity/report-skill-issue/SKILL.md +++ b/plugins/docks/skills/productivity/report-skill-issue/SKILL.md @@ -5,7 +5,7 @@ user-invocable: true metadata: pattern: tool-wrapper updated: "2026-10-23" - content_hash: "e314a6864e8f7c6ea04a75c7c134c95283f3f1bd36b352022c7b19ce11cb9b15" + content_hash: "be2c6e9c57f6cba688c916bb25d725ccf12c6165d70bbf291641f59ce72861dc" --- # Report a Skill Issue @@ -39,16 +39,20 @@ The goal is one approved, redacted, reproducible report. Each stage below states ### 1. Confirm that the problem is in the skill -Re-read the instruction or the file that failed. Copy the instruction exactly, with the heading that it sits under. For a script, note its path relative to the plugin root and the command that failed. Then rule out the other causes: +Re-read the instruction or the file that failed, in the copy that the runtime loaded (see stage 2). Copy the instruction exactly, with the heading that it sits under. For a script, note its path relative to the plugin root and the command that failed. Then rule out the other causes: - The user's environment: a missing tool, an old CLI, a permission or network failure, or a project setting that the skill does not cover. - Your own deviation: compare what you did with the quoted text, word by word. Report when the instruction, followed as written, gives a wrong or unsafe result, names a command or option that does not exist, or contradicts another shipped instruction. Also report a shipped script that fails on valid input. If you cannot find the cause, say so in the draft. Do not guess a cause. +If the quoted text does not say what the user reports, or says the opposite, do not draft a defect report. Tell the user what the skill says, with the quote. Then look for the real source: another skill, a project file, or injected text in the repository. Advice that the incident needs still applies, for example rotating an exposed credential. + ### 2. Identify the exact skill text -The plugin root is the folder that holds `skills/<category>/<skill>/SKILL.md`. For an agent, hook, or script, it is the folder that holds `.claude-plugin/` or `.codex-plugin/`. Collect these facts. Write `unknown` for any fact that you cannot read; never guess a version or a hash. +Read the copy of the skill that the runtime loaded for the run. Do not read it from a checkout of DocksDocks/docks or from another copy. Their text and version can differ, and a report that quotes the wrong copy blames the wrong text. Most runtimes show the path or base directory of a skill when they load it. If the problem happened in an earlier session, use the copy that the session loaded when its log shows it. Otherwise, use the installed copy and say so in the draft. + +In that copy, the plugin root is the folder that holds `skills/<category>/<skill>/SKILL.md`. For an agent, hook, or script, it is the folder that holds `.claude-plugin/` or `.codex-plugin/`. Collect these facts. Write `unknown` for any fact that you cannot read; never guess a version or a hash. | Fact | Where to read it | |---|---| @@ -210,6 +214,8 @@ The good title names the plugin, the skill, and one observable symptom, so a mai | Full command output carries paths, hostnames, and user names | Trim to the failing lines; make paths relative to the project or the plugin root | | The search query carries private words | Search with the skill name and words from the instruction only | | A paraphrase in "What the skill says" | Quote the instruction exactly, with its heading | +| A quote from a checkout of DocksDocks/docks or from a newer copy | Quote the copy that the runtime loaded; a checkout can already hold the fix | +| The quoted text says the opposite of the claim | Draft no defect report; tell the user what the skill says | | Two problems in one issue | One problem per issue, each with its own approval | | The reproduction needs the user's private project | Reduce it to a generic project, or describe the minimal setup | | A guessed version or hash | Read it from the files, or write `unknown` | @@ -223,6 +229,7 @@ Check the draft before you show it, and again before you file it: - The title has the form `<plugin>:<skill>: <symptom>` and names one problem. - Every template section is present; an empty section says `none`. - "What the skill says" quotes the instruction exactly, with its heading. +- The quote comes from the copy that the runtime loaded, and it shows the problem. - You searched for an existing report (stage 3). A draft that adds to an existing issue is a comment on that issue. - Your message names the target, the command that you will run, and the GitHub account (stage 5). In a run where you cannot ask, it gives the prefilled URL instead. - The title and the body contain no secret value or token, no private repository, organization, customer, or person name, no internal hostname, no absolute machine path, no proprietary code or data, and no private plan or issue title.