feat(workbench)!: abstract the code forge behind a contract - #7
Conversation
Workbench abstracts the tracker but hardcodes GitHub. `gh` and GitHub-specific concepts are called directly from shared contract files, so a non-GitHub forge cannot be added by writing an adapter — it requires editing the shared contracts. On a machine without `gh`, preflight stops the run before any work begins, in both approval modes. This adds the contract and its adapters. Nothing is rewired to call them yet, so behavior is unchanged. - Add `forges.md`: five operations (`verifyForge`, `resolveBase`, `openReview`, `publishReview`, `getReviewState`) and a declared capability table, mirroring the existing tracker abstraction - Add three capability tiers so an unknown forge degrades to a manual handoff instead of hard-stopping preflight - Establish the **adapter or nothing** rule: anything the agent executes against a forge comes from a written adapter file, never from an inference made in the moment - Add `forges/github.md` holding every `gh` invocation currently inline; behavior-preserving - Add `forges/generic-git.md` as the tier-3 fallback, declaring `reviewLookup: none` and stating that issues will not auto-close - Add `forges/TEMPLATE.md` so a team can support an internal forge via a repo-local `.workbench/forge.md` without forking this plugin - Move base-branch resolution out of `trackers.md`; it is a forge concern - Add the design doc, the implementation plan, and the portability feedback they respond to - Track the 37 beads issues seeded from the plan Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The done-on-merge sweep records an abandoned review by pushing a marker to the base branch. Most shared repos protect `main`, so that push fails and the marker never lands — which means the same dead review is reported on every invocation, as a stop that fires in both approval modes. Moving that marker to a comment on the tracker issue removes the need for base-branch write access, but `getIssue` does not return comments. This adds the operation that makes the move possible. The sweep itself is rewired in a follow-up commit. - Add `listComments(ref)` as the ninth tracker contract operation, scoped to reading markers this plugin wrote rather than to summarizing discussion - Map it in the Asana adapter, filtering to comment-type stories so system stories for field changes and section moves cannot match a sentinel - Map it in the Linear adapter, reading comments from the `getIssue` response on builds that return them inline rather than making a second call - Define unavailability as a documented fallback rather than an error, following the fallback-chain precedent already in the Asana adapter Tool coverage is not yet confirmed against live MCPs; tracked as wb-y3n.7.4 and wired as a blocker on the pre-release read-through. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`trackers.md` carried 27 GitHub references and called `gh` directly from shared contract prose. It now has none: preflight, the sweep, and first-run setup all go through the forge contract, and a non-GitHub forge no longer requires editing this file. Two live defects are fixed along the way. - Preflight resolves the forge adapter and calls `verifyForge` instead of running `gh auth status`. An unverified forge selects a capability tier rather than stopping the run; only the tracker MCP still gates the start - Rebuild the done-on-merge sweep on `getReviewState(id)` per recorded review, replacing branch-name matching against a listed set. This removes the pagination hazard where a listing bound silently dropped the oldest records once a repo outgrew it. Legacy branch-only records fall back and are rewritten with an id so the fallback drains - **Delete the stamp push.** Recording a merge required pushing a marker to the base branch, which fails on any protected branch — so the marker never landed and the issue was re-swept forever. Tracker state is now the sole idempotency mechanism for the merged path, and the sweep no longer writes or pushes anything - Move the closed-unmerged marker to a tracker comment carrying a `workbench: review-closed-unmerged` sentinel. This path had no tracker state to key on, so on a protected-branch repo it re-asked about the same dead review on every run, as a stop that fires in both approval modes - Gate the merge-closer question on the forge's `ciHooks` capability, so a forge with no CI hooks records `none` instead of installing a workflow that never runs and asserting a capability it lacks - Add the forge question to first-run setup, ordered before merge-closer since the gating depends on it - Move base-branch resolution to `forges.md`; it is a forge concern BREAKING CHANGE: the sweep now prefers a recorded review id over a branch name, and the closed-unmerged marker moves from a pushed file line to a tracker comment. Existing `.workbench/` records keep working — branch fallback and legacy marker lines are both still read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Removes the last `gh` invocation from a skill body. No file outside `forges/github.md` now names a forge CLI, and "pull request" survives only inside GitHub-scoped sections where it describes real GitHub behavior. - Rewrite `execute` step 11 to call `openReview` then `publishReview` instead of pushing and shelling out to `gh`. It honors `pushesForYou`, since a forge whose CLI pushes the review ref itself produces a wrong branch state when the caller pushes too - Record `- Review: <id> <url>` at open so the sweep can look the review up by id; add the field to the checklist format with `PR` kept as a read-only legacy line - Add an explicit manual-tier path: no review is opened, the handoff is printed, and the run says plainly that nothing will auto-close the issue - Refuse to act on a cleanup phrase when `reviewLookup` is `none`. The claim cannot be confirmed, and a wrong close is what confirmation exists to prevent - Delete `scaffold`'s forge gate. It creates tracker issues and never opens a review, so gating it on `gh` was a bug rather than a portability issue - Gate both tracker adapters' merge-closer offers on `ciHooks`, and scope Linear's GitHub-integration logic to the GitHub forge - Narrow the never-skipped preflight stop to the tracker MCP, and add accepting a probed forge CLI as a stop auto mode cannot skip - Name the resolved adapter's CLI in the permissions allowlist - State in `conventions.md` that every forge reviews committed work only, which is the universal hazard the commit reconciliation guards Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Add a **Forges** section covering the five-operation contract, the three capability tiers, and how to support a forge workbench does not ship by copying the bundled template to `.workbench/forge.md` - State the manual tier's consequence plainly: nothing observes the review, so no run closes the issue and that is yours to do - Rewrite "How the tracker learns a PR merged" around the `ciHooks` capability instead of three GitHub arrangements - Move `gh` from an unconditional prerequisite to a GitHub-specific one in both the README and the guide - Add the forge question to the first-run table and `forge` to the profile - Note in troubleshooting that the repeating abandoned-review report was a real defect caused by the protected-branch marker push, and is fixed - Record that repos set up before forge support need no migration Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Bump the plugin to 1.1.0 and propagate via `bin/sync-versions.sh` - Add `forge` and `code-review` keywords; update the plugin description - Reconcile the two SkillSpector suppression reasons that referenced the merge-closer Action, which is now gated on the forge's `ciHooks` - Fix hand-written PR references the version sync does not regenerate, and add a forge-portable feature bullet Read-through verified: no `gh` outside `forges/github.md`, all five forge and nine tracker operations resolve to a contract, all five capabilities are declared in every adapter, and no cross-file reference dangles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SkillSpector v2.5.1 run against all three skills: zero active findings. The new PATH-probing and forge-CLI prose introduced none, and all nine baseline suppressions still match live findings, so none went stale despite the Phase C deletions. No baseline changes were needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A maintainer-facing walkthrough of everything on this branch, written to be the source for the PR description. - Measures the result: real `gh` invocations 7 -> 0 outside the new GitHub adapter, "pull request" 32 -> 0 in shared contracts - Explains the one counter-intuitive number: "GitHub" mentions rose in the tracker adapters because implicit coupling became explicit scoping - Separates the two live defect fixes from the portability work, since those are the only changes that alter GitHub behavior - Splits behavior changes by audience: GitHub users, other forges, and Asana/Linear users who will now see a new tracker comment - States what was verified (SkillSpector, static read-through) and what was not (live-MCP `listComments` coverage, any end-to-end run) - Gives a suggested review order Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughWorkbench 1.1.0 adds forge-neutral review contracts, adapter tiers, tracker review reconciliation, manual fallbacks, and updated lifecycle documentation. The change also initializes Beads configuration, issue state, ignore rules, and Git hooks. ChangesForge portability
Beads repository setup
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
skills/workbench-shared/memory/checklist.md-32-35 (1)
32-35: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUpdate
parentTaskto readReviewfirst.These lines make
Reviewthe primary record, but theparentTaskmapping at Line 49 still reads onlyPR. New files created byinitcontain noPRline. ReadReviewfirst and fall back to legacyPR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/workbench-shared/memory/checklist.md` around lines 32 - 35, Update the parentTask mapping to read the Review field first, using its recorded id and URL for new records, and fall back to the legacy PR field when Review is absent. Preserve legacy branch-matching behavior for records that only contain PR, while ensuring new records do not require or write PR.skills/workbench-shared/approval.md-48-50 (1)
48-50: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQualify the “once” claim for the fallback path.
The tracker-comment marker can make the stop fire once, but the documented file-marker fallback can repeat across clones or protected branches. State that the one-time behavior requires available comment listing, or provide a durable fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/workbench-shared/approval.md` around lines 48 - 50, Update the documentation around the abandoned-review stop behavior to qualify the “once” guarantee: it applies only when tracker comment listing is available and the marker can be recorded there. Explain that the file-marker fallback may repeat across clones or protected branches, unless a durable shared fallback is implemented.docs/plans/2026-08-02-forge-portability-changes.md-101-103 (1)
101-103: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language to both fenced blocks.
docs/plans/2026-08-02-forge-portability-changes.md#L101-L103: use```text.docs/plans/2026-08-02-forge-portability-design.md#L121-L123: use```text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/2026-08-02-forge-portability-changes.md` around lines 101 - 103, Specify the text language on both fenced blocks containing the review-closed-unmerged command: update docs/plans/2026-08-02-forge-portability-changes.md lines 101-103 and docs/plans/2026-08-02-forge-portability-design.md lines 121-123 to use text fences.Source: Linters/SAST tools
skills/workbench-shared/trackers/linear.md-21-21 (1)
21-21: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve comments returned by
getIssue.This fallback reads comments from the response already saved by
getIssue, but thegetIssuemapping does not require saving comment bodies. If comments are available only in that response, the sweep cannot find existing sentinels and will post duplicate abandonment comments. Require the adapter to retain comment bodies and their ordering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/workbench-shared/trackers/linear.md` at line 21, Update the getIssue mapping and its saved response representation to retain comment bodies and their ordering, so listComments(ref) can use comments returned by getIssue when no dedicated comments tool exists. Ensure the adapter requires and preserves each comment’s body in the order provided by the response.skills/workbench-shared/forges/TEMPLATE.md-6-7 (1)
6-7: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix references after copying the adapter.
After this file is copied to
.workbench/forge.md,../forges.md,github.md, andgeneric-git.mdresolve relative to.workbench/, not the plugin's shared adapter directory. Use references that the runtime agent can resolve after copying, or state that these links apply only before copying.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/workbench-shared/forges/TEMPLATE.md` around lines 6 - 7, Update the documentation references in TEMPLATE.md so they remain resolvable after the file is copied to .workbench/forge.md. Replace the relative links to forges.md, github.md, and generic-git.md with runtime-resolvable references, or explicitly mark them as pre-copy-only references..beads/issues.jsonl-19-19 (1)
19-19: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReconcile the validation status in this issue record.
Line 19 records
SCAN RUN ... PASSinnotes, butclose_reasonsaysScan itself NOT run..beads/interactions.jsonlLine 54 records the latter. Keep one verified result and update the stale field; otherwise the Beads state presents conflicting release-gate evidence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.beads/issues.jsonl at line 19, Reconcile the conflicting validation claims in issue record wb-y3n.7.1 by updating its notes or close_reason so both fields consistently reflect the single verified scan result recorded in .beads/interactions.jsonl. Preserve the accurate release-gate evidence and remove the stale claim that the scan was not run..beads/README.md-62-64 (1)
62-64: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winUse the current Beads installer URL.
The Beads installation instructions now target
gastownhall/beads/scripts/install.sh; thesteveyegge/beadspath is the former repository location. Update line 64 to the current project URL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.beads/README.md around lines 62 - 64, Update the Beads installation command in the “Install Beads” section to use the current gastownhall/beads installer URL instead of the former steveyegge/beads repository path, preserving the existing curl-and-bash invocation.README.md-38-38 (1)
38-38: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the bundled
generic-gitfallback consistently.The documentation requires a repo-local adapter for every non-GitHub forge, although Workbench ships a
generic-gitfallback.
README.md#L38-L38: state that GitHub andgeneric-gitare built-in adapters; reserve.workbench/forge.mdfor unsupported forges.README.md#L68-L68: exclude the bundled fallback from the repo-local adapter statement.docs/workbench.md#L42-L43: include the fallback in the setup requirements.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 38, Update README.md lines 38-38 to identify GitHub and generic-git as built-in adapters and reserve .workbench/forge.md for unsupported forges; update README.md lines 68-68 to exclude generic-git from the repo-local adapter requirement; update docs/workbench.md lines 42-43 to include generic-git in the setup requirements..claude-plugin/marketplace.json-15-15 (1)
15-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRefresh the marketplace description with version 1.1.0.
The version now advertises the forge-portable release, but the same marketplace entry still describes opening a pull request. Update the catalog description to match
.claude-plugin/plugin.jsonandREADME.md.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude-plugin/marketplace.json at line 15, Update the marketplace entry’s description alongside the version 1.1.0 change so it matches the forge-portable release wording in .claude-plugin/plugin.json and README.md, replacing the outdated pull-request description while preserving the existing catalog structure.docs/workbench.md-34-34 (1)
34-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winQualify the default workflow summaries by forge tier.
The overview and execution summary say that a run produces and opens a review. The manual tier only pushes the branch and hands off the base, title, and body. The assisted tier asks before using a candidate CLI. State the tier-dependent outcome in both summaries.
Also applies to: 212-212
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/workbench.md` at line 34, Update the workflow summaries in docs/workbench.md to qualify the review outcome by forge tier: describe the manual tier as pushing the branch and handing off the base, title, and body, while describing the assisted tier as requesting confirmation before using a candidate CLI to open the review. Apply this clarification to both the overview and execution summary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.beads/config.yaml:
- Line 68: Remove the tracked sync.remote setting from .beads/config.yaml so
each clone can configure its own authorized Dolt sync destination locally; do
not leave a shared cross-fork remote in version control.
In @.beads/hooks/post-checkout:
- Around line 1-33: Git is not currently connected to the Beads hook wrappers.
Configure Git’s core.hooksPath to .beads/hooks during setup, or install/copy the
wrappers into .git/hooks; apply this consistently for .beads/hooks/post-checkout
(anchor), .beads/hooks/post-merge, .beads/hooks/pre-commit,
.beads/hooks/pre-push, and .beads/hooks/prepare-commit-msg so Git invokes each
hook.
In `@docs/plans/2026-08-02-forge-portability-design.md`:
- Around line 85-105: Reconcile the Tier 2 behavior with the “Adapter or
nothing” rule by requiring a written, user-approved temporary adapter before
executing any probed forge CLI, or remove the one-time execution choice; update
both docs/plans/2026-08-02-forge-portability-design.md lines 85-105 and
docs/plans/2026-08-02-forge-portability-changes.md lines 121-147 consistently,
preserving the existing manual fallback and permanent .workbench/forge.md
option.
In `@docs/workbench.md`:
- Around line 201-202: Update the review-state behavior documentation around the
cleanup text, safety stop, lifecycle table, tier table, and sweep section to
condition confirmation and sweep execution on reviewLookup: by-id. Document that
adapters with reviewLookup: none, including the manual tier, cannot observe or
verify review state and therefore must not claim confirmation or run the
review-state sweep.
In `@skills/execute/SKILL.md`:
- Line 70: After selecting the base branch, explicitly call resolveBase before
openReview, using the resolved base value and preserving the existing fetch and
branch-creation flow. Ensure base validation occurs before creating the review
rather than relying on fetching the branch alone.
In `@skills/scaffold/SKILL.md`:
- Around line 42-43: Update the first-run setup instructions in the scaffold
skill to enumerate all six required ordered setup steps, placing forge
confirmation before merge-closer configuration. Add forge explicitly to the
checklist and require its profile field so tier resolution has the necessary
configuration.
In `@skills/workbench-shared/forges.md`:
- Around line 95-117: Resolve the Tier 2 contradiction by removing the direct
“use the candidate for this run” execution choice or requiring the user to
create and explicitly approve a written one-run adapter before any forge
operation. Update the Tier 2 and “Adapter or nothing” sections so candidate
detection remains suggestion-only and no unreviewed CLI is pushed or invoked.
- Around line 88-103: Define a safe fallback tier in
skills/workbench-shared/forges.md for a resolved adapter whose verifyForge()
check fails, including its permitted operations and handoff behavior; do not
treat it as Tier 1. Apply the same failed-verification rule during tracker
preflight in skills/workbench-shared/trackers.md, including the resulting
handoff behavior, and keep both documents consistent.
- Around line 17-18: Define one explicit manual-handoff contract across all
affected documentation: in skills/workbench-shared/forges.md lines 17-18, add a
typed manual-handoff result or move manual handoff outside the five-operation
adapter contract; in skills/workbench-shared/forges/generic-git.md lines 42-57,
classify generic-git as Tier 3 or document the explicit handoff result; and in
skills/execute/SKILL.md lines 104-113, skip publishReview and review-id
persistence when execution receives a manual-handoff result.
- Around line 25-26: Update skills/workbench-shared/forges.md lines 25-26 to
define a contract-supported legacy lookup operation or explicitly supported
legacy input that can resolve records without review ids; do not treat branch
names as review ids. Update skills/workbench-shared/trackers.md lines 83-94 to
use that operation when backfilling missing ids, or remove the unsupported
branch fallback and specify a safe alternative.
In `@skills/workbench-shared/memory/checklist.md`:
- Around line 32-35: Update the compatibility section to remove instructions for
appending “- Closed” markers. Describe legacy closed markers as read-only
compatibility data that may be read, while stating that new sweep processing
uses tracker state and tracker comments without writing markers.
In `@skills/workbench-shared/trackers.md`:
- Around line 182-193: Gate merge-closer setup on forge support for
GitHub-native integrations, not merely ciHooks: update
skills/workbench-shared/trackers.md lines 182-193 to require a merge-closer
capability or GitHub-compatible forge; update
skills/workbench-shared/trackers/asana.md lines 64-87 to withhold the Asana
GitHub Action on unsupported forges; and update
skills/workbench-shared/trackers/linear.md lines 27-29 to skip Linear’s GitHub
integration and Action for every non-GitHub forge.
---
Minor comments:
In @.beads/issues.jsonl:
- Line 19: Reconcile the conflicting validation claims in issue record
wb-y3n.7.1 by updating its notes or close_reason so both fields consistently
reflect the single verified scan result recorded in .beads/interactions.jsonl.
Preserve the accurate release-gate evidence and remove the stale claim that the
scan was not run.
In @.beads/README.md:
- Around line 62-64: Update the Beads installation command in the “Install
Beads” section to use the current gastownhall/beads installer URL instead of the
former steveyegge/beads repository path, preserving the existing curl-and-bash
invocation.
In @.claude-plugin/marketplace.json:
- Line 15: Update the marketplace entry’s description alongside the version
1.1.0 change so it matches the forge-portable release wording in
.claude-plugin/plugin.json and README.md, replacing the outdated pull-request
description while preserving the existing catalog structure.
In `@docs/plans/2026-08-02-forge-portability-changes.md`:
- Around line 101-103: Specify the text language on both fenced blocks
containing the review-closed-unmerged command: update
docs/plans/2026-08-02-forge-portability-changes.md lines 101-103 and
docs/plans/2026-08-02-forge-portability-design.md lines 121-123 to use text
fences.
In `@docs/workbench.md`:
- Line 34: Update the workflow summaries in docs/workbench.md to qualify the
review outcome by forge tier: describe the manual tier as pushing the branch and
handing off the base, title, and body, while describing the assisted tier as
requesting confirmation before using a candidate CLI to open the review. Apply
this clarification to both the overview and execution summary.
In `@README.md`:
- Line 38: Update README.md lines 38-38 to identify GitHub and generic-git as
built-in adapters and reserve .workbench/forge.md for unsupported forges; update
README.md lines 68-68 to exclude generic-git from the repo-local adapter
requirement; update docs/workbench.md lines 42-43 to include generic-git in the
setup requirements.
In `@skills/workbench-shared/approval.md`:
- Around line 48-50: Update the documentation around the abandoned-review stop
behavior to qualify the “once” guarantee: it applies only when tracker comment
listing is available and the marker can be recorded there. Explain that the
file-marker fallback may repeat across clones or protected branches, unless a
durable shared fallback is implemented.
In `@skills/workbench-shared/forges/TEMPLATE.md`:
- Around line 6-7: Update the documentation references in TEMPLATE.md so they
remain resolvable after the file is copied to .workbench/forge.md. Replace the
relative links to forges.md, github.md, and generic-git.md with
runtime-resolvable references, or explicitly mark them as pre-copy-only
references.
In `@skills/workbench-shared/memory/checklist.md`:
- Around line 32-35: Update the parentTask mapping to read the Review field
first, using its recorded id and URL for new records, and fall back to the
legacy PR field when Review is absent. Preserve legacy branch-matching behavior
for records that only contain PR, while ensuring new records do not require or
write PR.
In `@skills/workbench-shared/trackers/linear.md`:
- Line 21: Update the getIssue mapping and its saved response representation to
retain comment bodies and their ordering, so listComments(ref) can use comments
returned by getIssue when no dedicated comments tool exists. Ensure the adapter
requires and preserves each comment’s body in the order provided by the
response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a92644ee-8fde-4e91-b891-bcc97a654189
📒 Files selected for processing (35)
.beads/.gitignore.beads/README.md.beads/config.yaml.beads/hooks/post-checkout.beads/hooks/post-merge.beads/hooks/pre-commit.beads/hooks/pre-push.beads/hooks/prepare-commit-msg.beads/interactions.jsonl.beads/issues.jsonl.beads/metadata.json.claude-plugin/marketplace.json.claude-plugin/plugin.json.gitignore.skillspector-baseline.yamlREADME.mddocs/plans/2026-08-02-forge-portability-changes.mddocs/plans/2026-08-02-forge-portability-design.mddocs/plans/2026-08-02-forge-portability-plan.mddocs/plans/Workbench skill — forge portability feedback.mddocs/workbench.mdskills/execute/SKILL.mdskills/scaffold/SKILL.mdskills/workbench-shared/agents.mdskills/workbench-shared/approval.mdskills/workbench-shared/conventions.mdskills/workbench-shared/forges.mdskills/workbench-shared/forges/TEMPLATE.mdskills/workbench-shared/forges/generic-git.mdskills/workbench-shared/forges/github.mdskills/workbench-shared/memory.mdskills/workbench-shared/memory/checklist.mdskills/workbench-shared/trackers.mdskills/workbench-shared/trackers/asana.mdskills/workbench-shared/trackers/linear.md
Resolve the tier-2 rule so one-run acceptance and the written-adapter requirement no longer contradict, define the manual-handoff result and an optional findReviewByBranch operation in the forge contract, pin the failed-verifyForge case to the manual tier, gate merge-closer offers on the forge being GitHub rather than only ciHooks, call resolveBase before openReview in execute, correct the six-step setup enumeration in scaffold, drop the stale closed-marker sweep text from the checklist adapter, and sync docs, marketplace description, beads config and install URL accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/workbench-shared/approval.md`:
- Line 51: In the approval guidance text, replace the phrase “The once
guarantee” with “The one-time guarantee” while leaving the surrounding behavior
and explanation unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a60f21ec-f8ac-4a92-b1da-76b8ee8359f6
📒 Files selected for processing (19)
.beads/README.md.beads/config.yaml.beads/issues.jsonl.claude-plugin/marketplace.jsonREADME.mddocs/plans/2026-08-02-forge-portability-changes.mddocs/plans/2026-08-02-forge-portability-design.mddocs/workbench.mdskills/execute/SKILL.mdskills/scaffold/SKILL.mdskills/workbench-shared/approval.mdskills/workbench-shared/forges.mdskills/workbench-shared/forges/TEMPLATE.mdskills/workbench-shared/forges/generic-git.mdskills/workbench-shared/forges/github.mdskills/workbench-shared/memory/checklist.mdskills/workbench-shared/trackers.mdskills/workbench-shared/trackers/asana.mdskills/workbench-shared/trackers/linear.md
🚧 Files skipped from review as they are similar to previous changes (14)
- .beads/issues.jsonl
- .beads/README.md
- .claude-plugin/marketplace.json
- skills/workbench-shared/memory/checklist.md
- skills/execute/SKILL.md
- skills/workbench-shared/forges/generic-git.md
- skills/workbench-shared/trackers/asana.md
- docs/plans/2026-08-02-forge-portability-changes.md
- skills/workbench-shared/forges/github.md
- docs/workbench.md
- README.md
- skills/workbench-shared/trackers.md
- skills/workbench-shared/trackers/linear.md
- docs/plans/2026-08-02-forge-portability-design.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
Workbench abstracted the tracker but hardcoded GitHub.
ghwas called directly from shared contract prose and fromexecute's procedure body, so supporting another forge meant editing shared contracts rather than adding an adapter — and on a machine withoutgh, preflight stopped the run before any work began, in both approval modes.This extracts a five-operation forge contract with adapters, mirroring the existing tracker abstraction, and adds three capability tiers so an unresolvable forge degrades to a manual handoff instead of a hard stop.
On GitHub the behavior is unchanged, except for two live defects that are fixed. Everything else is additive, and no existing repo needs migrating.
📄 Full walkthrough:
docs/plans/2026-08-02-forge-portability-changes.md— measurements, file-by-file, compatibility, and a suggested review order.The measurable result
Real
ghinvocations (gh pr,gh auth, "GitHub CLI"):workbench-shared/trackers.mdexecute/SKILL.mdscaffold/SKILL.mdworkbench-shared/forges/github.md(new)"Pull request" in shared contracts: 32 → 0.
One number that looks wrong but isn't: "GitHub" mentions rose in the tracker adapters (
asana.md7→11,linear.md3→7). Those files always contained GitHub-specific logic — the GitHub App, the Actions template, Linear's GitHub integration — it was just unmarked. The new text says so explicitly, so a reader can tell scoped behavior from universal behavior. Implicit coupling became explicit scoping.Two live defects fixed
These are the only changes that alter GitHub behavior, and the parts most worth reviewing carefully.
1. The stamp push could never succeed on a protected branch. Recording a merge meant pushing a marker to the base branch. Most shared repos protect
main, so the push failed, the marker never landed, and the skip condition never fired — the issue was re-swept forever. Deleted entirely:trackers.md:87-88already re-read issue state, so the merged path was already idempotent and the stamp was redundant.2. The closed-unmerged path re-asked forever, in both approval modes. Worse, because there was no tracker state to fall back on — the issue sits in
inRevieweither way.approval.md:47makes it a stop that fires in both modes, and the user's answer was persisted nowhere. The marker is now a comment on the tracker issue carrying aworkbench: review-closed-unmergedsentinel, which needs no branch write access.New API surface
skills/workbench-shared/forges.md—verifyForge,resolveBase,openReview,publishReview,getReviewState, plus a declared capability table (ciHooks,draftState,pushesForYou,reviewLookup,stackedReviews). Capabilities are static facts declared in the adapter file rather than a runtimecapabilities()call.Adapters:
forges/github.md(behavior-preserving),forges/generic-git.md(tier 3),forges/TEMPLATE.md.listComments(ref)was added to the tracker contract (8 → 9 operations) — the honest cost of removing the stamp push, sincegetIssuedoes not return comments.Internal forges
We cannot ship adapters for forges whose names, hosts, and CLIs we have never seen. So a team copies
forges/TEMPLATE.mdto.workbench/forge.md, fills in five operations, and commits it. A repo-local adapter beats every bundled one — no fork of this plugin, no change to any shared contract.The safety rule worth reviewing
Tier 2 probes
PATHfor forge CLIs, which sits close to the Absolute boundary. The reconciliation, stated inforges.mditself:The probe reports and stops; accepting a probed CLI is a stop that fires in both approval modes. An agent driving an unfamiliar CLI unattended can push the wrong ref or publish a draft prematurely — with the user's credentials against the user's server.
Compatibility
No migration required. A profile with no
forge:field gets the question on next load (same repair pattern already used formerge-closer); records with a branch and no review id fall back to branch matching and get rewritten; legacy- Closed:and- PR:lines are still read.The
!on46b9108marks a breaking contract change, not a breaking upgrade.Verified
PATH-probing prose introduced none, and all 9 baseline suppressions still match live findings.ghoutsideforges/github.md; all 5 forge and 9 tracker operations resolve to a contract; all 5 capabilities declared in every adapter; no dangling cross-references.Not verified — please treat as open
listCommentstool coverage is unconfirmed against live MCPs. No Asana or Linear MCP was connected while this was written, so those tool names are documented hints. If either tracker cannot list comments, the sentinel falls back to the file marker (already specified), so this is a confirmation task rather than a blocker.forge: none) are the highest-value manual checks.Deliberately deferred
Excluded so this stays behavior-preserving on GitHub and therefore reviewable:
inRevieweven though it is explicitly not ready. Real defect; the contract now haspublishReviewto fix it with.- PR:field — wait until repos have cycled through the new format.Non-goals: multi-repo reviews (the memory model is single-repo end to end), and shipping adapters for specific public non-GitHub forges.
Opened from a fork for access reasons, not by preference.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores