Skip to content

Commit cdfb146

Browse files
committed
fix(agent): address CodeRabbit comments on Gemini target
- Make MANAGED_SECTION_BEGIN target-neutral ('testsprite agent install') instead of 'testsprite agent install codex'; add MANAGED_SECTION_BEGIN_LEGACY for backward-compat recognition of existing AGENTS.md files - classifySection now accepts both the new and legacy BEGIN sentinel - Add exit-code and JSON output assertions to Gemini e2e install test - Add managed-section user-content preservation test: pre-populates GEMINI.md, runs install twice, asserts prefix/suffix intact and section byte-identical across runs - Clarify renderForTarget unit test: sentinel wrapping is an install-time concern, not a renderForTarget concern
1 parent 3df4a37 commit cdfb146

4 files changed

Lines changed: 87 additions & 4 deletions

File tree

src/commands/agent.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
renderForTarget,
2323
renderOwnFileWithMarker,
2424
MANAGED_SECTION_BEGIN,
25+
MANAGED_SECTION_BEGIN_LEGACY,
2526
MANAGED_SECTION_END,
2627
} from '../lib/agent-targets.js';
2728

@@ -206,7 +207,8 @@ function classifySection(existing: string, section: string): SectionState {
206207

207208
for (let i = 0; i < lines.length; i++) {
208209
const stripped = (lines[i] ?? '').trimEnd();
209-
if (stripped === MANAGED_SECTION_BEGIN) beginLines.push(i);
210+
if (stripped === MANAGED_SECTION_BEGIN || stripped === MANAGED_SECTION_BEGIN_LEGACY)
211+
beginLines.push(i);
210212
else if (stripped === MANAGED_SECTION_END) endLines.push(i);
211213
}
212214

src/lib/agent-targets.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,13 @@ describe('renderForTarget("gemini")', () => {
409409

410410
it('renders the verify body content (managed-section, compact codex body)', () => {
411411
// Uses real bodies: gemini uses the codex contribution body (compact).
412+
// renderForTarget for managed-section returns the unwrapped body (no
413+
// sentinels) — sentinel wrapping happens at install time via buildSection.
412414
const gemini = renderForTarget('gemini', 'testsprite-verify');
413415
expect(gemini.content).toContain('testsprite test run');
416+
// Body must NOT be wrapped in sentinels at this layer
417+
expect(gemini.content).not.toContain(MANAGED_SECTION_BEGIN);
418+
expect(gemini.content).not.toContain(MANAGED_SECTION_END);
414419
});
415420
});
416421

src/lib/agent-targets.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,11 +300,20 @@ export const TARGETS: Record<AgentTarget, TargetSpec> = {
300300
},
301301
};
302302

303-
/** Sentinel pair that bounds our managed section in AGENTS.md. */
303+
/** Sentinel pair that bounds our managed section in AGENTS.md / GEMINI.md. */
304304
export const MANAGED_SECTION_BEGIN =
305-
'<!-- BEGIN TESTSPRITE AGENT SECTION (testsprite agent install codex) -->';
305+
'<!-- BEGIN TESTSPRITE AGENT SECTION (testsprite agent install) -->';
306306
export const MANAGED_SECTION_END = '<!-- END TESTSPRITE AGENT SECTION -->';
307307

308+
/**
309+
* Legacy sentinel written by versions prior to this change.
310+
* Kept for backward-compatibility: existing AGENTS.md files that contain
311+
* the old Codex-labelled marker are still recognised as a valid managed
312+
* section during classify / replace operations.
313+
*/
314+
export const MANAGED_SECTION_BEGIN_LEGACY =
315+
'<!-- BEGIN TESTSPRITE AGENT SECTION (testsprite agent install codex) -->';
316+
308317
// ---------------------------------------------------------------------------
309318
// Install marker (stale-skill detection, issue #123)
310319
// ---------------------------------------------------------------------------

