Skip to content

Fix #90 and let agents report skill problems to this repository - #97

Merged
DocksDocks merged 7 commits into
mainfrom
96-fix-90-and-let-agents-report-skill-problems-to-this-repository
Oct 23, 2026
Merged

Fix #90 and let agents report skill problems to this repository#97
DocksDocks merged 7 commits into
mainfrom
96-fix-90-and-let-agents-report-skill-problems-to-this-repository

Conversation

@DocksDocks

@DocksDocks DocksDocks commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Yolo run: #96 Fix #90 and let agents report skill problems to this repository

Pull request: #97. Checks: all 5 pass. Reviewed head: a55d88f5.

What changed

  • Partial-merge reopen (plan-lifecycle: reopen right after a partial merge can leave the issue closed with reason REOPENED #90): a new helper command, reopen <n> --pr <pr>, replaces the hand reopen after a partial merge. It checks that the pull request merged and closes the plan. It reads the plan until GitHub records the close by that pull request or one of its commits, waits 10 s, reads again, and rechecks the owner before the write. One GraphQL request then reopens the plan, posts the contract comment, and claims an unassigned plan. A plan that is open again, a close by someone else before or after the merge, a change during the wait, a new owner, and the stuck state each stop with their own message. show advises on an issue that GitHub left closed with reason REOPENED, and the contract records the fallback. (plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs, references/plan-contract.md, SKILL.md)
  • Skill problem reports: the new docks skill report-skill-issue drafts one reproducible issue for this repository. The draft names <plugin>:<skill>, the plugin version, the skill's date and hash prefix, the runtime, and the exact instruction. The skill 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 link that a script builds. Private names, secrets, and machine paths stay out, and a security problem never goes public. (plugins/docks/skills/productivity/report-skill-issue/SKILL.md)
  • Report section in every skill: all 30 shipped skills of both plugins end with one byte-identical section that points to the new skill. Without docks installed, an agent only drafts for the user. plugins/docks/skills/AGENTS.md owns the rule and its verify command, and both READMEs list the skill.
  • Test contract registry: scripts/tests/test-contracts.mjs derives each suite's runner from the gate and requires the matching toolchain token. The registry lists bun-1 for the 16 Bun suites and bun-1 plus node-24 for repo-plan-cli. (scripts/config/test-contracts.json)
  • Tests: 15 new plan-CLI cases. A test-only --import hook skips and logs the waits, so the 10 s settle wait is asserted without slowing the suite. The fake gh gains close times, scheduled changes, the pull request query, and the three mutations. (scripts/tests/plan-cli/, scripts/tests/fixtures/gh-stub/gh)

Decisions and corrections

  • The helper waits for evidence of the close and then a fixed 10 s. A fixed sleep alone does not prove the close, and a wait computed from the event time depends on the local clock (Research decision 1).
  • One request carries the reopen, the comment, and the claim, so the claim lands in the same write. If GitHub rejects the reopen, the other two fields still run; the helper then prints the stuck refusal. This is kept by design (self-review).
  • Self-review found that a plan closed before the merge got "run reopen again once it has", which loops forever, because GitHub cannot close a closed issue. That plan now gets a refusal that names the earlier closer (commit 6a3ef0e8).
  • Code review round 1 found that the reopen did not recheck the owner at its pre-write read, where other writes compare the body. It now refuses a new owner there and claims only an unassigned plan (commit b7fc4fef).
  • Correction: the a12 baseline check of the report skill was first recorded as passing in round 2, and it did not pass. One probe quoted the development checkout's already-fixed text under the installed version, so its report blamed the fix. Another drafted a defect that its own quote refuted. The skill now quotes the copy that the runtime loaded and drafts no defect report when the quote refutes the claim (commit a55d88f5). Round 3 passed, and Fix #90 and let agents report skill problems to this repository #96 Verification Results records the correction.
  • Reports go through a skill and one shared section, not an issue template, because an agent sees a skill's section exactly when it uses the skill. Private vulnerability reporting is off in this repository, so the skill tells the user to send a security draft privately to the owner.
  • Request scope: "check open issues" found one open issue, plan-lifecycle: reopen right after a partial merge can leave the issue closed with reason REOPENED #90, which this pull request fixes. Probe: plan.mjs GraphQL write path (#89) #91 to Probe: plan.mjs label removal by snapshot id (#89) #94 are Optimize the gate and the plan helper after the Bun switch #89's closed probe issues.
  • Corrections to the Cut the gate to 7 s and a gh round trip from each plan write #95 run summary are recorded in Fix #90 and let agents report skill problems to this repository #96 Research: the kept remote branch, two behavior changes, and the release follow-up.

Reviews

Plan review passed in round 2. Code review passed in round 3.
Open findings: none. Round 1's MEDIUM finding, the missing owner recheck at the pre-write read, is fixed in b7fc4fef.

Verification

  • bun scripts/ci.mjs and bun scripts/tests/ci-plugin-targeting.mjs exit 0. node scripts/tests/plan-cli.mjs passes 63 of 63, and plan.mjs line coverage is 99.68%.
  • Red baselines, each recorded before its fix: the reopen spec (13 failed, 48 passed), the registry check (16 failed, 1 passed), the plan closed before the merge, and the claim before the write.
  • Real GitHub with the branch helper: show 89 succeeds with one query. reopen 94 --pr 95 reports that Probe: plan.mjs label removal by snapshot id (#89) #94 is not among the closing references of Cut the gate to 7 s and a gh round trip from each plan write #95, and exits 0 after one query and no write; Probe: plan.mjs label removal by snapshot id (#89) #94 is unchanged.
  • A temporary worktree that lists only bun-1 for repo-plan-cli makes the registry check fail, naming only that contract.
  • Report skill: the 30 sections hash to one value, and the skill scores 15. In a trigger check, 9 of 9 report requests loaded the skill and 9 of 9 near-misses went elsewhere, including the held-back thirds. With the skill, three fresh agents met every applicable assertion after two revisions. They ran the duplicate search, used every section, asked for approval first, and left out private names. They also quoted and hashed the installed copy and drafted no defect report when the quoted text refuted the claim. A dry run on plan-lifecycle: reopen right after a partial merge can leave the issue closed with reason REOPENED #90's facts produced a complete redacted draft, and its link body equals the shown draft byte for byte.

Limits and failures

  • The fix reaches users with the plan-lifecycle release, and the skill with the docks release (After merge).
  • The issue that plan-lifecycle: reopen right after a partial merge can leave the issue closed with reason REOPENED #90 describes stays stuck until GitHub Support repairs it. This change prevents new cases.
  • The reopen path is tested with a fake gh and the wait hook. On real GitHub only the "does not close" path ran, because a real partial merge would close and reopen a real plan.
  • The behavioral checks used one model as a proxy for agents in every runtime.

Kept resources

  • After Merge now: the remote branch 96-fix-90-and-let-agents-report-skill-problems-to-this-repository, because the repository does not delete merged branches and this run deletes no remote branch. The remote branches of earlier plans also stay. The review scratch docks-review/yolo-96 and the round files stay in the main checkout's git directory. No worktree or process remains.
  • After Leave pull request open: also the local branch of the same name, and the open pull request and issues.

After merge

  • Step 5: release docks (minor, 0.21.0), with its own question.
  • Step 6: release plan-lifecycle (minor, 0.16.0), with its own question.

Closes #96
Closes #90

@DocksDocks
test: require each test contract to name its gate runner
6bbf8b3 
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-<major> from the
workflow's node-version and bun-<major> 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
@DocksDocks
fix(plan-lifecycle): wait for GitHub's close before a partial-merge r…
347f0a5 
…eopen

#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 <n> --pr <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
@DocksDocks
feat(docks): add report-skill-issue and a report section in every skill
8542d79 
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
@DocksDocks
fix(docks): build the report-skill-issue link with a script
093c13f 
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
@DocksDocks
fix(plan-lifecycle): refuse reopen for a plan closed before the merge
6a3ef0e 
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
@DocksDocks
fix(plan-lifecycle): recheck the owner at reopen's pre-write read
b7fc4fe 
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
@DocksDocks
fix(docks): report-skill-issue quotes the loaded copy, not a checkout
a55d88f 
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
@DocksDocks DocksDocks linked an issue Oct 23, 2026 that may be closed by this pull request
Fix #90 and let agents report skill problems to this repository #96
Closed
@DocksDocks DocksDocks mentioned this pull request Oct 23, 2026
Closed
@DocksDocks
DocksDocks merged commit d71bb56 into main Oct 23, 2026
5 checks passed
@DocksDocks
DocksDocks deleted the 96-fix-90-and-let-agents-report-skill-problems-to-this-repository branch October 23, 2026 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

1 participant

@DocksDocks