Skip to content

Cut the gate to 7 s and a gh round trip from each plan write - #95

Merged
DocksDocks merged 5 commits into
mainfrom
89-optimize-the-gate-and-the-plan-helper-after-the-bun-switch
Oct 23, 2026
Merged

Cut the gate to 7 s and a gh round trip from each plan write#95
DocksDocks merged 5 commits into
mainfrom
89-optimize-the-gate-and-the-plan-helper-after-the-bun-switch

Conversation

@DocksDocks

@DocksDocks DocksDocks commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Yolo run: #89 Optimize the gate and the plan helper after the Bun switch

Pull request: #95. Checks: all 5 pass. Reviewed head: 1b2fe575.

What changed

  • Unit suite: the two blocking background-task tests sleep instead of busy-looping, use shorter windows with the same margins, and sit in their own registered files. The vacuity files start all their gates at module load. (scripts/tests/unit/, scripts/config/test-contracts.json)
  • Plan-CLI suite: the 44 cases became node:test tests, each with its own scratch repository and fake-gh state, in 9 files that Node's runner executes in parallel. scripts/tests/plan-cli.mjs stays the entry, so the gate, the targeting contracts, and the registry do not change. A new case pins the list order. (scripts/tests/plan-cli/, scripts/tests/plan-cli.mjs)
  • Shipped plan.mjs: each write sends one GraphQL request instead of gh issue edit plus a read-back. updateIssue carries the body and any claim, and discrete label mutations follow. list uses one paged query. Writes go from 4 gh calls to 3, retire from 5 to 4, and list from 2 to 1. Refusal strings and the contract do not change. (plugins/plan-lifecycle/skills/productivity/plan-manager/scripts/plan.mjs)
  • Fake gh: it now mirrors gh's -F handling (an @ file is sent verbatim; other values are converted and get placeholders filled) and gh's 100-label issue view. (scripts/tests/fixtures/gh-stub/gh)

Decisions and corrections

  • The plan body travels as -F body=@<file>. gh fills {owner}, {repo}, and {branch} in any other -F value, so a body that quotes them would be stored rewritten (cli/cli pkg/cmd/api/fields.go).
  • Labels use discrete add and remove mutations, not a full labelIds list. A full list would drop a label that someone adds at the same time, and gh avoids this on purpose (cli/cli pkg/cmd/pr/shared/editable_http.go).
  • Request correction: the gate's check scheduling in scripts/ci.mjs and the workflow stay as they are. Overlapping checks would save only about 2 to 4 s more and would touch the timing and targeting contracts.
  • Rejected: Node's compile cache (25.7 ms against 25.0 ms per plan.mjs start).

Reviews

Plan review passed in round 3. Rounds 4 and 5 passed again after two one-sentence changes to Research. Code review passed in round 2.
Open findings: none. Round 1's two MEDIUM findings had one root cause, label removal IDs taken from the 100-label gh issue view, and commit 1b2fe575 fixes it.