test/e2e/agent-install.e2e.test.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,23 @@ describe('content integrity', () => {
274274

275275
it('gemini GEMINI.md contains ONE managed section with verify and onboard content', () => {
276276
const tmpDir = freshTmpDir();
277-
runCli(['agent', 'install', '--target=gemini', '--dir', tmpDir, '--output', 'json']);
277+
const result = runCli(['agent', 'install', '--target=gemini', '--dir', tmpDir, '--output', 'json']);
278+
279+
// Exit code must be 0
280+
expect(result.status, 'gemini install: exit code').toBe(0);
281+
282+
// stdout must be valid JSON with the expected entry
283+
const parsed = JSON.parse(result.stdout) as Array<{
284+
target: string;
285+
path: string;
286+
action: string;
287+
skills: string[];
288+
}>;
289+
expect(Array.isArray(parsed), 'output should be a JSON array').toBe(true);
290+
const entry = parsed.find(r => r.target === 'gemini');
291+
expect(entry, 'gemini entry in JSON output').toBeDefined();
292+
expect(entry!.action, 'gemini action').toBe('section-installed');
293+
expect(entry!.path).toBe(TARGETS.gemini.path);
278294

279295
const filePath = join(tmpDir, TARGETS.gemini.path);
280296
const content = readFileSync(filePath, 'utf8');
@@ -302,6 +318,57 @@ describe('content integrity', () => {
302318
// (d) No frontmatter fence -- GEMINI.md is plain prose
303319
expect(content.startsWith('---'), 'gemini: must NOT start with ---').toBe(false);
304320
});
321+
322+
it('gemini managed-section preserves user content outside sentinels across re-runs', () => {
323+
const tmpDir = freshTmpDir();
324+
const filePath = join(tmpDir, TARGETS.gemini.path);
325+
326+
// Pre-populate GEMINI.md with user content above and below where the
327+
// managed section will land.
328+
const userPrefix = '# My Project\n\nSome hand-written instructions.\n\n';
329+
const userSuffix = '\n## Extra notes\n\nMore user content here.\n';
330+
writeFileSync(filePath, userPrefix + userSuffix, 'utf8');
331+
332+
// First install: merges managed section into the file
333+
const first = runCli(['agent', 'install', '--target=gemini', '--dir', tmpDir, '--output', 'json']);
334+
expect(first.status, 'first install: exit code').toBe(0);
335+
336+
const afterFirst = readFileSync(filePath, 'utf8');
337+
expect(afterFirst).toContain(userPrefix.trimEnd());
338+
expect(afterFirst).toContain(userSuffix.trim());
339+
expect(afterFirst).toContain(MANAGED_SECTION_BEGIN);
340+
expect(afterFirst).toContain(MANAGED_SECTION_END);
341+
342+
// Capture the managed section content for comparison across runs
343+
const sectionAfterFirst = afterFirst.slice(
344+
afterFirst.indexOf(MANAGED_SECTION_BEGIN),
345+
afterFirst.indexOf(MANAGED_SECTION_END) + MANAGED_SECTION_END.length,
346+
);
347+
348+
// Second install (re-run): must be idempotent and preserve user content
349+
const second = runCli(['agent', 'install', '--target=gemini', '--dir', tmpDir, '--output', 'json']);
350+
expect(second.status, 'second install: exit code').toBe(0);
351+
352+
const afterSecond = readFileSync(filePath, 'utf8');
353+
354+
// User content outside sentinels must be intact
355+
expect(afterSecond).toContain(userPrefix.trimEnd());
356+
expect(afterSecond).toContain(userSuffix.trim());
357+
358+
// Exactly one pair of sentinels after re-run
359+
const escRe2 = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
360+
const beginCount2 = (afterSecond.match(new RegExp(escRe2(MANAGED_SECTION_BEGIN), 'g')) ?? []).length;
361+
const endCount2 = (afterSecond.match(new RegExp(escRe2(MANAGED_SECTION_END), 'g')) ?? []).length;
362+
expect(beginCount2, 'exactly one BEGIN sentinel after re-run').toBe(1);
363+
expect(endCount2, 'exactly one END sentinel after re-run').toBe(1);
364+
365+
// Managed section content must be byte-identical between runs
366+
const sectionAfterSecond = afterSecond.slice(
367+
afterSecond.indexOf(MANAGED_SECTION_BEGIN),
368+
afterSecond.indexOf(MANAGED_SECTION_END) + MANAGED_SECTION_END.length,
369+
);
370+
expect(sectionAfterSecond).toBe(sectionAfterFirst);
371+
});
305372
});
306373
// ---------------------------------------------------------------------------
307374
// 3. Idempotent re-run

0 commit comments

Comments
 (0)