ReviewOS
Reload

also looking at this

stacks/buddy

feat(setup): make the presets deliver their descriptions, and delete the mode nothing could reach

#1440
Merged glennmichael123 wants to merge chore/decisions into main
8 files +99 -50
docs/ai/agent.mdmodified+1-2
@@ -12,11 +12,10 @@ A **mode** is a playbook plus the capability tiers it may draw tools from. The t
1212|---|---|---|
1313| `plan` | read | No |
1414| `review` | read, comment | No |
15| `restricted` | read | No |
1615| `fix-ci` | read, write, shell, git, comment | Yes, on the working branch |
1716| `implement` | read, write, shell, git, comment | Yes, on the working branch |
1817
19`restricted` is what a run triggered by someone **without write access** gets. Public repositories accept input from anyone, so the actor is gated the same way the task is.
18A run triggered by someone **without write access** never reaches a mode at all: public repositories accept input from anyone, so `checkPermission` refuses non-collaborators every agent-running command before one is chosen.
2019
2120An unknown mode name raises an error rather than falling back to a default — silently landing in a permissive mode on a typo would be the worst possible failure.
2221
docs/cli/setup.mdmodified+1-1
@@ -246,7 +246,7 @@ Choose from carefully crafted presets:
246246| **Development/Testing** | Manual triggers + frequent checks for testing |
247247| **Custom Configuration** | Create your own schedule |
248248
249The preset names the intent you picked; the workflow it writes is the same either way. See [Preset Details](#preset-details) for what that means once setup is done.
249Each preset writes its own schedule into the workflow — the cadences and per-slot update strategies are live data, not labels. See [Preset Details](#preset-details) for what each generates.
250250
251251### Step 9: Configuration File Generation
252252
src/agent/index.tsmodified+0-1
@@ -4,7 +4,6 @@ export {
44 getAgentMode,
55 implementMode,
66 planMode,
7 restrictedMode,
87 reviewMode,
98} from './modes'
109export { runPlan } from './plan'
src/agent/modes.tsmodified+0-18
@@ -82,30 +82,12 @@ Name the files that would change and why, the order of the work, the risks,
8282and how the result would be verified.`,
8383}
8484
85/**
86 * The tier every non-collaborator gets.
87 *
88 * Public repositories accept input from anyone, so a run triggered by someone
89 * without write access is capped at reading — the same reasoning as
90 * review-mode's tier list, applied to the actor rather than the task.
91 */
92export const restrictedMode: AgentMode = {
93 name: 'restricted',
94 tiers: ['read'],
95 maxToolCalls: 20,
96 playbook: `${SHARED_PREAMBLE}
97
98This run was triggered by someone without write access to the repository. You
99can read and answer, but cannot modify anything or post on their behalf.`,
100}
101
10285/** Every built-in mode, by name. */
10386export const AGENT_MODES: Record<string, AgentMode> = {
10487 'review': reviewMode,
10588 'implement': implementMode,
10689 'fix-ci': fixCiMode,
10790 'plan': planMode,
108 'restricted': restrictedMode,
10991}
11092
11193/**
src/setup.tsmodified+45-16
@@ -1390,6 +1390,8 @@ export type ScheduledJob = 'update' | 'dashboard' | 'check' | 'report'
13901390export interface ScheduleEntry {
13911391 cron: string
13921392 jobs: ScheduledJob[]
1393 /** Strategy an `update` tick runs with; absent falls back to the preset default */
1394 updateStrategy?: string
13931395}
13941396
13951397/**
@@ -1421,8 +1423,9 @@ export interface ScheduleEntry {
14211423 */
14221424export function schedulePlan(preset: WorkflowPreset): ScheduleEntry[] {
14231425 const byCron = new Map<string, ScheduledJob[]>()
1426 const strategyByCron = new Map<string, string>()
14241427
1425 const add = (cron: string | undefined, job: ScheduledJob): void => {
1428 const add = (cron: string | undefined, job: ScheduledJob, strategy?: string): void => {
14261429 if (!cron || cron === 'manual')
14271430 return
14281431
@@ -1431,14 +1434,26 @@ export function schedulePlan(preset: WorkflowPreset): ScheduleEntry[] {
14311434 jobs.push(job)
14321435
14331436 byCron.set(cron, jobs)
1437
1438 if (job === 'update' && strategy && !strategyByCron.has(cron))
1439 strategyByCron.set(cron, strategy)
14341440 }
14351441
1436 add(preset.schedules.updates, 'update')
1442 add(preset.schedules.updates, 'update', preset.strategy)
1443 // The preset's custom cadences are real schedule slots, each with its own
1444 // strategy — this is what lets a preset deliver "daily patch, weekly minor,
1445 // monthly major" instead of one cron proposing everything.
1446 for (const custom of preset.custom ?? [])
1447 add(custom.schedule, 'update', custom.strategy)
14371448 add(preset.schedules.dashboard, 'dashboard')
14381449 add(CLEANUP_SCHEDULE, 'check')
14391450 add(REPORT_SCHEDULE, 'report')
14401451
1441 return [...byCron].map(([cron, jobs]) => ({ cron, jobs }))
1452 return [...byCron].map(([cron, jobs]) => ({
1453 cron,
1454 jobs,
1455 ...(strategyByCron.has(cron) ? { updateStrategy: strategyByCron.get(cron) } : {}),
1456 }))
14421457}
14431458
14441459/** What each job is for, so the generated YAML explains its own schedule. */
@@ -1469,9 +1484,10 @@ function renderScheduleDispatch(plan: ScheduleEntry[]): string {
14691484 if (plan.length === 0)
14701485 return ' echo "ℹ️ No scheduled jobs are configured"'
14711486
1472 const branches = plan.map(({ cron, jobs }) => [
1487 const branches = plan.map(({ cron, jobs, updateStrategy }) => [
14731488 ` "${cron}")`,
14741489 ...jobs.map(job => ` echo "run_${job}=true" >> $GITHUB_OUTPUT`),
1490 ...(updateStrategy ? [` echo "update_strategy=${updateStrategy}" >> $GITHUB_OUTPUT`] : []),
14751491 ' ;;',
14761492 ].join('\n'))
14771493
@@ -1620,6 +1636,7 @@ jobs:
16201636 run_fixci: \${{ steps.determine.outputs.run_fixci }}
16211637 run_report: \${{ steps.determine.outputs.run_report }}
16221638 run_any: \${{ steps.determine.outputs.run_any }}
1639 update_strategy: \${{ steps.determine.outputs.update_strategy }}
16231640 steps:
16241641 - name: Determine which jobs to run
16251642 id: determine
@@ -2104,7 +2121,7 @@ ${generateComposerSetupSteps()}
21042121 - name: Display update configuration
21052122 run: |
21062123 echo "🧪 **Buddy Update Mode**"
2107 echo "Strategy: \$"
2124 echo "Strategy: \$"
21082125 echo "Dry Run: \$"
21092126 echo "Packages: \$"
21102127 echo "Verbose: \$"
@@ -2115,7 +2132,7 @@ ${generateComposerSetupSteps()}
21152132 - name: Run Buddy dependency updates
21162133 if: \${{ github.event.inputs.dry_run != 'true' }}
21172134 run: |
2118 STRATEGY="\$"
2135 STRATEGY="\$"
21192136 PACKAGES="\$"
21202137 VERBOSE="\$"
21212138
@@ -2154,7 +2171,7 @@ ${generateComposerSetupSteps()}
21542171 run: |
21552172 echo "## 🚀 Dependency Update Summary" >> \$GITHUB_STEP_SUMMARY
21562173 echo "" >> \$GITHUB_STEP_SUMMARY
2157 echo "- **Strategy**: \$" >> \$GITHUB_STEP_SUMMARY
2174 echo "- **Strategy**: \$" >> \$GITHUB_STEP_SUMMARY
21582175 echo "- **Triggered by**: \$" >> \$GITHUB_STEP_SUMMARY
21592176 echo "- **Dry run**: \$" >> \$GITHUB_STEP_SUMMARY
21602177 echo "- **Packages**: \$" >> \$GITHUB_STEP_SUMMARY
@@ -2427,11 +2444,16 @@ export function getWorkflowPreset(useCase: string): WorkflowPreset {
24272444 },
24282445 schedules: {
24292446 dashboard: '0 9 * * 1,3,5',
2430 updates: '0 9 * * 1,3,5',
2447 updates: '0 9 * * *',
24312448 },
2432 strategy: 'all',
2449 // The description is the contract: patches daily on the main slot,
2450 // minors weekly and majors monthly on their own slots below.
2451 strategy: 'patch',
24332452 autoMerge: false,
2434 custom: [],
2453 custom: [
2454 { name: 'weekly-minor', schedule: '0 9 * * 1', strategy: 'minor', autoMerge: false },
2455 { name: 'monthly-major', schedule: '0 9 1 * *', strategy: 'major', autoMerge: false },
2456 ],
24352457 },
24362458 'high-frequency': {
24372459 name: 'High Frequency Updates',
@@ -2439,9 +2461,12 @@ export function getWorkflowPreset(useCase: string): WorkflowPreset {
24392461 templates: {},
24402462 schedules: {
24412463 dashboard: '0 9 * * *',
2442 updates: '0 */6 * * *',
2464 // The four custom slots below are the cadence; a fifth catch-all
2465 // here would double-run beside them with a wider strategy than the
2466 // preset describes.
2467 updates: 'manual',
24432468 },
2444 strategy: 'all',
2469 strategy: 'patch',
24452470 autoMerge: true,
24462471 custom: [
24472472 { name: 'morning-updates', schedule: '0 6 * * *', strategy: 'patch', autoMerge: true, autoMergeStrategy: 'squash' },
@@ -2458,10 +2483,12 @@ export function getWorkflowPreset(useCase: string): WorkflowPreset {
24582483 dashboard: '0 9 * * *',
24592484 updates: '0 */4 * * *',
24602485 },
2461 strategy: 'all',
2486 // "Frequent patch updates": the every-four-hours slot proposes
2487 // patches and minors wait for the weekly slot. The old
2488 // security-patches slot duplicated the main cadence and is gone.
2489 strategy: 'patch',
24622490 autoMerge: true,
24632491 custom: [
2464 { name: 'security-patches', schedule: '0 */6 * * *', strategy: 'patch', autoMerge: true, autoMergeStrategy: 'squash' },
24652492 { name: 'weekly-minor', schedule: '0 9 * * 1', strategy: 'minor', autoMerge: false },
24662493 ],
24672494 },
@@ -2476,9 +2503,11 @@ export function getWorkflowPreset(useCase: string): WorkflowPreset {
24762503 dashboard: '0 9 * * 1',
24772504 updates: '0 9 * * 1',
24782505 },
2479 strategy: 'all',
2506 strategy: 'patch',
24802507 autoMerge: false,
2481 custom: [],
2508 custom: [
2509 { name: 'monthly-updates', schedule: '0 9 1 * *', strategy: 'all', autoMerge: false },
2510 ],
24822511 },
24832512 'docker': {
24842513 name: 'Docker Project',
test/agent-runtime.test.tsmodified+6-2
@@ -157,8 +157,12 @@ describe('mode tool permissions', () => {
157157 expect(output.content).toContain('No such tool')
158158 })
159159
160 it('failure case - the restricted tier grants reads only', () => {
161 const belt = new Toolbelt(getAgentMode('restricted'))
160 it('failure case - the plan mode grants reads only', () => {
161 // The read tier alone: no comment tool, nothing that writes. (This was
162 // `restricted` until that mode was deleted — no code path could select
163 // it, since checkPermission refuses non-collaborators before a mode is
164 // ever chosen.)
165 const belt = new Toolbelt(getAgentMode('plan'))
162166
163167 expect(belt.names()).toEqual(['read_file', 'list_dir'])
164168 })
test/cli-setup-extended.test.tsmodified+1-1
@@ -73,7 +73,7 @@ describe('CLI Setup - Extended Tests', () => {
7373 // Security preset
7474 const security = getWorkflowPreset('security')
7575 expect(security.name).toBe('Security Focused')
76 expect(security.custom?.some(c => c.name === 'security-patches')).toBe(true)
76 expect(security.custom?.some(c => c.name === 'weekly-minor')).toBe(true)
7777
7878 // Minimal preset
7979 const minimal = getWorkflowPreset('minimal')
test/workflow-presets.test.tsmodified+45-9
@@ -35,7 +35,8 @@ describe('workflow presets', () => {
3535 describe('the schedule plan', () => {
3636 it('success case - a preset\'s own cadence reaches the workflow', () => {
3737 expect(crons('security')).toContain('0 */4 * * *')
38 expect(crons('high-frequency')).toContain('0 */6 * * *')
38 expect(crons('high-frequency')).toContain('0 6 * * *')
39 expect(crons('standard')).toContain('0 9 1 * *')
3940 expect(crons('testing')).toContain('*/15 * * * *')
4041 })
4142
@@ -45,13 +46,13 @@ describe('workflow presets', () => {
4546 })
4647
4748 it('success case - a cron shared by two jobs starts both', () => {
48 // Standard puts updates and the dashboard on one expression. An if/elif
49 // Monorepo puts updates and the dashboard on one expression. An if/elif
4950 // chain would let the first branch win and drop the dashboard entirely.
50 const plan = schedulePlan(getWorkflowPreset('standard'))
51 const shared = plan.find(entry => entry.cron === '0 9 * * 1,3,5')
51 const plan = schedulePlan(getWorkflowPreset('monorepo'))
52 const shared = plan.find(entry => entry.cron === '0 9 * * *')
5253
5354 expect(shared?.jobs).toEqual(['update', 'dashboard'])
54 expect(crons('standard').filter(cron => cron === '0 9 * * 1,3,5')).toHaveLength(1)
55 expect(crons('monorepo').filter(cron => cron === '0 9 * * *')).toHaveLength(1)
5556 })
5657
5758 it('success case - a cron shared by three jobs starts all three', () => {
@@ -82,6 +83,40 @@ describe('workflow presets', () => {
8283 })
8384 })
8485
86 describe('per-slot update strategies', () => {
87 it('success case - a custom cadence is a real schedule slot with its own strategy', () => {
88 const plan = schedulePlan(getWorkflowPreset('standard'))
89
90 expect(plan.find(entry => entry.cron === '0 9 * * *')?.updateStrategy).toBe('patch')
91 expect(plan.find(entry => entry.cron === '0 9 * * 1')?.updateStrategy).toBe('minor')
92 expect(plan.find(entry => entry.cron === '0 9 1 * *')?.updateStrategy).toBe('major')
93 })
94
95 it('success case - the tick tells the update job which strategy fired', () => {
96 const yaml = generateUnifiedWorkflow(false, getWorkflowPreset('standard'))
97
98 expect(yaml).toContain('"0 9 1 * *")')
99 expect(yaml).toContain('echo "update_strategy=major" >> $GITHUB_OUTPUT')
100 expect(yaml).toContain('needs.determine-jobs.outputs.update_strategy')
101 })
102
103 it('success case - high-frequency is its four slots, not a fifth catch-all', () => {
104 const plan = schedulePlan(getWorkflowPreset('high-frequency'))
105 const updates = plan.filter(entry => entry.jobs.includes('update'))
106
107 expect(updates.map(entry => entry.cron).sort()).toEqual(['0 0 * * *', '0 12 * * *', '0 18 * * *', '0 6 * * *'])
108 // Midnight is the minor slot the preset describes as manual-review.
109 expect(plan.find(entry => entry.cron === '0 0 * * *')?.updateStrategy).toBe('minor')
110 })
111
112 it('success case - security stops duplicating its own patch cadence', () => {
113 const plan = schedulePlan(getWorkflowPreset('security'))
114
115 expect(plan.some(entry => entry.cron === '0 */6 * * *')).toBe(false)
116 expect(plan.find(entry => entry.cron === '0 */4 * * *')?.updateStrategy).toBe('patch')
117 })
118 })
119
85120 describe('triggers and their matcher agree', () => {
86121 it('success case - every declared cron has a branch that runs something', () => {
87122 // A cron declared in `schedule:` with no matching branch starts a
@@ -129,7 +164,7 @@ describe('workflow presets', () => {
129164 it('success case - scheduled runs use the preset strategy', () => {
130165 const yaml = generateUnifiedWorkflow(false, getWorkflowPreset('testing'))
131166
132 expect(yaml).toContain(`github.event.inputs.strategy || 'patch'`)
167 expect(yaml).toContain(`github.event.inputs.strategy || needs.determine-jobs.outputs.update_strategy || 'patch'`)
133168 })
134169
135170 it('success case - the manual trigger defaults to the same strategy', () => {
@@ -181,7 +216,8 @@ describe('the generated config file', () => {
181216
182217 it('success case - the update strategy comes from the preset', async () => {
183218 expect((await generated('testing')).packages?.strategy).toBe('patch')
184 expect((await generated('standard')).packages?.strategy).toBe('all')
219 // Standard's main slot is the daily-patch cadence its description names.
220 expect((await generated('standard')).packages?.strategy).toBe('patch')
185221 })
186222
187223 it('success case - the preset\'s custom workflows reach the config', async () => {
@@ -189,8 +225,8 @@ describe('the generated config file', () => {
189225 // and a config that drops them means the workflows are never written.
190226 const custom = (await generated('security')).workflows?.custom ?? []
191227
192 expect(custom.map(workflow => workflow.name)).toEqual(['security-patches', 'weekly-minor'])
193 expect(custom[0].schedule).toBe('0 */6 * * *')
228 expect(custom.map(workflow => workflow.name)).toEqual(['weekly-minor'])
229 expect(custom[0].schedule).toBe('0 9 * * 1')
194230 })
195231
196232 it('success case - templates come from the preset', async () => {