Verification

  • Gate time, origin/main against this branch, both on Bun (medians of 3 interleaved runs, the gate's own total_ms):
Gate origin/main Branch Change
--plugin plan-lifecycle 12.92 s 2.54 s -80.4%
--lane repo 9.33 s 4.44 s -52.4%
full gate 22.41 s 7.25 s -67.7%

Limits and failures

Kept resources

After merge

  • none

Closes #89

@DocksDocks
test: cut the unit suite's critical path from 6.4 s to 1.7 s
1d2b35e 
ci-background-task.test.mjs held the whole suite's wall time: its two
blocking tests ran a 2.5 s and a 3 s busy loop one after the other
inside a single worker, so --parallel=4 could not help (#89).

- The blockers now sleep. spawnSync already freezes the parent's event
  loop, so a busy loop adds nothing but CPU starvation of the measured
  child (the flake in 692c061).
- The windows shrink with their margins kept: a 300 ms child behind a
  1000 ms block (tolerance 250 ms), and a 1500 ms block with a 750 ms
  threshold for the 8 MiB writer, which took at most 190 ms under
  4-core load. A startTask mutant that relays the child's pipes and
  times the await still fails both tests.
- The two tests move to their own registered files so the parallel
  runner can place them on different workers.
- The vacuity files start every gate at module load and await each
  result, instead of running 7 and 4 gates one after another.

Measured on 4 cores: 6.43 s -> 1.72 s, 72 of 72 tests passing.

Refs #89
@DocksDocks
test: run the plan-CLI cases in parallel through node --test
5efaf4e 
scripts/tests/plan-cli.mjs ran about 32 cases in one process against
one shared fake-gh state and one scratch repository, so its 198
plan.mjs starts ran strictly in series: 12.4 s, about 95% of each
plugin gate (#89).

Each case is now a node:test test with its own context (scratch repo,
fake-gh state seeded with the plan labels, environment), built by
scripts/tests/plan-cli/harness.mjs. The 44 cases sit in 9 files
balanced by measured cost, and Node's runner gives each file its own
process. The frozen-steps case and the status-transitions case that
reused its plan are one test.

scripts/tests/plan-cli.mjs stays the entry, so the gate invocation,
the targeting call counts, the test-contract registry, and the
validator-table row do not change; it runs the case files with
--test-concurrency set to the available parallelism. plan.mjs still
runs on Node through process.execPath.

A new case pins the contract's list order (open plans first, then
closed, each ascending) over a mixed state. The shared state used to
exercise that path only by accident; plan.mjs line coverage stays at
996 of 1006 lines.

Measured on 4 cores: 12.4 s serial -> 3.9 s.

Refs #89
@DocksDocks
perf(plan-lifecycle): cut a gh round trip from every plan write and list
267482e 
Every plan.mjs write ran four gh processes: the snapshot, a pre-write
`gh issue view`, `gh issue edit`, and a read-back `gh issue view`.
`list` ran a repository lookup and then `gh issue list` (#89).

- A write now sends one `gh api graphql` request. Its first mutation,
  updateIssue, carries the body and any claim, so the body and the
  claim succeed or fail together. Label repair follows as discrete
  add and remove mutations: a full labelIds list would drop a label
  added concurrently, which gh avoids for the same reason. The last
  mutation returns the stored state, which replaces the read-back.
- The pre-write read stays: the contract requires it and a case tests
  the race. Retire still closes only after its label write checks out.
- The body travels as `-F body=@<file>`. gh fills `{owner}`, `{repo}`,
  and `{branch}` in any other -F value, so a quoted token would be
  stored rewritten; an @ file is sent byte for byte.
- `list` reads the repository's plan issues in one paged query.
- A status write on a repository that lacks a phase label now creates
  it, as `new` does, instead of failing inside `gh issue edit`.

On GitHub: a status write takes about 2.8 s instead of 3.2 s, and
`list` about 0.6 s instead of 1.3 s. Refusal strings and the contract
are unchanged. The fake gh now mirrors gh's -F handling (an @ file is
verbatim, other values are converted and placeholder-filled), so the
plan-CLI suite catches a body sent the wrong way.

Refs #89
@DocksDocks
fix(plan-lifecycle): close the three self-review findings on #89
21e0fae 
- The write mutation no longer selects the stored assignees. No check
  reads them, so the field only added a way for a write to fail.
- REPOSITORY_LABELS stays a joined string, as before.
- sharedFilesAdvice and printAdvice get their original formatting back;
  the change does not touch them, and the gate does not format this
  shipped file.

Refs #89
@DocksDocks
fix(plan-lifecycle): take label removal ids from the paged snapshot
1b2fe57 
Code review round 1 on #89: the write path took the label names to
remove from the snapshot, which pages every label, but took their ids
from `gh issue view`, which reads only labels(first:100). A phase label
past the first 100 got no removal id: `status` wrote the body, kept
both phase labels, and refused afterwards; `archive` and `retire` on
an owned plan returned early without the label check, so archive
reported success and retire closed the issue with the label attached.
The released helper removed by name through `gh issue edit`, so this
was a regression.

- The snapshot now reads label ids with the names, and the removal ids
  come from that complete list. The pre-write read drops labels.
- The early return for "nothing left to change" is gone: names and ids
  now come from one list, so it could not be reached, and it was the
  path that skipped the label check.
- The fake gh truncates `issue view` labels at 100, as real gh does.
  With that, the paged-label status case and a new archive case fail
  on the previous commit and pass now.

Refs #89
@DocksDocks DocksDocks linked an issue Oct 23, 2026 that may be closed by this pull request
Optimize the gate and the plan helper after the Bun switch #89
Closed
@DocksDocks DocksDocks mentioned this pull request Oct 23, 2026
Closed
@DocksDocks
DocksDocks merged commit e49e144 into main Oct 23, 2026
5 checks passed
This was referenced Oct 23, 2026
Closed
Merged
@DocksDocks
DocksDocks deleted the 89-optimize-the-gate-and-the-plan-helper-after-the-bun-switch 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

Development

Successfully merging this pull request may close these issues.

Optimize the gate and the plan helper after the Bun switch

1 participant

@DocksDocks