-
Notifications
You must be signed in to change notification settings - Fork 6
feat(cos): multiple ordered code-review agents per task #459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
8e28732
feat(cos): support multiple ordered code-review agents per task
atomantic dfdd16d
address review (codex): fix copilot ordering + stop-mode merge gate
atomantic 6937d5b
address review (gemini): tighten multi-reviewer prompt wording
atomantic 52c7fae
address review (copilot): update cleanupAgentWorktree doc for multi-r…
atomantic 446374b
address review (copilot): de-dupe reviewers in ReviewerPicker render
atomantic 7f4e009
address review (copilot): spawn follow-up on failed pre-request + UI/…
atomantic b18512f
address review (copilot): make CLI-reviewer diff instruction forge-ne…
atomantic e0abdd2
address review (copilot): harden stop-mode merge gate + drop dangling…
atomantic 6359799
fix(test): de-flake peerSync autoSubscribe short-circuit test in CI
atomantic ea3434f
address review (copilot): docblock, boolean coercion, copilot-first w…
atomantic 3506228
address review (copilot): accurate Code Review prompt for copilot ord…
atomantic 0f53975
address review (copilot): rename {reviewer} prompt placeholder to {re…
atomantic 3a8aaa9
address review (copilot): changelog accuracy, cap clarification, var …
atomantic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../../../lib/slashdo/commands/do/depfree.md |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../../../lib/slashdo/commands/do/pr-better.md |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| ../../../lib/slashdo/commands/do/scan.md |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| import { useId } from 'react'; | ||
| import { Plus, X, ChevronUp, ChevronDown } from 'lucide-react'; | ||
| import { | ||
| REVIEWER_OPTIONS, | ||
| REVIEW_STOP_MODES, | ||
| DEFAULT_REVIEW_STOP_MODE | ||
| } from './constants'; | ||
|
|
||
| const labelFor = (value) => REVIEWER_OPTIONS.find(o => o.value === value)?.label || value; | ||
|
|
||
| /** | ||
| * Ordered multi-reviewer picker. Click a reviewer to append it (run order = | ||
| * click order), reorder with the arrows, remove with ✕. Maps to slashdo's | ||
| * `--review-with a,b,c` plus the stop-mode / `--reviewer-applies` flags. | ||
| * | ||
| * Controlled: emits the full next shape via onChange so the parent can store | ||
| * `reviewers` / `reviewStopMode` / `reviewerApplies` however it persists them. | ||
| */ | ||
| export default function ReviewerPicker({ | ||
| reviewers = [], | ||
| stopMode = DEFAULT_REVIEW_STOP_MODE, | ||
| reviewerApplies = false, | ||
| onChange, | ||
| disabled = false | ||
| }) { | ||
| const id = useId(); | ||
| // Render the parent's list (de-duped, order-preserving) so display === stored | ||
| // state for valid input while staying robust to malformed/legacy duplicates — | ||
| // dupes would otherwise collide on the `key={value}` below and corrupt | ||
| // reorder/remove. An empty list shows the "defaults to Copilot" hint and lets | ||
| // the user clear copilot; the server/submit layer resolves [] → ['copilot']. | ||
| const selected = Array.isArray(reviewers) ? [...new Set(reviewers)] : []; | ||
| const available = REVIEWER_OPTIONS.filter(o => !selected.includes(o.value)); | ||
| const hasNonCopilot = selected.some(r => r !== 'copilot'); | ||
|
|
||
| const emit = (next) => onChange?.({ | ||
| reviewers: selected, | ||
| stopMode, | ||
| reviewerApplies, | ||
| ...next | ||
| }); | ||
|
|
||
| const add = (value) => emit({ reviewers: [...selected, value] }); | ||
| const remove = (value) => emit({ reviewers: selected.filter(r => r !== value) }); | ||
| const move = (index, delta) => { | ||
| const target = index + delta; | ||
| if (target < 0 || target >= selected.length) return; | ||
| const next = [...selected]; | ||
| [next[index], next[target]] = [next[target], next[index]]; | ||
| emit({ reviewers: next }); | ||
| }; | ||
|
|
||
| return ( | ||
| <div className="flex flex-col gap-2 w-full"> | ||
| <div className="flex flex-wrap items-center gap-1.5"> | ||
| <span className="text-xs text-gray-500 mr-1">Reviewers (in order):</span> | ||
| {selected.map((value, index) => ( | ||
| <span | ||
| key={value} | ||
| className="inline-flex items-center gap-1 pl-1.5 pr-1 py-0.5 bg-port-bg border border-port-border rounded text-xs text-gray-300" | ||
| title={REVIEWER_OPTIONS.find(o => o.value === value)?.description} | ||
| > | ||
| <span className="text-port-accent font-mono">{index + 1}.</span> | ||
| {labelFor(value)} | ||
| <button | ||
| type="button" | ||
| disabled={disabled || index === 0} | ||
| onClick={() => move(index, -1)} | ||
| className="text-gray-500 hover:text-white disabled:opacity-30 disabled:hover:text-gray-500" | ||
| aria-label={`Move ${labelFor(value)} earlier`} | ||
| > | ||
| <ChevronUp size={12} /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| disabled={disabled || index === selected.length - 1} | ||
| onClick={() => move(index, 1)} | ||
| className="text-gray-500 hover:text-white disabled:opacity-30 disabled:hover:text-gray-500" | ||
| aria-label={`Move ${labelFor(value)} later`} | ||
| > | ||
| <ChevronDown size={12} /> | ||
| </button> | ||
| <button | ||
| type="button" | ||
| disabled={disabled} | ||
| onClick={() => remove(value)} | ||
| className="text-gray-500 hover:text-port-error" | ||
| aria-label={`Remove ${labelFor(value)}`} | ||
| > | ||
| <X size={12} /> | ||
| </button> | ||
| </span> | ||
| ))} | ||
| {selected.length === 0 && ( | ||
| <span className="text-xs text-gray-600 italic">none — defaults to Copilot</span> | ||
| )} | ||
| </div> | ||
|
|
||
| {available.length > 0 && ( | ||
| <div className="flex flex-wrap items-center gap-1.5"> | ||
| <span className="text-xs text-gray-600 mr-1">Add:</span> | ||
| {available.map(opt => ( | ||
| <button | ||
| key={opt.value} | ||
| type="button" | ||
| disabled={disabled} | ||
| onClick={() => add(opt.value)} | ||
| title={opt.description} | ||
| className="inline-flex items-center gap-1 px-1.5 py-0.5 bg-transparent border border-port-border rounded text-xs text-gray-400 hover:text-white hover:border-port-accent disabled:opacity-50" | ||
| > | ||
| <Plus size={11} /> | ||
| {opt.label} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| )} | ||
|
|
||
| {selected.length >= 2 && ( | ||
| <div className="flex items-center gap-2"> | ||
| <label htmlFor={`${id}-stopmode`} className="text-xs text-gray-500">Stop mode:</label> | ||
| <select | ||
| id={`${id}-stopmode`} | ||
| value={stopMode} | ||
| disabled={disabled} | ||
| onChange={e => emit({ stopMode: e.target.value })} | ||
| className="px-1.5 py-0.5 bg-port-bg border border-port-border rounded text-xs text-gray-300 min-h-[28px]" | ||
| > | ||
| {REVIEW_STOP_MODES.map(m => ( | ||
| <option key={m.value} value={m.value} title={m.description}>{m.label}</option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
|
|
||
| {hasNonCopilot && ( | ||
| <label htmlFor={`${id}-applies`} className="flex items-center gap-2 cursor-pointer select-none text-xs text-gray-500"> | ||
| <input | ||
| id={`${id}-applies`} | ||
| type="checkbox" | ||
| checked={reviewerApplies} | ||
| disabled={disabled} | ||
| onChange={e => emit({ reviewerApplies: e.target.checked })} | ||
| className="w-3.5 h-3.5 rounded border-port-border bg-port-bg text-port-accent focus:ring-port-accent focus:ring-offset-0" | ||
| /> | ||
| Reviewer applies fixes (CLI edits the working tree; no effect on Copilot) | ||
| </label> | ||
| )} | ||
| </div> | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import { describe, it, expect, vi } from 'vitest'; | ||
| import { render, screen } from '@testing-library/react'; | ||
| import userEvent from '@testing-library/user-event'; | ||
| import ReviewerPicker from './ReviewerPicker'; | ||
|
|
||
| describe('ReviewerPicker', () => { | ||
| it('renders the selected reviewers in order with numbered badges', () => { | ||
| render(<ReviewerPicker reviewers={['codex', 'gemini', 'copilot']} onChange={() => {}} />); | ||
| expect(screen.getByText('1.')).toBeInTheDocument(); | ||
| expect(screen.getByText('2.')).toBeInTheDocument(); | ||
| expect(screen.getByText('3.')).toBeInTheDocument(); | ||
| // The not-yet-selected reviewer (claude) shows in the Add row. | ||
| expect(screen.getByRole('button', { name: /Claude/ })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('shows the empty-state hint when no reviewers are selected', () => { | ||
| render(<ReviewerPicker reviewers={[]} onChange={() => {}} />); | ||
| expect(screen.getByText(/none — defaults to Copilot/)).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('de-dupes a malformed list with duplicates (order-preserving)', () => { | ||
| render(<ReviewerPicker reviewers={['codex', 'codex', 'gemini']} onChange={() => {}} />); | ||
| // Two distinct pills (badges 1 and 2), not three. | ||
| expect(screen.getByText('1.')).toBeInTheDocument(); | ||
| expect(screen.getByText('2.')).toBeInTheDocument(); | ||
| expect(screen.queryByText('3.')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('emits an empty list when the last reviewer is removed (server resolves to copilot)', async () => { | ||
| const onChange = vi.fn(); | ||
| const user = userEvent.setup(); | ||
| render(<ReviewerPicker reviewers={['copilot']} onChange={onChange} />); | ||
| await user.click(screen.getByLabelText('Remove Copilot')); | ||
| expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ reviewers: [] })); | ||
| }); | ||
|
|
||
| it('appends a reviewer in click order on add', async () => { | ||
| const onChange = vi.fn(); | ||
| const user = userEvent.setup(); | ||
| render(<ReviewerPicker reviewers={['copilot']} onChange={onChange} />); | ||
| await user.click(screen.getByRole('button', { name: /Codex/ })); | ||
| expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ reviewers: ['copilot', 'codex'] })); | ||
| }); | ||
|
|
||
| it('reorders with the up arrow', async () => { | ||
| const onChange = vi.fn(); | ||
| const user = userEvent.setup(); | ||
| render(<ReviewerPicker reviewers={['codex', 'gemini', 'copilot']} onChange={onChange} />); | ||
| await user.click(screen.getByLabelText('Move Gemini earlier')); | ||
| expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ reviewers: ['gemini', 'codex', 'copilot'] })); | ||
| }); | ||
|
|
||
| it('removes a reviewer', async () => { | ||
| const onChange = vi.fn(); | ||
| const user = userEvent.setup(); | ||
| render(<ReviewerPicker reviewers={['codex', 'copilot']} onChange={onChange} />); | ||
| await user.click(screen.getByLabelText('Remove Codex')); | ||
| expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ reviewers: ['copilot'] })); | ||
| }); | ||
|
|
||
| it('shows the stop-mode select only for 2+ reviewers', () => { | ||
| const { rerender } = render(<ReviewerPicker reviewers={['codex']} onChange={() => {}} />); | ||
| expect(screen.queryByText('Stop mode:')).not.toBeInTheDocument(); | ||
| rerender(<ReviewerPicker reviewers={['codex', 'gemini']} onChange={() => {}} />); | ||
| expect(screen.getByText('Stop mode:')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('shows the reviewer-applies toggle only when a non-copilot reviewer is present', () => { | ||
| const { rerender } = render(<ReviewerPicker reviewers={['copilot']} onChange={() => {}} />); | ||
| expect(screen.queryByText(/Reviewer applies fixes/)).not.toBeInTheDocument(); | ||
| rerender(<ReviewerPicker reviewers={['codex']} onChange={() => {}} />); | ||
| expect(screen.getByText(/Reviewer applies fixes/)).toBeInTheDocument(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.