Skip to content

Commit 173eac1

Browse files
shreyas-lyzrclaude
andcommitted
fix: address review feedback on OpenCode adapter (#29)
- Use AGENTS.md instead of .opencode/instructions.md (per OpenCode docs) - Fix opencode.json config: model uses provider/model format, provider is object with npm package - Fix runner CLI: use `opencode run --prompt` for single-shot mode - Fix process.exit() before finally cleanup — move cleanup before exit - Extract shared buildComplianceSection to src/adapters/shared.ts (dedup from copilot.ts) - Fix importer to read AGENTS.md and parse provider/model format - Fix git auto-detection to check opencode.json instead of .opencode/ Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 998793a commit 173eac1

File tree

6 files changed

+121
-164
lines changed

6 files changed

+121
-164
lines changed

src/adapters/copilot.ts

Lines changed: 1 addition & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { join, resolve } from 'node:path';
33
import yaml from 'js-yaml';
44
import { loadAgentManifest, loadFileIfExists } from '../utils/loader.js';
55
import { loadAllSkills, getAllowedTools } from '../utils/skill-loader.js';
6+
import { buildComplianceSection } from './shared.js';
67

78
/**
89
* Export a gitagent to GitHub Copilot CLI format.
@@ -204,65 +205,3 @@ function collectSkills(agentDir: string): Array<{ name: string; content: string
204205
return skills;
205206
}
206207

207-
function buildComplianceSection(compliance: NonNullable<ReturnType<typeof loadAgentManifest>['compliance']>): string {
208-
const c = compliance;
209-
const constraints: string[] = [];
210-
211-
if (c.supervision?.human_in_the_loop === 'always') {
212-
constraints.push('- All decisions require human approval before execution');
213-
}
214-
if (c.supervision?.escalation_triggers) {
215-
constraints.push('- Escalate to human supervisor when:');
216-
for (const trigger of c.supervision.escalation_triggers) {
217-
for (const [key, value] of Object.entries(trigger)) {
218-
constraints.push(` - ${key}: ${value}`);
219-
}
220-
}
221-
}
222-
if (c.communications?.fair_balanced) {
223-
constraints.push('- All communications must be fair and balanced (FINRA 2210)');
224-
}
225-
if (c.communications?.no_misleading) {
226-
constraints.push('- Never make misleading, exaggerated, or promissory statements');
227-
}
228-
if (c.data_governance?.pii_handling === 'redact') {
229-
constraints.push('- Redact all PII from outputs');
230-
}
231-
if (c.data_governance?.pii_handling === 'prohibit') {
232-
constraints.push('- Do not process any personally identifiable information');
233-
}
234-
235-
if (c.segregation_of_duties) {
236-
const sod = c.segregation_of_duties;
237-
constraints.push('- Segregation of duties is enforced:');
238-
if (sod.assignments) {
239-
for (const [agentName, roles] of Object.entries(sod.assignments)) {
240-
constraints.push(` - Agent "${agentName}" has role(s): ${roles.join(', ')}`);
241-
}
242-
}
243-
if (sod.conflicts) {
244-
constraints.push('- Duty separation rules (no single agent may hold both):');
245-
for (const [a, b] of sod.conflicts) {
246-
constraints.push(` - ${a} and ${b}`);
247-
}
248-
}
249-
if (sod.handoffs) {
250-
constraints.push('- The following actions require multi-agent handoff:');
251-
for (const h of sod.handoffs) {
252-
constraints.push(` - ${h.action}: must pass through roles ${h.required_roles.join(' → ')}${h.approval_required !== false ? ' (approval required)' : ''}`);
253-
}
254-
}
255-
if (sod.isolation?.state === 'full') {
256-
constraints.push('- Agent state/memory is fully isolated per role');
257-
}
258-
if (sod.isolation?.credentials === 'separate') {
259-
constraints.push('- Credentials are segregated per role');
260-
}
261-
if (sod.enforcement === 'strict') {
262-
constraints.push('- SOD enforcement is STRICT — violations will block execution');
263-
}
264-
}
265-
266-
if (constraints.length === 0) return '';
267-
return `## Compliance Constraints\n\n${constraints.join('\n')}`;
268-
}

src/adapters/opencode.ts

Lines changed: 19 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@ import { join, resolve } from 'node:path';
33
import yaml from 'js-yaml';
44
import { loadAgentManifest, loadFileIfExists } from '../utils/loader.js';
55
import { loadAllSkills, getAllowedTools } from '../utils/skill-loader.js';
6+
import { buildComplianceSection } from './shared.js';
67

78
/**
89
* Export a gitagent to OpenCode format.
910
*
1011
* OpenCode (sst/opencode) uses:
11-
* - .opencode/instructions.md (custom agent instructions)
12-
* - opencode.json (project configuration)
12+
* - AGENTS.md (custom agent instructions, project root)
13+
* - opencode.json (project configuration)
1314
*
1415
* Returns structured output with all files that should be written.
1516
*/
@@ -35,7 +36,7 @@ export function exportToOpenCodeString(dir: string): string {
3536
const exp = exportToOpenCode(dir);
3637
const parts: string[] = [];
3738

38-
parts.push('# === .opencode/instructions.md ===');
39+
parts.push('# === AGENTS.md ===');
3940
parts.push(exp.instructions);
4041
parts.push('\n# === opencode.json ===');
4142
parts.push(JSON.stringify(exp.config, null, 2));
@@ -177,8 +178,12 @@ function buildConfig(manifest: ReturnType<typeof loadAgentManifest>): Record<str
177178
if (manifest.model?.preferred) {
178179
const model = manifest.model.preferred;
179180
const provider = inferProvider(model);
180-
config.provider = provider;
181-
config.model = model;
181+
config.model = `${provider}/${model}`;
182+
config.provider = {
183+
[provider]: {
184+
npm: getNpmPackage(provider),
185+
},
186+
};
182187
}
183188

184189
return config;
@@ -193,65 +198,13 @@ function inferProvider(model: string): string {
193198
return 'openai';
194199
}
195200

196-
function buildComplianceSection(compliance: NonNullable<ReturnType<typeof loadAgentManifest>['compliance']>): string {
197-
const c = compliance;
198-
const constraints: string[] = [];
199-
200-
if (c.supervision?.human_in_the_loop === 'always') {
201-
constraints.push('- All decisions require human approval before execution');
202-
}
203-
if (c.supervision?.escalation_triggers) {
204-
constraints.push('- Escalate to human supervisor when:');
205-
for (const trigger of c.supervision.escalation_triggers) {
206-
for (const [key, value] of Object.entries(trigger)) {
207-
constraints.push(` - ${key}: ${value}`);
208-
}
209-
}
210-
}
211-
if (c.communications?.fair_balanced) {
212-
constraints.push('- All communications must be fair and balanced (FINRA 2210)');
213-
}
214-
if (c.communications?.no_misleading) {
215-
constraints.push('- Never make misleading, exaggerated, or promissory statements');
216-
}
217-
if (c.data_governance?.pii_handling === 'redact') {
218-
constraints.push('- Redact all PII from outputs');
219-
}
220-
if (c.data_governance?.pii_handling === 'prohibit') {
221-
constraints.push('- Do not process any personally identifiable information');
222-
}
223-
224-
if (c.segregation_of_duties) {
225-
const sod = c.segregation_of_duties;
226-
constraints.push('- Segregation of duties is enforced:');
227-
if (sod.assignments) {
228-
for (const [agentName, roles] of Object.entries(sod.assignments)) {
229-
constraints.push(` - Agent "${agentName}" has role(s): ${roles.join(', ')}`);
230-
}
231-
}
232-
if (sod.conflicts) {
233-
constraints.push('- Duty separation rules (no single agent may hold both):');
234-
for (const [a, b] of sod.conflicts) {
235-
constraints.push(` - ${a} and ${b}`);
236-
}
237-
}
238-
if (sod.handoffs) {
239-
constraints.push('- The following actions require multi-agent handoff:');
240-
for (const h of sod.handoffs) {
241-
constraints.push(` - ${h.action}: must pass through roles ${h.required_roles.join(' → ')}${h.approval_required !== false ? ' (approval required)' : ''}`);
242-
}
243-
}
244-
if (sod.isolation?.state === 'full') {
245-
constraints.push('- Agent state/memory is fully isolated per role');
246-
}
247-
if (sod.isolation?.credentials === 'separate') {
248-
constraints.push('- Credentials are segregated per role');
249-
}
250-
if (sod.enforcement === 'strict') {
251-
constraints.push('- SOD enforcement is STRICT — violations will block execution');
252-
}
253-
}
254-
255-
if (constraints.length === 0) return '';
256-
return `## Compliance Constraints\n\n${constraints.join('\n')}`;
201+
function getNpmPackage(provider: string): string {
202+
const packages: Record<string, string> = {
203+
anthropic: '@ai-sdk/anthropic',
204+
openai: '@ai-sdk/openai',
205+
google: '@ai-sdk/google',
206+
deepseek: '@ai-sdk/deepseek',
207+
ollama: '@ai-sdk/ollama',
208+
};
209+
return packages[provider] || `@ai-sdk/${provider}`;
257210
}

src/adapters/shared.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { loadAgentManifest } from '../utils/loader.js';
2+
3+
/**
4+
* Build a markdown compliance constraints section from a gitagent manifest.
5+
* Shared across adapters that emit markdown instructions.
6+
*/
7+
export function buildComplianceSection(compliance: NonNullable<ReturnType<typeof loadAgentManifest>['compliance']>): string {
8+
const c = compliance;
9+
const constraints: string[] = [];
10+
11+
if (c.supervision?.human_in_the_loop === 'always') {
12+
constraints.push('- All decisions require human approval before execution');
13+
}
14+
if (c.supervision?.escalation_triggers) {
15+
constraints.push('- Escalate to human supervisor when:');
16+
for (const trigger of c.supervision.escalation_triggers) {
17+
for (const [key, value] of Object.entries(trigger)) {
18+
constraints.push(` - ${key}: ${value}`);
19+
}
20+
}
21+
}
22+
if (c.communications?.fair_balanced) {
23+
constraints.push('- All communications must be fair and balanced (FINRA 2210)');
24+
}
25+
if (c.communications?.no_misleading) {
26+
constraints.push('- Never make misleading, exaggerated, or promissory statements');
27+
}
28+
if (c.data_governance?.pii_handling === 'redact') {
29+
constraints.push('- Redact all PII from outputs');
30+
}
31+
if (c.data_governance?.pii_handling === 'prohibit') {
32+
constraints.push('- Do not process any personally identifiable information');
33+
}
34+
35+
if (c.segregation_of_duties) {
36+
const sod = c.segregation_of_duties;
37+
constraints.push('- Segregation of duties is enforced:');
38+
if (sod.assignments) {
39+
for (const [agentName, roles] of Object.entries(sod.assignments)) {
40+
constraints.push(` - Agent "${agentName}" has role(s): ${roles.join(', ')}`);
41+
}
42+
}
43+
if (sod.conflicts) {
44+
constraints.push('- Duty separation rules (no single agent may hold both):');
45+
for (const [a, b] of sod.conflicts) {
46+
constraints.push(` - ${a} and ${b}`);
47+
}
48+
}
49+
if (sod.handoffs) {
50+
constraints.push('- The following actions require multi-agent handoff:');
51+
for (const h of sod.handoffs) {
52+
constraints.push(` - ${h.action}: must pass through roles ${h.required_roles.join(' → ')}${h.approval_required !== false ? ' (approval required)' : ''}`);
53+
}
54+
}
55+
if (sod.isolation?.state === 'full') {
56+
constraints.push('- Agent state/memory is fully isolated per role');
57+
}
58+
if (sod.isolation?.credentials === 'separate') {
59+
constraints.push('- Credentials are segregated per role');
60+
}
61+
if (sod.enforcement === 'strict') {
62+
constraints.push('- SOD enforcement is STRICT — violations will block execution');
63+
}
64+
}
65+
66+
if (constraints.length === 0) return '';
67+
return `## Compliance Constraints\n\n${constraints.join('\n')}`;
68+
}

src/commands/import.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,18 +190,18 @@ function importFromCrewAI(sourcePath: string, targetDir: string): void {
190190
function importFromOpenCode(sourcePath: string, targetDir: string): void {
191191
const sourceDir = resolve(sourcePath);
192192

193-
// Look for .opencode/instructions.md or opencode.json
194-
const instructionsPath = join(sourceDir, '.opencode', 'instructions.md');
193+
// Look for AGENTS.md (OpenCode's instruction file) or opencode.json
194+
const agentsMdPath = join(sourceDir, 'AGENTS.md');
195195
const configPath = join(sourceDir, 'opencode.json');
196196

197197
let instructions = '';
198198
let config: Record<string, unknown> = {};
199199

200-
if (existsSync(instructionsPath)) {
201-
instructions = readFileSync(instructionsPath, 'utf-8');
202-
info('Found .opencode/instructions.md');
200+
if (existsSync(agentsMdPath)) {
201+
instructions = readFileSync(agentsMdPath, 'utf-8');
202+
info('Found AGENTS.md');
203203
} else {
204-
throw new Error('No .opencode/instructions.md found in source directory');
204+
throw new Error('No AGENTS.md found in source directory');
205205
}
206206

207207
if (existsSync(configPath)) {
@@ -213,8 +213,9 @@ function importFromOpenCode(sourcePath: string, targetDir: string): void {
213213

214214
const dirName = basename(sourceDir);
215215

216-
// Determine model from opencode.json
217-
const model = (config.model as string) || undefined;
216+
// Determine model from opencode.json (format: "provider/model-id")
217+
const rawModel = (config.model as string) || undefined;
218+
const model = rawModel?.includes('/') ? rawModel.split('/').slice(1).join('/') : rawModel;
218219
const agentYaml: Record<string, unknown> = {
219220
spec_version: '0.1.0',
220221
name: dirName.toLowerCase().replace(/[^a-z0-9-]/g, '-'),

src/runners/git.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,8 @@ function detectAdapter(agentDir: string, manifest: AgentManifest): string {
182182
info('Auto-detected adapter: github (from .github_models)');
183183
return 'github';
184184
}
185-
if (existsSync(join(agentDir, '.opencode')) || existsSync(join(agentDir, 'opencode.json'))) {
186-
info('Auto-detected adapter: opencode (from .opencode/ or opencode.json)');
185+
if (existsSync(join(agentDir, 'opencode.json'))) {
186+
info('Auto-detected adapter: opencode (from opencode.json)');
187187
return 'opencode';
188188
}
189189

src/runners/opencode.ts

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ export interface OpenCodeRunOptions {
1515
* Run a gitagent agent using OpenCode (sst/opencode).
1616
*
1717
* Creates a temporary workspace with:
18-
* - .opencode/instructions.md (agent instructions)
19-
* - opencode.json (provider + model config)
18+
* - AGENTS.md (agent instructions)
19+
* - opencode.json (provider + model config)
2020
*
2121
* Then launches `opencode` in that workspace. OpenCode reads both files
2222
* automatically on startup.
2323
*
24-
* Supports both interactive mode (no prompt) and single-shot mode (-p).
24+
* Supports both interactive mode (no prompt) and single-shot mode (`opencode run -p`).
2525
*/
2626
export function runWithOpenCode(agentDir: string, manifest: AgentManifest, options: OpenCodeRunOptions = {}): void {
2727
const exp = exportToOpenCode(agentDir);
@@ -30,50 +30,46 @@ export function runWithOpenCode(agentDir: string, manifest: AgentManifest, optio
3030
const workspaceDir = join(tmpdir(), `gitagent-opencode-${randomBytes(4).toString('hex')}`);
3131
mkdirSync(workspaceDir, { recursive: true });
3232

33-
// Write .opencode/instructions.md
34-
const instructionsDir = join(workspaceDir, '.opencode');
35-
mkdirSync(instructionsDir, { recursive: true });
36-
writeFileSync(join(instructionsDir, 'instructions.md'), exp.instructions, 'utf-8');
33+
// Write AGENTS.md at project root
34+
writeFileSync(join(workspaceDir, 'AGENTS.md'), exp.instructions, 'utf-8');
3735

3836
// Write opencode.json
3937
writeFileSync(join(workspaceDir, 'opencode.json'), JSON.stringify(exp.config, null, 2), 'utf-8');
4038

4139
info(`Workspace prepared at ${workspaceDir}`);
42-
info(` .opencode/instructions.md, opencode.json`);
40+
info(` AGENTS.md, opencode.json`);
4341
if (manifest.model?.preferred) {
4442
info(` Model: ${manifest.model.preferred}`);
4543
}
4644

4745
// Build opencode CLI args
4846
const args: string[] = [];
4947

50-
// If a prompt is provided, pass it for single-shot mode
48+
// Single-shot mode uses `opencode run --prompt "..."`, interactive is just `opencode`
5149
if (options.prompt) {
52-
args.push('--prompt', options.prompt);
50+
args.push('run', '--prompt', options.prompt);
5351
}
5452

5553
info(`Launching OpenCode agent "${manifest.name}"...`);
5654
if (!options.prompt) {
5755
info('Starting interactive mode. Type your messages to chat.');
5856
}
5957

60-
try {
61-
const result = spawnSync('opencode', args, {
62-
stdio: 'inherit',
63-
cwd: workspaceDir,
64-
env: { ...process.env },
65-
});
58+
const result = spawnSync('opencode', args, {
59+
stdio: 'inherit',
60+
cwd: workspaceDir,
61+
env: { ...process.env },
62+
});
6663

67-
if (result.error) {
68-
error(`Failed to launch OpenCode: ${result.error.message}`);
69-
info('Make sure OpenCode is installed: npm install -g opencode');
70-
info('Or: brew install sst/tap/opencode');
71-
process.exit(1);
72-
}
64+
// Cleanup temp workspace before exiting
65+
try { rmSync(workspaceDir, { recursive: true, force: true }); } catch { /* ignore */ }
7366

74-
process.exit(result.status ?? 0);
75-
} finally {
76-
// Cleanup temp workspace
77-
try { rmSync(workspaceDir, { recursive: true, force: true }); } catch { /* ignore */ }
67+
if (result.error) {
68+
error(`Failed to launch OpenCode: ${result.error.message}`);
69+
info('Make sure OpenCode is installed: npm install -g opencode');
70+
info('Or: brew install sst/tap/opencode');
71+
process.exit(1);
7872
}
73+
74+
process.exit(result.status ?? 0);
7975
}

0 commit comments

Comments
 (0)