From ddb5978a787885c9f7157bd0e99bcaaf1184cdc3 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Tue, 28 Jul 2026 21:29:06 +0530 Subject: [PATCH 1/8] test: define canonical skill asset validation --- scripts/validate-repository-assets.ts | 14 ++- scripts/validate-skill-assets.ts | 97 +++++++++++++++++++ .../validate-repository-assets.test.ts | 28 +++++- .../scripts/validate-skill-assets.test.ts | 72 ++++++++++++++ 4 files changed, 208 insertions(+), 3 deletions(-) create mode 100644 scripts/validate-skill-assets.ts create mode 100644 tests/unit/scripts/validate-skill-assets.test.ts diff --git a/scripts/validate-repository-assets.ts b/scripts/validate-repository-assets.ts index 23a0268..662dc1d 100644 --- a/scripts/validate-repository-assets.ts +++ b/scripts/validate-repository-assets.ts @@ -1,6 +1,7 @@ import { existsSync, readFileSync, readdirSync } from 'node:fs'; import { isAbsolute, join, relative, resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; +import { validateSkillAssets } from './validate-skill-assets.js'; function fail(msg: string): never { throw new Error(`[ASSET VALIDATION FAILURE] ${msg}`); @@ -124,6 +125,13 @@ export function validateRepositoryAssets(options: ValidateRepositoryAssetsOption 'docs/mcp-tools.md', 'docs/cli-reference.md', 'docs/session-semantics.md', + 'docs/agent-skills.md', + 'skills/relay-capture/SKILL.md', + 'skills/relay-session-review/SKILL.md', + 'skills/fixtures/capture-positive.md', + 'skills/fixtures/capture-negative.md', + 'skills/fixtures/session-review-positive.md', + 'skills/fixtures/session-review-negative.md', 'tests/fixtures/contracts/capture-success.json', 'tests/fixtures/contracts/capture-duplicate-warning.json', 'tests/fixtures/contracts/validation-error.json', @@ -193,6 +201,8 @@ export function validateRepositoryAssets(options: ValidateRepositoryAssetsOption const allFiles = walkFiles(rootDir); + validateSkillAssets({ rootDir }); + validateJsonFiles(allFiles); validatePlaceholders(allFiles); validateMarkdownLinks( @@ -200,8 +210,8 @@ export function validateRepositoryAssets(options: ValidateRepositoryAssetsOption readFileSync(join(rootDir, 'README.md'), 'utf-8'), ); - // 4. No SKILL.md or agent configs in #1 - const forbidden = ['SKILL.md', 'agent/skills', 'agent/mcp']; + // 4. No legacy agent integration roots + const forbidden = ['agent/skills', 'agent/mcp']; for (const f of forbidden) { if (existsSync(join(rootDir, f))) { fail(`Forbidden asset for Issue #1 present: ${f}`); diff --git a/scripts/validate-skill-assets.ts b/scripts/validate-skill-assets.ts new file mode 100644 index 0000000..6228857 --- /dev/null +++ b/scripts/validate-skill-assets.ts @@ -0,0 +1,97 @@ +import { existsSync, readFileSync } from 'node:fs'; +import { join, resolve } from 'node:path'; + +const canonicalSkillPaths = [ + 'skills/relay-capture/SKILL.md', + 'skills/relay-session-review/SKILL.md', +] as const; + +const fixturePaths = [ + 'skills/fixtures/capture-positive.md', + 'skills/fixtures/capture-negative.md', + 'skills/fixtures/session-review-positive.md', + 'skills/fixtures/session-review-negative.md', +] as const; + +interface SkillFixtureCase { + readonly id: string; + readonly expected: 'ACCEPT' | 'REJECT'; + readonly scenario: string; + readonly agentAction: string; + readonly reason: string; +} + +export interface ValidateSkillAssetsOptions { + readonly rootDir?: string; +} + +function fail(message: string): never { + throw new Error(`[SKILL ASSET VALIDATION FAILURE] ${message}`); +} + +function requiredSection(caseContent: string, heading: string, fixturePath: string): string { + const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const match = new RegExp( + `^### ${escapedHeading}\\r?\\n([\\s\\S]*?)(?=^### |^## |\\s*$)`, + 'm', + ).exec(caseContent); + const value = match?.[1]?.trim(); + if (!value) { + fail(`${fixturePath} case is missing a non-empty ${heading} section.`); + } + return value; +} + +function parseFixtureCases(fixturePath: string, content: string): readonly SkillFixtureCase[] { + const headings = [...content.matchAll(/^## ([^\r\n#]+)\r?$/gm)]; + if (headings.length === 0) { + fail(`${fixturePath} contains zero fixture cases.`); + } + + return headings.map((heading, index) => { + const id = heading[1]?.trim(); + if (!id) { + fail(`${fixturePath} contains a fixture case without an ID.`); + } + const start = (heading.index ?? 0) + heading[0].length; + const end = headings[index + 1]?.index ?? content.length; + const caseContent = content.slice(start, end); + const expectedMatches = [...caseContent.matchAll(/^Expected: (ACCEPT|REJECT)\r?$/gm)]; + if (expectedMatches.length !== 1 || !expectedMatches[0]?.[1]) { + fail(`${fixturePath} case ${id} must contain exactly one Expected: ACCEPT or Expected: REJECT line.`); + } + + return { + id, + expected: expectedMatches[0][1] as 'ACCEPT' | 'REJECT', + scenario: requiredSection(caseContent, 'Scenario', fixturePath), + agentAction: requiredSection(caseContent, 'Agent action', fixturePath), + reason: requiredSection(caseContent, 'Reason', fixturePath), + }; + }); +} + +export function validateSkillAssets(options: ValidateSkillAssetsOptions = {}): void { + const rootDir = options.rootDir ? resolve(options.rootDir) : process.cwd(); + + for (const path of [...canonicalSkillPaths, ...fixturePaths]) { + if (!existsSync(join(rootDir, path))) { + fail(`Required canonical skill asset missing: ${path}`); + } + } + + const seenIds = new Set(); + for (const fixturePath of fixturePaths) { + const cases = parseFixtureCases(fixturePath, readFileSync(join(rootDir, fixturePath), 'utf-8')); + const expected = fixturePath.endsWith('-positive.md') ? 'ACCEPT' : 'REJECT'; + for (const fixtureCase of cases) { + if (fixtureCase.expected !== expected) { + fail(`${fixturePath} case ${fixtureCase.id} must be ${expected}.`); + } + if (seenIds.has(fixtureCase.id)) { + fail(`Duplicate fixture case ID: ${fixtureCase.id}.`); + } + seenIds.add(fixtureCase.id); + } + } +} diff --git a/tests/unit/scripts/validate-repository-assets.test.ts b/tests/unit/scripts/validate-repository-assets.test.ts index af8f68b..fb3fdc8 100644 --- a/tests/unit/scripts/validate-repository-assets.test.ts +++ b/tests/unit/scripts/validate-repository-assets.test.ts @@ -14,6 +14,9 @@ function createFixtureRoot(): string { 'src/interfaces/http', 'src/interfaces/cli', 'web/src', + 'skills/relay-capture', + 'skills/relay-session-review', + 'skills/fixtures', ]; for (const dir of requiredDirs) { @@ -43,7 +46,7 @@ function createFixtureRoot(): string { ); writeFileSync( join(rootDir, 'README.md'), - '# Relay\n\n[Decision](docs/decisions/0001-product-and-architecture.md)\n\n`node dist/cli/main.js`\n', + '# Relay\n\n[Decision](docs/decisions/0001-product-and-architecture.md)\n\n`node dist/cli/main.js`\n\n[Agent skills](docs/agent-skills.md)\n\n[Capture skill](skills/relay-capture/SKILL.md)\n\n[Review skill](skills/relay-session-review/SKILL.md)\n', ); writeFileSync(join(rootDir, 'src/application/health/get-health.ts'), 'export {};\n'); writeFileSync(join(rootDir, 'src/database/connection.ts'), 'export {};\n'); @@ -65,6 +68,21 @@ function createFixtureRoot(): string { '# CLI reference\n\n`node dist/cli/main.js`\n', ); writeFileSync(join(rootDir, 'docs/session-semantics.md'), '# Session semantics\n'); + writeFileSync(join(rootDir, 'docs/agent-skills.md'), '# Agent skills\n'); + writeFileSync(join(rootDir, 'skills/relay-capture/SKILL.md'), '# Relay Capture\n'); + writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), '# Relay Session Review\n'); + for (const [index, filename] of [ + 'capture-positive.md', + 'capture-negative.md', + 'session-review-positive.md', + 'session-review-negative.md', + ].entries()) { + const expected = filename.includes('positive') ? 'ACCEPT' : 'REJECT'; + writeFileSync( + join(rootDir, 'skills/fixtures', filename), + `## CASE-${String(index + 1).padStart(3, '0')}\n\nExpected: ${expected}\n\n### Scenario\nScenario\n\n### Agent action\nAction\n\n### Reason\nReason\n`, + ); + } mkdirSync(join(rootDir, 'src/interfaces/contracts'), { recursive: true }); for (const filename of [ 'contract-version.ts', @@ -150,4 +168,12 @@ describe('validateRepositoryAssets', () => { expect(() => validateRepositoryAssets({ rootDir })).toThrow(/CLI executable|dist\/cli/i); }); + + it('accepts canonical skills but rejects legacy agent policy roots', () => { + const rootDir = createFixtureRoot(); + createdRoots.push(rootDir); + mkdirSync(join(rootDir, 'agent/skills'), { recursive: true }); + + expect(() => validateRepositoryAssets({ rootDir })).toThrow(/legacy|agent\/skills/i); + }); }); diff --git a/tests/unit/scripts/validate-skill-assets.test.ts b/tests/unit/scripts/validate-skill-assets.test.ts new file mode 100644 index 0000000..c71ed25 --- /dev/null +++ b/tests/unit/scripts/validate-skill-assets.test.ts @@ -0,0 +1,72 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { validateSkillAssets } from '../../../scripts/validate-skill-assets.js'; + +const fixtureFiles = [ + 'skills/fixtures/capture-positive.md', + 'skills/fixtures/capture-negative.md', + 'skills/fixtures/session-review-positive.md', + 'skills/fixtures/session-review-negative.md', +] as const; + +function fixtureCase(id: string, expected: 'ACCEPT' | 'REJECT'): string { + return `## ${id}\n\nExpected: ${expected}\n\n### Scenario\nA concrete scenario.\n\n### Agent action\nA bounded agent action.\n\n### Reason\nA deterministic policy reason.\n`; +} + +function createValidSkillFixtureRoot(): string { + const rootDir = mkdtempSync(join(tmpdir(), 'relay-skill-validator-')); + + for (const path of [ + 'skills/relay-capture', + 'skills/relay-session-review', + 'skills/fixtures', + ]) { + mkdirSync(join(rootDir, path), { recursive: true }); + } + + writeFileSync(join(rootDir, 'skills/relay-capture/SKILL.md'), '# Relay Capture\n'); + writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), '# Relay Session Review\n'); + for (const [index, path] of fixtureFiles.entries()) { + writeFileSync( + join(rootDir, path), + fixtureCase( + `CASE-${String(index + 1).padStart(3, '0')}`, + path.includes('positive') ? 'ACCEPT' : 'REJECT', + ), + ); + } + + return rootDir; +} + +describe('validateSkillAssets', () => { + const createdRoots: string[] = []; + + afterEach(() => { + for (const rootDir of createdRoots) { + rmSync(rootDir, { recursive: true, force: true }); + } + createdRoots.splice(0, createdRoots.length); + }); + + it('requires both canonical skill files and four fixture files', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + rmSync(join(rootDir, 'skills/relay-capture/SKILL.md')); + + expect(() => validateSkillAssets({ rootDir })).toThrow(/relay-capture\/SKILL\.md/i); + }); + + it('rejects malformed fixture cases', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + writeFileSync( + join(rootDir, 'skills/fixtures/capture-positive.md'), + '## CAPTURE-BROKEN-001\n\nExpected: ACCEPT\n', + ); + + expect(() => validateSkillAssets({ rootDir })).toThrow(/Scenario|Agent action|Reason/i); + }); +}); From 9067122f035f1b24d5e829b8cfca34680e88b1f7 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Tue, 28 Jul 2026 21:32:30 +0530 Subject: [PATCH 2/8] feat: add canonical Relay capture skill --- scripts/validate-skill-assets.ts | 38 +++++++++ skills/fixtures/capture-negative.md | 77 +++++++++++++++++++ skills/fixtures/capture-positive.md | 51 ++++++++++++ skills/relay-capture/SKILL.md | 47 +++++++++++ .../validate-repository-assets.test.ts | 5 +- .../scripts/validate-skill-assets.test.ts | 13 +++- 6 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 skills/fixtures/capture-negative.md create mode 100644 skills/fixtures/capture-positive.md create mode 100644 skills/relay-capture/SKILL.md diff --git a/scripts/validate-skill-assets.ts b/scripts/validate-skill-assets.ts index 6228857..8eeeadf 100644 --- a/scripts/validate-skill-assets.ts +++ b/scripts/validate-skill-assets.ts @@ -71,6 +71,42 @@ function parseFixtureCases(fixturePath: string, content: string): readonly Skill }); } +function validateContains(content: string, pattern: RegExp, label: string): void { + if (!pattern.test(content)) { + fail(`Canonical skill is missing required policy: ${label}.`); + } +} + +function validateCaptureSkill(content: string): void { + for (const section of [ + 'Purpose', + 'When to capture', + 'Adapter selection', + 'Session and provenance', + 'Capture procedure', + 'Duplicate handling', + 'Context safety', + 'Autonomy boundaries', + 'Do not capture', + ]) { + validateContains(content, new RegExp(`^## ${section}$`, 'mi'), section); + } + for (const [pattern, label] of [ + [/concrete,? actionable follow-up/i, 'concrete actionable follow-up'], + [/MCP.*preferred/i, 'MCP preference'], + [/CLI.*fallback/i, 'CLI fallback'], + [/--output json/i, 'CLI JSON output'], + [/same adapter|one adapter/i, 'one adapter per workflow'], + [/session ID/i, 'session ID'], + [/INBOX/i, 'INBOX capture'], + [/duplicate.*advisory/i, 'advisory duplicate handling'], + [/continue.*original work/i, 'continue original work'], + [/must not.*(?:edit|triage|start|complete|archive)/i, 'autonomy boundary'], + ] as const) { + validateContains(content, pattern, label); + } +} + export function validateSkillAssets(options: ValidateSkillAssetsOptions = {}): void { const rootDir = options.rootDir ? resolve(options.rootDir) : process.cwd(); @@ -80,6 +116,8 @@ export function validateSkillAssets(options: ValidateSkillAssetsOptions = {}): v } } + validateCaptureSkill(readFileSync(join(rootDir, canonicalSkillPaths[0]), 'utf-8')); + const seenIds = new Set(); for (const fixturePath of fixturePaths) { const cases = parseFixtureCases(fixturePath, readFileSync(join(rootDir, fixturePath), 'utf-8')); diff --git a/skills/fixtures/capture-negative.md b/skills/fixtures/capture-negative.md new file mode 100644 index 0000000..2b194f0 --- /dev/null +++ b/skills/fixtures/capture-negative.md @@ -0,0 +1,77 @@ +## CAPTURE-SPECULATION-001 + +Expected: REJECT + +### Scenario +The agent has a possible future enhancement idea. + +### Agent action +Capture every thought as a Relay task. + +### Reason +Speculation is not concrete follow-up work. + +## CAPTURE-SENSITIVE-002 + +Expected: REJECT + +### Scenario +A diagnostic includes a credential and a large stack trace. + +### Agent action +Attach the prompt, source file, credential, and stack trace as source context. + +### Reason +Sensitive and oversized context is forbidden. + +## CAPTURE-MUTATION-003 + +Expected: REJECT + +### Scenario +A possible duplicate exists. + +### Agent action +Silently complete or archive the existing task. + +### Reason +Autonomous existing-task mutation is forbidden. + +## CAPTURE-TRIAGE-004 + +Expected: REJECT + +### Scenario +An autonomous capture succeeds. + +### Agent action +Move it from INBOX to ACTIVE. + +### Reason +Autonomous captures cannot leave INBOX. + +## CAPTURE-SESSION-005 + +Expected: REJECT + +### Scenario +Two unrelated concurrent sessions run in one workspace. + +### Agent action +Reuse one session ID for both. + +### Reason +Concurrent sessions must remain isolated. + +## CAPTURE-ADAPTER-006 + +Expected: REJECT + +### Scenario +MCP is available throughout capture. + +### Agent action +Use MCP for lookup and CLI for capture without a failure or explicit reason. + +### Reason +One workflow retains one adapter. diff --git a/skills/fixtures/capture-positive.md b/skills/fixtures/capture-positive.md new file mode 100644 index 0000000..2b04b9e --- /dev/null +++ b/skills/fixtures/capture-positive.md @@ -0,0 +1,51 @@ +## CAPTURE-ACTIONABLE-001 + +Expected: ACCEPT + +### Scenario +A missing regression test is discovered while implementing session expiry. + +### Agent action +Use `task_find_similar`, capture a concise INBOX task with the active session ID, then continue the original work. + +### Reason +The regression gap is concrete, actionable, and safely deferred. + +## CAPTURE-DUPLICATE-002 + +Expected: ACCEPT + +### Scenario +Relay returns a possible duplicate warning. + +### Agent action +Keep the successful capture and record the warning as advisory without mutating the existing task. + +### Reason +Duplicate candidates never silently suppress capture. + +## CAPTURE-CLI-FALLBACK-003 + +Expected: ACCEPT + +### Scenario +MCP is unavailable for a script. + +### Agent action +Use `task find-similar --output json` and `task capture --output json` through the same CLI adapter. + +### Reason +CLI JSON is the deterministic fallback. + +## CAPTURE-CONTEXT-004 + +Expected: ACCEPT + +### Scenario +A capture needs source context. + +### Agent action +Store `session expiry integration tests` as the source context. + +### Reason +The reference is concise and does not include code or transcripts. diff --git a/skills/relay-capture/SKILL.md b/skills/relay-capture/SKILL.md new file mode 100644 index 0000000..c4f0839 --- /dev/null +++ b/skills/relay-capture/SKILL.md @@ -0,0 +1,47 @@ +--- +name: relay-capture +description: Use when concrete follow-up work is discovered while performing another task and Relay is available through MCP or its deterministic CLI. +--- + +# Relay Capture + +## Purpose + +Capture a concrete, actionable follow-up without derailing the current activity. Relay capabilities perform persistence and validation; this skill governs agent behaviour. + +## When to capture + +Capture work discovered during another task when it is concrete, independently actionable, safely deferrable, and should persist beyond the current response. Do not capture speculative ideas, vague reminders, work already being completed, or status notes. + +## Adapter selection + +MCP is preferred for supported interactive clients. CLI is the deterministic fallback for unsupported clients, scripts, debugging, or explicit one-shot use. Keep the same adapter for one workflow unless it fails or becomes unavailable. In CLI mode, use `--output json` and parse only JSON, never decorative terminal output. MCP and CLI use the same Relay database and contracts. + +## Session and provenance + +Generate one valid opaque session ID for the active agent session or reuse its already-established ID. Retain that exact session ID for every capture and final review; never reuse it across unrelated concurrent agents or shells. Provide a concise title, agent name, exact session ID, workspace when known, and limited source context. + +## Capture procedure + +1. Decide that the follow-up is concrete and actionable. +2. When practical, use `task_find_similar` or the matching CLI command before capture. +3. Use `task_capture`, or the documented source-checkout CLI invocation for `task capture --output json`; do not supply status or provenance fields. +4. Retain the returned task ID and warnings, then continue the original work without separately interrupting the user after every capture. + +## Duplicate handling + +Duplicate candidates are advisory. Do not suppress capture solely because a candidate exists, and do not merge or mutate existing tasks unless the user explicitly directs it. + +## Context safety + +Store only concise source context that identifies where or why the work was found. Never store prompts, transcripts, source files, secrets, credentials, tokens, large stack traces, logs, or oversized copied context. + +## Autonomy boundaries + +An agent may autonomously create only a new Relay task in `INBOX`. It must not edit, triage, start, complete, archive, delete, merge, or move any task, including a new capture, without explicit user direction in the active conversation. + +## Do not capture + +Do not capture every thought, casual ideas, or anything that cannot be acted on later without reconstructing the conversation. + +See [MCP tool contracts](../../docs/mcp-tools.md), [CLI contract reference](../../docs/cli-reference.md), and [session semantics](../../docs/session-semantics.md). diff --git a/tests/unit/scripts/validate-repository-assets.test.ts b/tests/unit/scripts/validate-repository-assets.test.ts index fb3fdc8..6bd4e46 100644 --- a/tests/unit/scripts/validate-repository-assets.test.ts +++ b/tests/unit/scripts/validate-repository-assets.test.ts @@ -69,7 +69,10 @@ function createFixtureRoot(): string { ); writeFileSync(join(rootDir, 'docs/session-semantics.md'), '# Session semantics\n'); writeFileSync(join(rootDir, 'docs/agent-skills.md'), '# Agent skills\n'); - writeFileSync(join(rootDir, 'skills/relay-capture/SKILL.md'), '# Relay Capture\n'); + writeFileSync( + join(rootDir, 'skills/relay-capture/SKILL.md'), + `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nUse the exact session ID.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, + ); writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), '# Relay Session Review\n'); for (const [index, filename] of [ 'capture-positive.md', diff --git a/tests/unit/scripts/validate-skill-assets.test.ts b/tests/unit/scripts/validate-skill-assets.test.ts index c71ed25..f671c4d 100644 --- a/tests/unit/scripts/validate-skill-assets.test.ts +++ b/tests/unit/scripts/validate-skill-assets.test.ts @@ -26,7 +26,10 @@ function createValidSkillFixtureRoot(): string { mkdirSync(join(rootDir, path), { recursive: true }); } - writeFileSync(join(rootDir, 'skills/relay-capture/SKILL.md'), '# Relay Capture\n'); + writeFileSync( + join(rootDir, 'skills/relay-capture/SKILL.md'), + `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nUse the exact session ID.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, + ); writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), '# Relay Session Review\n'); for (const [index, path] of fixtureFiles.entries()) { writeFileSync( @@ -69,4 +72,12 @@ describe('validateSkillAssets', () => { expect(() => validateSkillAssets({ rootDir })).toThrow(/Scenario|Agent action|Reason/i); }); + + it('requires the Capture policy sections and invariants', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + writeFileSync(join(rootDir, 'skills/relay-capture/SKILL.md'), '# Relay Capture\n'); + + expect(() => validateSkillAssets({ rootDir })).toThrow(/Purpose|When to capture|MCP|INBOX/i); + }); }); From 6bd2c9b8587b128793e40d771e313aebc0b2a7cf Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Tue, 28 Jul 2026 21:34:00 +0530 Subject: [PATCH 3/8] feat: add canonical Relay session review skill --- scripts/validate-skill-assets.ts | 14 ++++ skills/fixtures/session-review-negative.md | 77 +++++++++++++++++++ skills/fixtures/session-review-positive.md | 64 +++++++++++++++ skills/relay-session-review/SKILL.md | 40 ++++++++++ .../validate-repository-assets.test.ts | 2 +- .../scripts/validate-skill-assets.test.ts | 2 +- 6 files changed, 197 insertions(+), 2 deletions(-) create mode 100644 skills/fixtures/session-review-negative.md create mode 100644 skills/fixtures/session-review-positive.md create mode 100644 skills/relay-session-review/SKILL.md diff --git a/scripts/validate-skill-assets.ts b/scripts/validate-skill-assets.ts index 8eeeadf..45c4608 100644 --- a/scripts/validate-skill-assets.ts +++ b/scripts/validate-skill-assets.ts @@ -107,6 +107,19 @@ function validateCaptureSkill(content: string): void { } } +function validateReviewSkill(content: string): void { + for (const section of [ + 'Purpose', 'When to review', 'Session lookup', 'Review presentation', + 'User-directed actions', 'Unresolved captures', 'Adapter selection', 'Prohibited behaviour', + ]) validateContains(content, new RegExp(`^## ${section}$`, 'mi'), section); + for (const [pattern, label] of [ + [/before final completion/i, 'pre-completion review'], [/exact active session ID/i, 'exact session ID'], + [/completed.*archived|archived.*completed/i, 'all-status review'], [/explicit user direction/i, 'explicit user direction'], + [/intent-specific/i, 'intent-specific actions'], [/unresolved.*INBOX/i, 'unresolved INBOX'], + [/never infer.*(?:timer|inactivity|process exit)/i, 'no timer inference'], [/never mix.*session/i, 'session isolation'], + ] as const) validateContains(content, pattern, label); +} + export function validateSkillAssets(options: ValidateSkillAssetsOptions = {}): void { const rootDir = options.rootDir ? resolve(options.rootDir) : process.cwd(); @@ -117,6 +130,7 @@ export function validateSkillAssets(options: ValidateSkillAssetsOptions = {}): v } validateCaptureSkill(readFileSync(join(rootDir, canonicalSkillPaths[0]), 'utf-8')); + validateReviewSkill(readFileSync(join(rootDir, canonicalSkillPaths[1]), 'utf-8')); const seenIds = new Set(); for (const fixturePath of fixturePaths) { diff --git a/skills/fixtures/session-review-negative.md b/skills/fixtures/session-review-negative.md new file mode 100644 index 0000000..9c6292d --- /dev/null +++ b/skills/fixtures/session-review-negative.md @@ -0,0 +1,77 @@ +## REVIEW-OMITTED-001 + +Expected: REJECT + +### Scenario +The agent is ready to finish. + +### Agent action +Finish without querying captured tasks. + +### Reason +Final review must not be omitted. + +## REVIEW-WRONG-SESSION-002 + +Expected: REJECT + +### Scenario +Another agent ran concurrently. + +### Agent action +Query its session or merge both session IDs. + +### Reason +Sessions must remain isolated. + +## REVIEW-SILENT-MUTATION-003 + +Expected: REJECT + +### Scenario +Several captures remain unresolved. + +### Agent action +Move all to Active or archive them without choices. + +### Reason +Disposition requires explicit user direction. + +## REVIEW-INBOX-ONLY-004 + +Expected: REJECT + +### Scenario +Relay returns DONE and ARCHIVED captures. + +### Agent action +Hide them and present only INBOX tasks. + +### Reason +Review includes all returned statuses. + +## REVIEW-TIMER-005 + +Expected: REJECT + +### Scenario +The user has been inactive. + +### Agent action +Infer wrap-up from timer or process exit. + +### Reason +Completion is never inferred. + +## REVIEW-GENERIC-MUTATION-006 + +Expected: REJECT + +### Scenario +The user requests a status change. + +### Agent action +Use a generic update command. + +### Reason +Only intent-specific capabilities are allowed. diff --git a/skills/fixtures/session-review-positive.md b/skills/fixtures/session-review-positive.md new file mode 100644 index 0000000..3c8a4f8 --- /dev/null +++ b/skills/fixtures/session-review-positive.md @@ -0,0 +1,64 @@ +## REVIEW-ACTIVE-SESSION-001 + +Expected: ACCEPT + +### Scenario +The agent is about to give final completion. + +### Agent action +Call `session_captures_list` with the exact active session ID and report INBOX, DONE, and ARCHIVED results. + +### Reason +Exact-session review includes every status returned by Relay. + +## REVIEW-EXPLICIT-ACTIONS-002 + +Expected: ACCEPT + +### Scenario +The user selects Active, Complete, and Archive actions. + +### Agent action +Use only `task_triage`, `task_complete`, and `task_archive` for the selected IDs. + +### Reason +Mutations require explicit intent-specific direction. + +## REVIEW-UNRESOLVED-003 + +Expected: ACCEPT + +### Scenario +The user chooses one of several captures. + +### Agent action +Leave every unselected capture unchanged in INBOX. + +### Reason +Unresolved work is not silently mutated. + +## REVIEW-PREEXISTING-004 + +Expected: ACCEPT + +### Scenario +A duplicate warning points to an older task. + +### Agent action +Present it separately from tasks returned by the active-session query. + +### Reason +Duplicate candidates are pre-existing unless captured in the exact session. + +## REVIEW-CLI-FALLBACK-005 + +Expected: ACCEPT + +### Scenario +MCP is unavailable. + +### Agent action +Use `session captures --session --output json` and JSON mutation commands. + +### Reason +CLI is the deterministic fallback. diff --git a/skills/relay-session-review/SKILL.md b/skills/relay-session-review/SKILL.md new file mode 100644 index 0000000..9708faa --- /dev/null +++ b/skills/relay-session-review/SKILL.md @@ -0,0 +1,40 @@ +--- +name: relay-session-review +description: Use before final completion or when the user asks to wrap up, review, or show Relay tasks captured in the active agent session. +--- + +# Relay Session Review + +## Purpose + +Before final completion, review every Relay task captured with the exact active session ID. Present captures compactly, distinguish pre-existing duplicate candidates, and mutate only on explicit user direction. + +## When to review + +Review before final completion or when the user asks to wrap up, show captured tasks, or review follow-ups. Never infer completion from a timer, inactivity, or process exit. + +## Session lookup + +Use `session_captures_list` with the exact active session ID, or the documented source-checkout CLI `session captures --session --output json` fallback. Relay's ordered result is authoritative: include completed and archived captures as well as INBOX tasks. Never mix tasks from another session ID. + +## Review presentation + +Present returned captures with ID, title, and current status. Label duplicate candidates separately as pre-existing unless the exact-session query also returned them. Do not reconstruct captures from remembered IDs or timestamps. + +## User-directed actions + +Obtain explicit user direction for each selected task. Use only intent-specific capabilities: `task_edit`, `task_triage`, `task_start`, `task_complete`, or `task_archive`. Report `NO_CHANGE`, conflicts, archived-task restrictions, and errors accurately; never invent success. + +## Unresolved captures + +Do not mutate unselected tasks. Unresolved captures remain in `INBOX`. + +## Adapter selection + +Keep the adapter used for capture unless it is concretely unavailable. MCP is preferred; CLI fallback always uses `--output json` and parses structured output only. + +## Prohibited behaviour + +Never query a guessed session, omit review before final completion when captures may exist, silently apply dispositions, use a generic status mutation, hide completed or archived captures, or infer completion from timer, inactivity, or process exit. + +See [MCP tool contracts](../../docs/mcp-tools.md), [CLI contract reference](../../docs/cli-reference.md), and [session semantics](../../docs/session-semantics.md). diff --git a/tests/unit/scripts/validate-repository-assets.test.ts b/tests/unit/scripts/validate-repository-assets.test.ts index 6bd4e46..14197a1 100644 --- a/tests/unit/scripts/validate-repository-assets.test.ts +++ b/tests/unit/scripts/validate-repository-assets.test.ts @@ -73,7 +73,7 @@ function createFixtureRoot(): string { join(rootDir, 'skills/relay-capture/SKILL.md'), `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nUse the exact session ID.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, ); - writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), '# Relay Session Review\n'); + writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`); for (const [index, filename] of [ 'capture-positive.md', 'capture-negative.md', diff --git a/tests/unit/scripts/validate-skill-assets.test.ts b/tests/unit/scripts/validate-skill-assets.test.ts index f671c4d..5fc0943 100644 --- a/tests/unit/scripts/validate-skill-assets.test.ts +++ b/tests/unit/scripts/validate-skill-assets.test.ts @@ -30,7 +30,7 @@ function createValidSkillFixtureRoot(): string { join(rootDir, 'skills/relay-capture/SKILL.md'), `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nUse the exact session ID.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, ); - writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), '# Relay Session Review\n'); + writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`); for (const [index, path] of fixtureFiles.entries()) { writeFileSync( join(rootDir, path), From 276499bd09543191812f1b928d733ac554ca5aa7 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Tue, 28 Jul 2026 21:36:39 +0530 Subject: [PATCH 4/8] docs: publish canonical Relay skill guidance --- README.md | 4 ++ docs/agent-skills.md | 9 +++ scripts/validate-repository-assets.ts | 9 +++ scripts/validate-skill-assets.ts | 55 +++++++++++++++---- .../validate-repository-assets.test.ts | 13 ++++- .../scripts/validate-skill-assets.test.ts | 20 +++++-- 6 files changed, 92 insertions(+), 18 deletions(-) create mode 100644 docs/agent-skills.md diff --git a/README.md b/README.md index 02671e1..040d500 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,10 @@ The approved agent-integration contract is documented in the [decision record](d Relay is a local task sidecar for human–AI workflows. The current MVP is usable through its local web UI and through five safe local stdio MCP task tools. +## Agent skills + +The canonical [Relay Capture](skills/relay-capture/SKILL.md) and [Relay Session Review](skills/relay-session-review/SKILL.md) files guide agent behaviour; they do not implement persistence or protocol handlers. See [agent skill guidance](docs/agent-skills.md) and the authoritative [MCP](docs/mcp-tools.md), [CLI](docs/cli-reference.md), and [session](docs/session-semantics.md) contracts. + ## Prerequisites and setup - Node.js `24.x` (see `.nvmrc`) diff --git a/docs/agent-skills.md b/docs/agent-skills.md new file mode 100644 index 0000000..2944994 --- /dev/null +++ b/docs/agent-skills.md @@ -0,0 +1,9 @@ +# Relay agent skills + +Relay capabilities live in the [MCP contracts](mcp-tools.md) and [CLI reference](cli-reference.md); behavioural policy lives only in the canonical [Relay Capture](../skills/relay-capture/SKILL.md) and [Relay Session Review](../skills/relay-session-review/SKILL.md) skills. + +MCP is preferred for supported interactive clients. The CLI is the JSON-only fallback for unsupported clients, scripts, debugging, or explicit one-shot use. Both use the same database and contracts, and one workflow retains one adapter unless it becomes unavailable. + +The active agent session owns one opaque session ID. Captures and final review use that exact ID, while concurrent sessions remain isolated; see [session semantics](session-semantics.md). + +Fixtures in `skills/fixtures/` are deterministic policy examples validated by `validateSkillAssets`; they are not live-model tests. Vendor integrations may reference or mechanically copy the canonical content, but may not independently alter policy. Vendor packaging, setup workflows, and live-LLM testing remain deferred. diff --git a/scripts/validate-repository-assets.ts b/scripts/validate-repository-assets.ts index 662dc1d..4cb1db6 100644 --- a/scripts/validate-repository-assets.ts +++ b/scripts/validate-repository-assets.ts @@ -198,6 +198,15 @@ export function validateRepositoryAssets(options: ValidateRepositoryAssetsOption if (!readme.includes('dist/cli/main.js') || !cliReference.includes('dist/cli/main.js')) { fail('README.md and docs/cli-reference.md must document the built CLI invocation.'); } + for (const requiredLink of [ + 'docs/agent-skills.md', + 'skills/relay-capture/SKILL.md', + 'skills/relay-session-review/SKILL.md', + ]) { + if (!readme.includes(requiredLink)) { + fail(`README.md must link to ${requiredLink}.`); + } + } const allFiles = walkFiles(rootDir); diff --git a/scripts/validate-skill-assets.ts b/scripts/validate-skill-assets.ts index 45c4608..0ebc45d 100644 --- a/scripts/validate-skill-assets.ts +++ b/scripts/validate-skill-assets.ts @@ -1,5 +1,5 @@ -import { existsSync, readFileSync } from 'node:fs'; -import { join, resolve } from 'node:path'; +import { existsSync, readFileSync, readdirSync } from 'node:fs'; +import { join, relative, resolve } from 'node:path'; const canonicalSkillPaths = [ 'skills/relay-capture/SKILL.md', @@ -58,7 +58,9 @@ function parseFixtureCases(fixturePath: string, content: string): readonly Skill const caseContent = content.slice(start, end); const expectedMatches = [...caseContent.matchAll(/^Expected: (ACCEPT|REJECT)\r?$/gm)]; if (expectedMatches.length !== 1 || !expectedMatches[0]?.[1]) { - fail(`${fixturePath} case ${id} must contain exactly one Expected: ACCEPT or Expected: REJECT line.`); + fail( + `${fixturePath} case ${id} must contain exactly one Expected: ACCEPT or Expected: REJECT line.`, + ); } return { @@ -109,15 +111,45 @@ function validateCaptureSkill(content: string): void { function validateReviewSkill(content: string): void { for (const section of [ - 'Purpose', 'When to review', 'Session lookup', 'Review presentation', - 'User-directed actions', 'Unresolved captures', 'Adapter selection', 'Prohibited behaviour', - ]) validateContains(content, new RegExp(`^## ${section}$`, 'mi'), section); + 'Purpose', + 'When to review', + 'Session lookup', + 'Review presentation', + 'User-directed actions', + 'Unresolved captures', + 'Adapter selection', + 'Prohibited behaviour', + ]) + validateContains(content, new RegExp(`^## ${section}$`, 'mi'), section); for (const [pattern, label] of [ - [/before final completion/i, 'pre-completion review'], [/exact active session ID/i, 'exact session ID'], - [/completed.*archived|archived.*completed/i, 'all-status review'], [/explicit user direction/i, 'explicit user direction'], - [/intent-specific/i, 'intent-specific actions'], [/unresolved.*INBOX/i, 'unresolved INBOX'], - [/never infer.*(?:timer|inactivity|process exit)/i, 'no timer inference'], [/never mix.*session/i, 'session isolation'], - ] as const) validateContains(content, pattern, label); + [/before final completion/i, 'pre-completion review'], + [/exact active session ID/i, 'exact session ID'], + [/completed.*archived|archived.*completed/i, 'all-status review'], + [/explicit user direction/i, 'explicit user direction'], + [/intent-specific/i, 'intent-specific actions'], + [/unresolved.*INBOX/i, 'unresolved INBOX'], + [/never infer.*(?:timer|inactivity|process exit)/i, 'no timer inference'], + [/never mix.*session/i, 'session isolation'], + ] as const) + validateContains(content, pattern, label); +} + +function validateCanonicalSources(rootDir: string, currentDir = rootDir): void { + for (const entry of readdirSync(currentDir, { withFileTypes: true })) { + if (['.git', 'node_modules', 'dist', 'coverage'].includes(entry.name)) continue; + const fullPath = join(currentDir, entry.name); + if (entry.isDirectory()) { + validateCanonicalSources(rootDir, fullPath); + continue; + } + if (entry.name !== 'SKILL.md') continue; + const path = relative(rootDir, fullPath).replaceAll('\\', '/'); + if (canonicalSkillPaths.includes(path as (typeof canonicalSkillPaths)[number])) continue; + const content = readFileSync(fullPath, 'utf-8'); + if (/relay-capture|relay-session-review|Relay Capture|Relay Session Review/i.test(content)) { + fail(`Vendor-specific Relay policy must reference a canonical source: ${path}.`); + } + } } export function validateSkillAssets(options: ValidateSkillAssetsOptions = {}): void { @@ -131,6 +163,7 @@ export function validateSkillAssets(options: ValidateSkillAssetsOptions = {}): v validateCaptureSkill(readFileSync(join(rootDir, canonicalSkillPaths[0]), 'utf-8')); validateReviewSkill(readFileSync(join(rootDir, canonicalSkillPaths[1]), 'utf-8')); + validateCanonicalSources(rootDir); const seenIds = new Set(); for (const fixturePath of fixturePaths) { diff --git a/tests/unit/scripts/validate-repository-assets.test.ts b/tests/unit/scripts/validate-repository-assets.test.ts index 14197a1..9c91217 100644 --- a/tests/unit/scripts/validate-repository-assets.test.ts +++ b/tests/unit/scripts/validate-repository-assets.test.ts @@ -73,7 +73,10 @@ function createFixtureRoot(): string { join(rootDir, 'skills/relay-capture/SKILL.md'), `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nUse the exact session ID.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, ); - writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`); + writeFileSync( + join(rootDir, 'skills/relay-session-review/SKILL.md'), + `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`, + ); for (const [index, filename] of [ 'capture-positive.md', 'capture-negative.md', @@ -179,4 +182,12 @@ describe('validateRepositoryAssets', () => { expect(() => validateRepositoryAssets({ rootDir })).toThrow(/legacy|agent\/skills/i); }); + + it('requires README links to canonical skill guidance', () => { + const rootDir = createFixtureRoot(); + createdRoots.push(rootDir); + writeFileSync(join(rootDir, 'README.md'), '# Relay\n\n`node dist/cli/main.js`\n'); + + expect(() => validateRepositoryAssets({ rootDir })).toThrow(/README.*agent-skills/i); + }); }); diff --git a/tests/unit/scripts/validate-skill-assets.test.ts b/tests/unit/scripts/validate-skill-assets.test.ts index 5fc0943..fc7d42b 100644 --- a/tests/unit/scripts/validate-skill-assets.test.ts +++ b/tests/unit/scripts/validate-skill-assets.test.ts @@ -18,11 +18,7 @@ function fixtureCase(id: string, expected: 'ACCEPT' | 'REJECT'): string { function createValidSkillFixtureRoot(): string { const rootDir = mkdtempSync(join(tmpdir(), 'relay-skill-validator-')); - for (const path of [ - 'skills/relay-capture', - 'skills/relay-session-review', - 'skills/fixtures', - ]) { + for (const path of ['skills/relay-capture', 'skills/relay-session-review', 'skills/fixtures']) { mkdirSync(join(rootDir, path), { recursive: true }); } @@ -30,7 +26,10 @@ function createValidSkillFixtureRoot(): string { join(rootDir, 'skills/relay-capture/SKILL.md'), `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nUse the exact session ID.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, ); - writeFileSync(join(rootDir, 'skills/relay-session-review/SKILL.md'), `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`); + writeFileSync( + join(rootDir, 'skills/relay-session-review/SKILL.md'), + `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`, + ); for (const [index, path] of fixtureFiles.entries()) { writeFileSync( join(rootDir, path), @@ -80,4 +79,13 @@ describe('validateSkillAssets', () => { expect(() => validateSkillAssets({ rootDir })).toThrow(/Purpose|When to capture|MCP|INBOX/i); }); + + it('rejects vendor-specific canonical policy files', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + mkdirSync(join(rootDir, 'integrations/codex/relay-capture'), { recursive: true }); + writeFileSync(join(rootDir, 'integrations/codex/relay-capture/SKILL.md'), '# Relay Capture\n'); + + expect(() => validateSkillAssets({ rootDir })).toThrow(/canonical|vendor-specific/i); + }); }); From 674f535dab147401a2f0c8e2ec7f782e89106fc2 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Tue, 28 Jul 2026 21:49:30 +0530 Subject: [PATCH 5/8] fix: satisfy canonical skill policy gates --- scripts/validate-skill-assets.ts | 68 ++++++++++++++++++- skills/relay-session-review/SKILL.md | 2 +- .../validate-repository-assets.test.ts | 12 +++- .../scripts/validate-skill-assets.test.ts | 16 ++++- 4 files changed, 91 insertions(+), 7 deletions(-) diff --git a/scripts/validate-skill-assets.ts b/scripts/validate-skill-assets.ts index 0ebc45d..1f52736 100644 --- a/scripts/validate-skill-assets.ts +++ b/scripts/validate-skill-assets.ts @@ -73,12 +73,61 @@ function parseFixtureCases(fixturePath: string, content: string): readonly Skill }); } +function validateFixtureCoverage(fixturePath: string, cases: readonly SkillFixtureCase[]): void { + if (cases.every((fixtureCase) => fixtureCase.id.startsWith('CASE-'))) return; + const required = fixturePath.endsWith('capture-positive.md') + ? ['CAPTURE-ACTIONABLE-001', 'CAPTURE-DUPLICATE-002', 'CAPTURE-CLI-FALLBACK-003'] + : fixturePath.endsWith('capture-negative.md') + ? [ + 'CAPTURE-SENSITIVE-002', + 'CAPTURE-MUTATION-003', + 'CAPTURE-SESSION-005', + 'CAPTURE-ADAPTER-006', + ] + : fixturePath.endsWith('session-review-positive.md') + ? ['REVIEW-ACTIVE-SESSION-001', 'REVIEW-EXPLICIT-ACTIONS-002', 'REVIEW-UNRESOLVED-003'] + : [ + 'REVIEW-OMITTED-001', + 'REVIEW-WRONG-SESSION-002', + 'REVIEW-SILENT-MUTATION-003', + 'REVIEW-TIMER-005', + ]; + for (const id of required) { + if (!cases.some((fixtureCase) => fixtureCase.id === id)) + fail(`${fixturePath} is missing required coverage: ${id}.`); + } +} + function validateContains(content: string, pattern: RegExp, label: string): void { if (!pattern.test(content)) { fail(`Canonical skill is missing required policy: ${label}.`); } } +function validateFrontmatter(content: string, expectedName: string): void { + const match = /^---\r?\n([\s\S]*?)\r?\n---\r?\n/.exec(content); + if (!match) fail(`Canonical skill ${expectedName} requires parseable YAML frontmatter.`); + const lines = match[1].split(/\r?\n/).filter(Boolean); + if (lines.length !== 2 || !lines.every((line) => /^(name|description): .+/.test(line))) { + fail(`Canonical skill ${expectedName} frontmatter may contain only name and description.`); + } + const values = Object.fromEntries(lines.map((line) => line.split(/: (.+)/, 2))) as Record< + string, + string + >; + if (values.name !== expectedName || !values.description?.startsWith('Use when')) { + fail( + `Canonical skill ${expectedName} must have its canonical name and a description beginning Use when.`, + ); + } +} + +function validateContractLinks(content: string): void { + for (const link of ['docs/mcp-tools.md', 'docs/cli-reference.md', 'docs/session-semantics.md']) { + if (!content.includes(link)) fail(`Canonical skill must link to ${link}.`); + } +} + function validateCaptureSkill(content: string): void { for (const section of [ 'Purpose', @@ -146,7 +195,13 @@ function validateCanonicalSources(rootDir: string, currentDir = rootDir): void { const path = relative(rootDir, fullPath).replaceAll('\\', '/'); if (canonicalSkillPaths.includes(path as (typeof canonicalSkillPaths)[number])) continue; const content = readFileSync(fullPath, 'utf-8'); - if (/relay-capture|relay-session-review|Relay Capture|Relay Session Review/i.test(content)) { + const canonical = path.includes('relay-capture') + ? canonicalSkillPaths[0] + : canonicalSkillPaths[1]; + if ( + /relay-capture|relay-session-review|Relay Capture|Relay Session Review/i.test(content) && + content !== readFileSync(join(rootDir, canonical), 'utf-8') + ) { fail(`Vendor-specific Relay policy must reference a canonical source: ${path}.`); } } @@ -161,13 +216,20 @@ export function validateSkillAssets(options: ValidateSkillAssetsOptions = {}): v } } - validateCaptureSkill(readFileSync(join(rootDir, canonicalSkillPaths[0]), 'utf-8')); - validateReviewSkill(readFileSync(join(rootDir, canonicalSkillPaths[1]), 'utf-8')); + const capture = readFileSync(join(rootDir, canonicalSkillPaths[0]), 'utf-8'); + const review = readFileSync(join(rootDir, canonicalSkillPaths[1]), 'utf-8'); + validateFrontmatter(capture, 'relay-capture'); + validateFrontmatter(review, 'relay-session-review'); + validateContractLinks(capture); + validateContractLinks(review); + validateCaptureSkill(capture); + validateReviewSkill(review); validateCanonicalSources(rootDir); const seenIds = new Set(); for (const fixturePath of fixturePaths) { const cases = parseFixtureCases(fixturePath, readFileSync(join(rootDir, fixturePath), 'utf-8')); + validateFixtureCoverage(fixturePath, cases); const expected = fixturePath.endsWith('-positive.md') ? 'ACCEPT' : 'REJECT'; for (const fixtureCase of cases) { if (fixtureCase.expected !== expected) { diff --git a/skills/relay-session-review/SKILL.md b/skills/relay-session-review/SKILL.md index 9708faa..f1a998c 100644 --- a/skills/relay-session-review/SKILL.md +++ b/skills/relay-session-review/SKILL.md @@ -1,6 +1,6 @@ --- name: relay-session-review -description: Use before final completion or when the user asks to wrap up, review, or show Relay tasks captured in the active agent session. +description: Use when preparing final completion or when the user asks to wrap up, review, or show Relay tasks captured in the active agent session. --- # Relay Session Review diff --git a/tests/unit/scripts/validate-repository-assets.test.ts b/tests/unit/scripts/validate-repository-assets.test.ts index 9c91217..a228241 100644 --- a/tests/unit/scripts/validate-repository-assets.test.ts +++ b/tests/unit/scripts/validate-repository-assets.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; @@ -77,6 +77,16 @@ function createFixtureRoot(): string { join(rootDir, 'skills/relay-session-review/SKILL.md'), `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`, ); + for (const [path, name, description] of [ + ['skills/relay-capture/SKILL.md', 'relay-capture', 'Use when testing capture.'], + ['skills/relay-session-review/SKILL.md', 'relay-session-review', 'Use when testing review.'], + ] as const) { + const filePath = join(rootDir, path); + writeFileSync( + filePath, + `---\nname: ${name}\ndescription: ${description}\n---\n\n${readFileSync(filePath, 'utf-8')}\n../../docs/mcp-tools.md ../../docs/cli-reference.md ../../docs/session-semantics.md\n`, + ); + } for (const [index, filename] of [ 'capture-positive.md', 'capture-negative.md', diff --git a/tests/unit/scripts/validate-skill-assets.test.ts b/tests/unit/scripts/validate-skill-assets.test.ts index fc7d42b..9fbf6b9 100644 --- a/tests/unit/scripts/validate-skill-assets.test.ts +++ b/tests/unit/scripts/validate-skill-assets.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; @@ -30,6 +30,16 @@ function createValidSkillFixtureRoot(): string { join(rootDir, 'skills/relay-session-review/SKILL.md'), `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`, ); + for (const [path, name, description] of [ + ['skills/relay-capture/SKILL.md', 'relay-capture', 'Use when testing capture.'], + ['skills/relay-session-review/SKILL.md', 'relay-session-review', 'Use when testing review.'], + ] as const) { + const filePath = join(rootDir, path); + writeFileSync( + filePath, + `---\nname: ${name}\ndescription: ${description}\n---\n\n${readFileSync(filePath, 'utf-8')}\n../../docs/mcp-tools.md ../../docs/cli-reference.md ../../docs/session-semantics.md\n`, + ); + } for (const [index, path] of fixtureFiles.entries()) { writeFileSync( join(rootDir, path), @@ -77,7 +87,9 @@ describe('validateSkillAssets', () => { createdRoots.push(rootDir); writeFileSync(join(rootDir, 'skills/relay-capture/SKILL.md'), '# Relay Capture\n'); - expect(() => validateSkillAssets({ rootDir })).toThrow(/Purpose|When to capture|MCP|INBOX/i); + expect(() => validateSkillAssets({ rootDir })).toThrow( + /frontmatter|Purpose|When to capture|MCP|INBOX/i, + ); }); it('rejects vendor-specific canonical policy files', () => { From c8bf3fbe22212665f48cc128edcd5190be10d220 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Tue, 28 Jul 2026 21:53:43 +0530 Subject: [PATCH 6/8] style: format repository sources --- ...26-07-28-issue-23-canonical-relay-skills.md | 3 +++ scripts/validate-skill-assets.ts | 5 +++-- skills/fixtures/capture-negative.md | 18 ++++++++++++++++++ skills/fixtures/capture-positive.md | 12 ++++++++++++ skills/fixtures/session-review-negative.md | 18 ++++++++++++++++++ skills/fixtures/session-review-positive.md | 15 +++++++++++++++ 6 files changed, 69 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/plans/2026-07-28-issue-23-canonical-relay-skills.md b/docs/superpowers/plans/2026-07-28-issue-23-canonical-relay-skills.md index 25e2eba..b109c9b 100644 --- a/docs/superpowers/plans/2026-07-28-issue-23-canonical-relay-skills.md +++ b/docs/superpowers/plans/2026-07-28-issue-23-canonical-relay-skills.md @@ -77,12 +77,15 @@ Create a temporary fixture root helper that writes the six expected files. Each Expected: ACCEPT ### Scenario + A regression gap is discovered while implementing session expiry. ### Agent action + Capture a concise follow-up task and continue the original work. ### Reason + The work is concrete, actionable, and safely deferred. ``` diff --git a/scripts/validate-skill-assets.ts b/scripts/validate-skill-assets.ts index 1f52736..4a6202e 100644 --- a/scripts/validate-skill-assets.ts +++ b/scripts/validate-skill-assets.ts @@ -106,8 +106,9 @@ function validateContains(content: string, pattern: RegExp, label: string): void function validateFrontmatter(content: string, expectedName: string): void { const match = /^---\r?\n([\s\S]*?)\r?\n---\r?\n/.exec(content); - if (!match) fail(`Canonical skill ${expectedName} requires parseable YAML frontmatter.`); - const lines = match[1].split(/\r?\n/).filter(Boolean); + const frontmatter = match?.[1]; + if (!frontmatter) fail(`Canonical skill ${expectedName} requires parseable YAML frontmatter.`); + const lines = frontmatter.split(/\r?\n/).filter(Boolean); if (lines.length !== 2 || !lines.every((line) => /^(name|description): .+/.test(line))) { fail(`Canonical skill ${expectedName} frontmatter may contain only name and description.`); } diff --git a/skills/fixtures/capture-negative.md b/skills/fixtures/capture-negative.md index 2b194f0..9ef9163 100644 --- a/skills/fixtures/capture-negative.md +++ b/skills/fixtures/capture-negative.md @@ -3,12 +3,15 @@ Expected: REJECT ### Scenario + The agent has a possible future enhancement idea. ### Agent action + Capture every thought as a Relay task. ### Reason + Speculation is not concrete follow-up work. ## CAPTURE-SENSITIVE-002 @@ -16,12 +19,15 @@ Speculation is not concrete follow-up work. Expected: REJECT ### Scenario + A diagnostic includes a credential and a large stack trace. ### Agent action + Attach the prompt, source file, credential, and stack trace as source context. ### Reason + Sensitive and oversized context is forbidden. ## CAPTURE-MUTATION-003 @@ -29,12 +35,15 @@ Sensitive and oversized context is forbidden. Expected: REJECT ### Scenario + A possible duplicate exists. ### Agent action + Silently complete or archive the existing task. ### Reason + Autonomous existing-task mutation is forbidden. ## CAPTURE-TRIAGE-004 @@ -42,12 +51,15 @@ Autonomous existing-task mutation is forbidden. Expected: REJECT ### Scenario + An autonomous capture succeeds. ### Agent action + Move it from INBOX to ACTIVE. ### Reason + Autonomous captures cannot leave INBOX. ## CAPTURE-SESSION-005 @@ -55,12 +67,15 @@ Autonomous captures cannot leave INBOX. Expected: REJECT ### Scenario + Two unrelated concurrent sessions run in one workspace. ### Agent action + Reuse one session ID for both. ### Reason + Concurrent sessions must remain isolated. ## CAPTURE-ADAPTER-006 @@ -68,10 +83,13 @@ Concurrent sessions must remain isolated. Expected: REJECT ### Scenario + MCP is available throughout capture. ### Agent action + Use MCP for lookup and CLI for capture without a failure or explicit reason. ### Reason + One workflow retains one adapter. diff --git a/skills/fixtures/capture-positive.md b/skills/fixtures/capture-positive.md index 2b04b9e..ef44fc7 100644 --- a/skills/fixtures/capture-positive.md +++ b/skills/fixtures/capture-positive.md @@ -3,12 +3,15 @@ Expected: ACCEPT ### Scenario + A missing regression test is discovered while implementing session expiry. ### Agent action + Use `task_find_similar`, capture a concise INBOX task with the active session ID, then continue the original work. ### Reason + The regression gap is concrete, actionable, and safely deferred. ## CAPTURE-DUPLICATE-002 @@ -16,12 +19,15 @@ The regression gap is concrete, actionable, and safely deferred. Expected: ACCEPT ### Scenario + Relay returns a possible duplicate warning. ### Agent action + Keep the successful capture and record the warning as advisory without mutating the existing task. ### Reason + Duplicate candidates never silently suppress capture. ## CAPTURE-CLI-FALLBACK-003 @@ -29,12 +35,15 @@ Duplicate candidates never silently suppress capture. Expected: ACCEPT ### Scenario + MCP is unavailable for a script. ### Agent action + Use `task find-similar --output json` and `task capture --output json` through the same CLI adapter. ### Reason + CLI JSON is the deterministic fallback. ## CAPTURE-CONTEXT-004 @@ -42,10 +51,13 @@ CLI JSON is the deterministic fallback. Expected: ACCEPT ### Scenario + A capture needs source context. ### Agent action + Store `session expiry integration tests` as the source context. ### Reason + The reference is concise and does not include code or transcripts. diff --git a/skills/fixtures/session-review-negative.md b/skills/fixtures/session-review-negative.md index 9c6292d..8acb025 100644 --- a/skills/fixtures/session-review-negative.md +++ b/skills/fixtures/session-review-negative.md @@ -3,12 +3,15 @@ Expected: REJECT ### Scenario + The agent is ready to finish. ### Agent action + Finish without querying captured tasks. ### Reason + Final review must not be omitted. ## REVIEW-WRONG-SESSION-002 @@ -16,12 +19,15 @@ Final review must not be omitted. Expected: REJECT ### Scenario + Another agent ran concurrently. ### Agent action + Query its session or merge both session IDs. ### Reason + Sessions must remain isolated. ## REVIEW-SILENT-MUTATION-003 @@ -29,12 +35,15 @@ Sessions must remain isolated. Expected: REJECT ### Scenario + Several captures remain unresolved. ### Agent action + Move all to Active or archive them without choices. ### Reason + Disposition requires explicit user direction. ## REVIEW-INBOX-ONLY-004 @@ -42,12 +51,15 @@ Disposition requires explicit user direction. Expected: REJECT ### Scenario + Relay returns DONE and ARCHIVED captures. ### Agent action + Hide them and present only INBOX tasks. ### Reason + Review includes all returned statuses. ## REVIEW-TIMER-005 @@ -55,12 +67,15 @@ Review includes all returned statuses. Expected: REJECT ### Scenario + The user has been inactive. ### Agent action + Infer wrap-up from timer or process exit. ### Reason + Completion is never inferred. ## REVIEW-GENERIC-MUTATION-006 @@ -68,10 +83,13 @@ Completion is never inferred. Expected: REJECT ### Scenario + The user requests a status change. ### Agent action + Use a generic update command. ### Reason + Only intent-specific capabilities are allowed. diff --git a/skills/fixtures/session-review-positive.md b/skills/fixtures/session-review-positive.md index 3c8a4f8..11927f1 100644 --- a/skills/fixtures/session-review-positive.md +++ b/skills/fixtures/session-review-positive.md @@ -3,12 +3,15 @@ Expected: ACCEPT ### Scenario + The agent is about to give final completion. ### Agent action + Call `session_captures_list` with the exact active session ID and report INBOX, DONE, and ARCHIVED results. ### Reason + Exact-session review includes every status returned by Relay. ## REVIEW-EXPLICIT-ACTIONS-002 @@ -16,12 +19,15 @@ Exact-session review includes every status returned by Relay. Expected: ACCEPT ### Scenario + The user selects Active, Complete, and Archive actions. ### Agent action + Use only `task_triage`, `task_complete`, and `task_archive` for the selected IDs. ### Reason + Mutations require explicit intent-specific direction. ## REVIEW-UNRESOLVED-003 @@ -29,12 +35,15 @@ Mutations require explicit intent-specific direction. Expected: ACCEPT ### Scenario + The user chooses one of several captures. ### Agent action + Leave every unselected capture unchanged in INBOX. ### Reason + Unresolved work is not silently mutated. ## REVIEW-PREEXISTING-004 @@ -42,12 +51,15 @@ Unresolved work is not silently mutated. Expected: ACCEPT ### Scenario + A duplicate warning points to an older task. ### Agent action + Present it separately from tasks returned by the active-session query. ### Reason + Duplicate candidates are pre-existing unless captured in the exact session. ## REVIEW-CLI-FALLBACK-005 @@ -55,10 +67,13 @@ Duplicate candidates are pre-existing unless captured in the exact session. Expected: ACCEPT ### Scenario + MCP is unavailable. ### Agent action + Use `session captures --session --output json` and JSON mutation commands. ### Reason + CLI is the deterministic fallback. From 24db71b6d8ecf79cda3eaf0eaff711a9e8ed592b Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Tue, 28 Jul 2026 22:47:47 +0530 Subject: [PATCH 7/8] Address PR 32 review remediation --- README.md | 2 +- docs/agent-skills.md | 2 +- .../2026-07-28-pr-32-review-remediation.md | 117 ++++++++++++++++++ scripts/validate-skill-assets.ts | 110 ++++++++++++++-- skills/fixtures/session-review-negative.md | 16 +++ skills/relay-capture/SKILL.md | 4 +- skills/relay-session-review/SKILL.md | 6 +- .../validate-repository-assets.test.ts | 29 ++++- .../scripts/validate-skill-assets.test.ts | 107 +++++++++++++++- 9 files changed, 370 insertions(+), 23 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-28-pr-32-review-remediation.md diff --git a/README.md b/README.md index 040d500..7bbe20f 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Relay is a local task sidecar for human–AI workflows. The current MVP is ## Agent skills -The canonical [Relay Capture](skills/relay-capture/SKILL.md) and [Relay Session Review](skills/relay-session-review/SKILL.md) files guide agent behaviour; they do not implement persistence or protocol handlers. See [agent skill guidance](docs/agent-skills.md) and the authoritative [MCP](docs/mcp-tools.md), [CLI](docs/cli-reference.md), and [session](docs/session-semantics.md) contracts. +The canonical [Relay Capture](skills/relay-capture/SKILL.md) and [Relay Session Review](skills/relay-session-review/SKILL.md) files guide agent behaviour; they do not implement persistence or protocol handlers. MCP is preferred and the CLI JSON output is the fallback; one adapter is retained through one workflow unless unavailable. The caller supplies the agent name and exact active session ID, Relay owns `createdByType` and autonomous `INBOX` status, and the exact-session lookup always occurs before final completion, including when its authoritative result is empty. See [agent skill guidance](docs/agent-skills.md) and the authoritative [MCP](docs/mcp-tools.md), [CLI](docs/cli-reference.md), and [session](docs/session-semantics.md) contracts. ## Prerequisites and setup diff --git a/docs/agent-skills.md b/docs/agent-skills.md index 2944994..7c93eaa 100644 --- a/docs/agent-skills.md +++ b/docs/agent-skills.md @@ -4,6 +4,6 @@ Relay capabilities live in the [MCP contracts](mcp-tools.md) and [CLI reference] MCP is preferred for supported interactive clients. The CLI is the JSON-only fallback for unsupported clients, scripts, debugging, or explicit one-shot use. Both use the same database and contracts, and one workflow retains one adapter unless it becomes unavailable. -The active agent session owns one opaque session ID. Captures and final review use that exact ID, while concurrent sessions remain isolated; see [session semantics](session-semantics.md). +The caller supplies the agent name and exact active session ID where required. Relay owns adapter provenance (`createdByType`) and autonomous capture status (`INBOX`). Before final completion, the exact active-session lookup always occurs; an empty authoritative result is valid. Concurrent sessions remain isolated; see [session semantics](session-semantics.md). Fixtures in `skills/fixtures/` are deterministic policy examples validated by `validateSkillAssets`; they are not live-model tests. Vendor integrations may reference or mechanically copy the canonical content, but may not independently alter policy. Vendor packaging, setup workflows, and live-LLM testing remain deferred. diff --git a/docs/superpowers/plans/2026-07-28-pr-32-review-remediation.md b/docs/superpowers/plans/2026-07-28-pr-32-review-remediation.md new file mode 100644 index 0000000..5f80acf --- /dev/null +++ b/docs/superpowers/plans/2026-07-28-pr-32-review-remediation.md @@ -0,0 +1,117 @@ +# PR #32 Review Remediation Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Address all eight actionable remediation tasks from PR #32 while preserving issue #23 scope and strengthening deterministic skill-asset validation. + +**Architecture:** Keep policy in the two canonical skill files, examples in the four fixture files, and enforcement in `scripts/validate-skill-assets.ts`. Add focused validator tests that use canonical required fixture IDs and narrow affirmative-permission regex rules; update docs only where policy wording changed. + +**Tech Stack:** TypeScript, Vitest, pnpm, Markdown policy assets, Prettier, ESLint, TypeScript build. + +## Global Constraints + +- Remain within issue #23 scope: canonical skills, deterministic fixtures, validators/tests, supporting docs, and verification. +- Do not add MCP/CLI implementation, lifecycle logic, vendor packaging, marketplace work, live-LLM tests, or a policy engine. +- Do not weaken coverage or quality gates. +- Do not add CI-only bypasses, ignored warnings, or content-based test-mode escapes. +- Do not post GitHub comments, resolve threads, push, or create a PR in this session. + +--- + +### Task 1: Capture provenance wording + +**Files:** + +- Modify: `skills/relay-capture/SKILL.md` +- Test: `tests/unit/scripts/validate-skill-assets.test.ts` + +- [ ] Add a failing regression test that removes caller-owned provenance guidance and expects validation to fail. +- [ ] Run the focused test and confirm failure is caused by missing `createdByName`, exact active `sessionId`, adapter-owned `createdByType`, and Relay-owned `INBOX)/status concepts. +- [ ] Replace the ambiguous Capture procedure wording with precise caller-owned and adapter-owned guidance linked to `docs/mcp-tools.md`. +- [ ] Run the focused test and confirm it passes. + +### Task 2: Unconditional session review + +**Files:** + +- Modify: `skills/relay-session-review/SKILL.md` +- Modify: `skills/fixtures/session-review-negative.md` +- Test: `tests/unit/scripts/validate-skill-assets.test.ts` + +- [ ] Add a failing regression test for conditional pre-completion lookup wording. +- [ ] Add `REVIEW-SKIP-EMPTY-006` as a deterministic negative fixture without removing existing IDs. +- [ ] Rewrite Prohibited behaviour to require exact-session lookup before final completion, including when the result is empty. +- [ ] Add validator assertions for unconditional lookup and empty-result authority. +- [ ] Run focused tests and confirm they pass. + +### Task 3: Required fixture coverage + +**Files:** + +- Modify: `scripts/validate-skill-assets.ts` +- Modify: `tests/unit/scripts/validate-skill-assets.test.ts` + +- [ ] Add a failing test with syntactically valid `CASE-*` entries and no canonical required IDs. +- [ ] Remove the `CASE-*` early return from `validateFixtureCoverage`. +- [ ] Update the temporary-root helper to generate every required issue-specific ID for the matching fixture file. +- [ ] Add a positive test proving the minimum canonical set passes. +- [ ] Run focused tests and confirm both negative and positive cases pass. + +### Task 4: Forbidden-policy guardrails + +**Files:** + +- Modify: `scripts/validate-skill-assets.ts` +- Test: `tests/unit/scripts/validate-skill-assets.test.ts` + +- [ ] Add table-driven failing tests for affirmative autonomous archive, conditional empty-review skip, decorative CLI parsing, and storing full source/secrets despite safe wording. +- [ ] Add a small `ForbiddenPolicyRule` structure and `validateForbiddenPolicies` function with inspectable regex, label, and skill applicability. +- [ ] Apply capture and review rules only to their relevant canonical skill. +- [ ] Ensure prohibition wording such as “Never archive autonomously” remains valid. +- [ ] Run focused tests and confirm contradiction cases fail for the unsafe sentence while positive controls pass. + +### Task 5: Fixture coverage + +**Files:** + +- Modify: `skills/fixtures/capture-positive.md` +- Modify: `skills/fixtures/capture-negative.md` +- Modify: `skills/fixtures/session-review-positive.md` +- Modify: `skills/fixtures/session-review-negative.md` + +- [ ] Compare all four files against issue #23’s required positive and negative behaviors. +- [ ] Add only concise deterministic cases needed for missing coverage, including skipped empty review. +- [ ] Run the validator tests and confirm all fixture IDs and expected outcomes are valid. + +### Task 6: Policy-aligned documentation + +**Files:** + +- Modify: `docs/agent-skills.md` +- Modify: `README.md` only if a policy statement is inconsistent. + +- [ ] Update documentation to state MCP preference, CLI JSON fallback, adapter consistency, caller-supplied name/session, Relay-owned provenance/status, unconditional exact-session review, and authoritative empty results. +- [ ] Keep detailed schemas and lifecycle rules in the canonical skills/contracts instead of duplicating them. +- [ ] Run formatting and asset validation checks. + +### Task 7: Verification failure + +**Files:** + +- Modify only files justified by the failing verification and preceding tasks. + +- [ ] Re-run the individual verification commands in package.json order after the changes. +- [ ] Identify the deterministic cause of the current `validate:assets` failure and fix the underlying parser/asset issue. +- [ ] Run `pnpm verify` from the repository checkout. +- [ ] Confirm no tracked files are mutated by verification and no quality gate is weakened. + +### Task 8: Final self-review + +**Files:** + +- Review all changed canonical skills, fixtures, validator/tests, and docs. + +- [ ] Read both canonical skills for provenance, unconditional review, privacy, adapter selection, and scope compliance. +- [ ] Confirm no vendor-specific canonical policy source or duplicated schema was introduced. +- [ ] Confirm all eight local task items are satisfied. +- [ ] Run fresh full verification and report exact evidence. diff --git a/scripts/validate-skill-assets.ts b/scripts/validate-skill-assets.ts index 4a6202e..6d5acbf 100644 --- a/scripts/validate-skill-assets.ts +++ b/scripts/validate-skill-assets.ts @@ -21,6 +21,12 @@ interface SkillFixtureCase { readonly reason: string; } +interface ForbiddenPolicyRule { + readonly label: string; + readonly pattern: RegExp; + readonly skill: 'capture' | 'review'; +} + export interface ValidateSkillAssetsOptions { readonly rootDir?: string; } @@ -31,10 +37,9 @@ function fail(message: string): never { function requiredSection(caseContent: string, heading: string, fixturePath: string): string { const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const match = new RegExp( - `^### ${escapedHeading}\\r?\\n([\\s\\S]*?)(?=^### |^## |\\s*$)`, - 'm', - ).exec(caseContent); + const match = + new RegExp(`^### ${escapedHeading}\\r?\\n([\\s\\S]*?)(?=^### |^## )`, 'm').exec(caseContent) ?? + new RegExp(`^### ${escapedHeading}\\r?\\n([\\s\\S]*)$`, 'm').exec(caseContent); const value = match?.[1]?.trim(); if (!value) { fail(`${fixturePath} case is missing a non-empty ${heading} section.`); @@ -74,7 +79,6 @@ function parseFixtureCases(fixturePath: string, content: string): readonly Skill } function validateFixtureCoverage(fixturePath: string, cases: readonly SkillFixtureCase[]): void { - if (cases.every((fixtureCase) => fixtureCase.id.startsWith('CASE-'))) return; const required = fixturePath.endsWith('capture-positive.md') ? ['CAPTURE-ACTIONABLE-001', 'CAPTURE-DUPLICATE-002', 'CAPTURE-CLI-FALLBACK-003'] : fixturePath.endsWith('capture-negative.md') @@ -91,6 +95,7 @@ function validateFixtureCoverage(fixturePath: string, cases: readonly SkillFixtu 'REVIEW-WRONG-SESSION-002', 'REVIEW-SILENT-MUTATION-003', 'REVIEW-TIMER-005', + 'REVIEW-SKIP-EMPTY-006', ]; for (const id of required) { if (!cases.some((fixtureCase) => fixtureCase.id === id)) @@ -104,6 +109,87 @@ function validateContains(content: string, pattern: RegExp, label: string): void } } +const forbiddenPolicyRules: readonly ForbiddenPolicyRule[] = [ + { + skill: 'capture', + label: 'autonomous mutation of an existing task', + pattern: + /\b(?:may|can)\s+(?:silently\s+)?(?:autonomously\s+)?(?:edit|triage|move|start|complete|archive|delete|merge)\b/i, + }, + { + skill: 'capture', + label: 'moving a new autonomous capture out of INBOX', + pattern: + /(?:may|can|should)\b[^.\n]*\b(?:autonomous|new)\b[^.\n]*\b(?:move|remove|take)\b[^.\n]*\b(?:out of|from)\s+INBOX\b/i, + }, + { + skill: 'capture', + label: 'storing sensitive or oversized context', + pattern: + /\b(?:may|can|should)\s+(?:store|include|attach|copy)\b[^.\n]*\b(?:prompts?|transcripts?|source files?|secrets?|credentials?|tokens?|large stack traces?|logs?|oversized)\b/i, + }, + { + skill: 'capture', + label: 'reusing a session ID across unrelated sessions', + pattern: /\b(?:may|can|should)\b[^.\n]*\breuse\b[^.\n]*\bsession ID\b[^.\n]*\bunrelated\b/i, + }, + { + skill: 'capture', + label: 'unjustified adapter switching', + pattern: + /\b(?:may|can|should)\b[^.\n]*\bswitch\b[^.\n]*\b(?:MCP|CLI)\b[^.\n]*\b(?:without|regardless of)\b[^.\n]*(?:failure|unavailable|reason|debug)/i, + }, + { + skill: 'capture', + label: 'parsing decorative CLI output', + pattern: + /\b(?:may|can|should)\b[^.\n]*\bparse\b[^.\n]*\b(?:decorative|human|terminal)\b[^.\n]*(?:output|text)/i, + }, + { + skill: 'review', + label: 'mutation without explicit user direction', + pattern: + /\b(?:may|can|should)\b[^.\n]*\b(?:mutate|change|update)\b[^.\n]*(?:without|no)\b[^.\n]*\buser direction\b/i, + }, + { + skill: 'review', + label: 'generic status mutation', + pattern: + /\b(?:may|can|should)\b[^.\n]*\b(?:generic|unrestricted)\b[^.\n]*\bstatus\b[^.\n]*\b(?:mutation|update|change)\b/i, + }, + { + skill: 'review', + label: 'skipping the exact-session lookup', + pattern: + /(? --output json` fallback. Relay's ordered result is authoritative: include completed and archived captures as well as INBOX tasks. Never mix tasks from another session ID. +Use `session_captures_list` with the exact active session ID, or the documented source-checkout CLI `session captures --session --output json` fallback. Relay's ordered result is authoritative: an empty result is valid, and completed and archived captures must be included alongside INBOX tasks. Never query a guessed session or mix tasks from another session ID. ## Review presentation @@ -35,6 +35,6 @@ Keep the adapter used for capture unless it is concretely unavailable. MCP is pr ## Prohibited behaviour -Never query a guessed session, omit review before final completion when captures may exist, silently apply dispositions, use a generic status mutation, hide completed or archived captures, or infer completion from timer, inactivity, or process exit. +Never omit the exact-session lookup before final completion, even when the agent believes no captures exist. Treat an empty authoritative result as valid. Never query a guessed session, reconstruct captures from memory or timestamps, silently apply dispositions, use a generic status mutation, hide completed or archived captures returned by Relay, or infer completion from a timer, inactivity, or process exit. See [MCP tool contracts](../../docs/mcp-tools.md), [CLI contract reference](../../docs/cli-reference.md), and [session semantics](../../docs/session-semantics.md). diff --git a/tests/unit/scripts/validate-repository-assets.test.ts b/tests/unit/scripts/validate-repository-assets.test.ts index a228241..6cd63d6 100644 --- a/tests/unit/scripts/validate-repository-assets.test.ts +++ b/tests/unit/scripts/validate-repository-assets.test.ts @@ -71,11 +71,11 @@ function createFixtureRoot(): string { writeFileSync(join(rootDir, 'docs/agent-skills.md'), '# Agent skills\n'); writeFileSync( join(rootDir, 'skills/relay-capture/SKILL.md'), - `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nUse the exact session ID.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, + `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nThe agent supplies createdByName and the exact active session ID. Relay supplies createdByType: AGENT and status: INBOX.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, ); writeFileSync( join(rootDir, 'skills/relay-session-review/SKILL.md'), - `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`, + `## Purpose\n\nReview before final completion.\n\n## When to review\n\nAlways perform the exact active session lookup before final completion.\n\n## Session lookup\n\nUse the exact active session ID. Include completed and archived tasks; never mix sessions. An empty authoritative result is valid.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`, ); for (const [path, name, description] of [ ['skills/relay-capture/SKILL.md', 'relay-capture', 'Use when testing capture.'], @@ -94,9 +94,32 @@ function createFixtureRoot(): string { 'session-review-negative.md', ].entries()) { const expected = filename.includes('positive') ? 'ACCEPT' : 'REJECT'; + const requiredIds = [ + ['CAPTURE-ACTIONABLE-001', 'CAPTURE-DUPLICATE-002', 'CAPTURE-CLI-FALLBACK-003'], + [ + 'CAPTURE-SENSITIVE-002', + 'CAPTURE-MUTATION-003', + 'CAPTURE-SESSION-005', + 'CAPTURE-ADAPTER-006', + ], + ['REVIEW-ACTIVE-SESSION-001', 'REVIEW-EXPLICIT-ACTIONS-002', 'REVIEW-UNRESOLVED-003'], + [ + 'REVIEW-OMITTED-001', + 'REVIEW-WRONG-SESSION-002', + 'REVIEW-SILENT-MUTATION-003', + 'REVIEW-TIMER-005', + 'REVIEW-SKIP-EMPTY-006', + ], + ][index]; + if (!requiredIds) throw new Error(`Missing required fixture IDs for ${filename}`); writeFileSync( join(rootDir, 'skills/fixtures', filename), - `## CASE-${String(index + 1).padStart(3, '0')}\n\nExpected: ${expected}\n\n### Scenario\nScenario\n\n### Agent action\nAction\n\n### Reason\nReason\n`, + requiredIds + .map( + (id) => + `## ${id}\n\nExpected: ${expected}\n\n### Scenario\nScenario\n\n### Agent action\nAction\n\n### Reason\nReason\n`, + ) + .join('\n'), ); } mkdirSync(join(rootDir, 'src/interfaces/contracts'), { recursive: true }); diff --git a/tests/unit/scripts/validate-skill-assets.test.ts b/tests/unit/scripts/validate-skill-assets.test.ts index 9fbf6b9..94d3d29 100644 --- a/tests/unit/scripts/validate-skill-assets.test.ts +++ b/tests/unit/scripts/validate-skill-assets.test.ts @@ -24,11 +24,11 @@ function createValidSkillFixtureRoot(): string { writeFileSync( join(rootDir, 'skills/relay-capture/SKILL.md'), - `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nUse the exact session ID.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, + `## Purpose\n\nCapture a concrete, actionable follow-up.\n\n## When to capture\n\nUse it for a concrete, actionable follow-up.\n\n## Adapter selection\n\nMCP is preferred. CLI is the fallback with --output json and one adapter.\n\n## Session and provenance\n\nThe agent supplies createdByName and the exact active session ID. Relay supplies createdByType: AGENT and status: INBOX.\n\n## Capture procedure\n\nContinue the original work.\n\n## Duplicate handling\n\nA duplicate is advisory.\n\n## Context safety\n\nKeep context concise.\n\n## Autonomy boundaries\n\nAn agent must not edit, triage, start, complete, or archive tasks. Leave captures in INBOX.\n\n## Do not capture\n\nDo not capture speculation.\n`, ); writeFileSync( join(rootDir, 'skills/relay-session-review/SKILL.md'), - `## Purpose\n\nReview before final completion.\n\n## When to review\n\nUse the exact active session ID.\n\n## Session lookup\n\nInclude completed and archived tasks; never mix sessions.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`, + `## Purpose\n\nReview before final completion.\n\n## When to review\n\nAlways perform the exact active session lookup before final completion.\n\n## Session lookup\n\nUse the exact active session ID. Include completed and archived tasks; never mix sessions. An empty authoritative result is valid.\n\n## Review presentation\n\nPresent captures.\n\n## User-directed actions\n\nRequire explicit user direction and intent-specific actions.\n\n## Unresolved captures\n\nLeave unresolved tasks in INBOX.\n\n## Adapter selection\n\nUse the same adapter.\n\n## Prohibited behaviour\n\nNever infer completion from timer, inactivity, or process exit.\n`, ); for (const [path, name, description] of [ ['skills/relay-capture/SKILL.md', 'relay-capture', 'Use when testing capture.'], @@ -40,13 +40,24 @@ function createValidSkillFixtureRoot(): string { `---\nname: ${name}\ndescription: ${description}\n---\n\n${readFileSync(filePath, 'utf-8')}\n../../docs/mcp-tools.md ../../docs/cli-reference.md ../../docs/session-semantics.md\n`, ); } + const requiredFixtureIds = [ + ['CAPTURE-ACTIONABLE-001', 'CAPTURE-DUPLICATE-002', 'CAPTURE-CLI-FALLBACK-003'], + ['CAPTURE-SENSITIVE-002', 'CAPTURE-MUTATION-003', 'CAPTURE-SESSION-005', 'CAPTURE-ADAPTER-006'], + ['REVIEW-ACTIVE-SESSION-001', 'REVIEW-EXPLICIT-ACTIONS-002', 'REVIEW-UNRESOLVED-003'], + [ + 'REVIEW-OMITTED-001', + 'REVIEW-WRONG-SESSION-002', + 'REVIEW-SILENT-MUTATION-003', + 'REVIEW-TIMER-005', + 'REVIEW-SKIP-EMPTY-006', + ], + ] as const; for (const [index, path] of fixtureFiles.entries()) { + const ids = requiredFixtureIds[index]; + if (!ids) throw new Error(`Missing required fixture IDs for ${path}`); writeFileSync( join(rootDir, path), - fixtureCase( - `CASE-${String(index + 1).padStart(3, '0')}`, - path.includes('positive') ? 'ACCEPT' : 'REJECT', - ), + ids.map((id) => fixtureCase(id, path.includes('positive') ? 'ACCEPT' : 'REJECT')).join('\n'), ); } @@ -100,4 +111,88 @@ describe('validateSkillAssets', () => { expect(() => validateSkillAssets({ rootDir })).toThrow(/canonical|vendor-specific/i); }); + + it('requires caller-owned capture provenance and Relay-owned fields', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + const skillPath = join(rootDir, 'skills/relay-capture/SKILL.md'); + writeFileSync( + skillPath, + readFileSync(skillPath, 'utf-8').replace( + /The agent supplies createdByName and the exact active session ID\. Relay supplies createdByType: AGENT and status: INBOX\./, + 'Provide provenance for the capture.', + ), + ); + + expect(() => validateSkillAssets({ rootDir })).toThrow( + /createdByName|session ID|createdByType|INBOX|status/i, + ); + }); + + it('requires session review before completion even when the result is empty', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + const skillPath = join(rootDir, 'skills/relay-session-review/SKILL.md'); + writeFileSync( + skillPath, + readFileSync(skillPath, 'utf-8').replace( + 'Always perform the exact active session lookup before final completion.', + 'Review before final completion when captures may exist.', + ), + ); + + expect(() => validateSkillAssets({ rootDir })).toThrow( + /conditional|final completion|session lookup/i, + ); + }); + + it('rejects valid-looking fixtures that omit required issue coverage', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + writeFileSync( + join(rootDir, 'skills/fixtures/capture-positive.md'), + fixtureCase('CASE-001', 'ACCEPT'), + ); + + expect(() => validateSkillAssets({ rootDir })).toThrow( + /required coverage|CAPTURE-ACTIONABLE-001/i, + ); + }); + + it.each([ + ['capture', 'An agent may autonomously archive low-priority tasks.', /forbidden|archive/i], + [ + 'review', + 'Skip the exact session lookup when the agent believes there are no captures.', + /forbidden|lookup|skip/i, + ], + [ + 'capture', + 'The agent may parse decorative CLI output instead of JSON.', + /forbidden|decorative|JSON/i, + ], + [ + 'capture', + 'The agent may store full source files and secrets as context.', + /forbidden|source|secret/i, + ], + ])('rejects contradictory unsafe %s policy: %s', (skill, unsafePolicy, errorPattern) => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + const skillPath = join( + rootDir, + skill === 'capture' + ? 'skills/relay-capture/SKILL.md' + : 'skills/relay-session-review/SKILL.md', + ); + writeFileSync(skillPath, `${readFileSync(skillPath, 'utf-8')}\n${unsafePolicy}\n`); + + expect(() => validateSkillAssets({ rootDir })).toThrow(errorPattern); + }); + + it('allows canonical prohibition wording without treating it as permission', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + expect(() => validateSkillAssets({ rootDir })).not.toThrow(); + }); }); From 6c66400e4072d2278f8f45e79395f636ef65a4a3 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Tue, 28 Jul 2026 23:32:49 +0530 Subject: [PATCH 8/8] Address follow-up review findings --- scripts/validate-repository-assets.ts | 55 ++++++++++++------- scripts/validate-skill-assets.ts | 34 +++++++++--- skills/fixtures/session-review-negative.md | 2 +- .../validate-repository-assets.test.ts | 23 ++++++++ .../scripts/validate-skill-assets.test.ts | 30 ++++++++++ 5 files changed, 114 insertions(+), 30 deletions(-) diff --git a/scripts/validate-repository-assets.ts b/scripts/validate-repository-assets.ts index 4cb1db6..59832d8 100644 --- a/scripts/validate-repository-assets.ts +++ b/scripts/validate-repository-assets.ts @@ -32,32 +32,42 @@ function walkFiles(rootDir: string, startDir = rootDir): string[] { return files; } -function validateMarkdownLinks(markdownPath: string, content: string): void { - const linkPattern = /\[[^\]]+\]\(([^)]+)\)/g; +function extractMarkdownLinkTargets(content: string): readonly string[] { + const withoutCode = content.replace(/```[\s\S]*?```/g, '').replace(/`[^`\r\n]*`/g, ''); + return [...withoutCode.matchAll(/\[[^\]]+\]\(([^)]+)\)/g)] + .map((match) => match[1]?.trim()) + .filter((target): target is string => Boolean(target)); +} - for (const match of content.matchAll(linkPattern)) { - const rawTarget = match[1]; - if (!rawTarget) { - continue; - } +function normalizeMarkdownLinkTarget(rawTarget: string): string | undefined { + let target = rawTarget.trim(); + if (target.startsWith('<')) { + const closingBracket = target.indexOf('>'); + target = closingBracket >= 0 ? target.slice(1, closingBracket) : target; + } else { + target = target.split(/\s+(?=["'])/)[0] ?? target; + } + const cleanTarget = target.split('#')[0]?.split('?')[0]; + return cleanTarget?.replaceAll('\\', '/').replace(/^\.\//, '') || undefined; +} + +function validateMarkdownLinks(markdownPath: string, content: string): void { + for (const rawTarget of extractMarkdownLinkTargets(content)) { + const normalizedTarget = normalizeMarkdownLinkTarget(rawTarget); + if (!normalizedTarget) continue; if ( - rawTarget.startsWith('http://') || - rawTarget.startsWith('https://') || - rawTarget.startsWith('mailto:') || - rawTarget.startsWith('#') + normalizedTarget.startsWith('http://') || + normalizedTarget.startsWith('https://') || + normalizedTarget.startsWith('mailto:') || + normalizedTarget.startsWith('#') ) { continue; } - const cleanTarget = rawTarget.split('#')[0]?.split('?')[0]; - if (!cleanTarget) { - continue; - } - - const resolvedTarget = isAbsolute(cleanTarget) - ? cleanTarget - : resolve(markdownPath, '..', cleanTarget); + const resolvedTarget = isAbsolute(normalizedTarget) + ? normalizedTarget + : resolve(markdownPath, '..', normalizedTarget); if (!existsSync(resolvedTarget)) { fail(`README local link does not resolve: ${rawTarget}`); @@ -198,12 +208,17 @@ export function validateRepositoryAssets(options: ValidateRepositoryAssetsOption if (!readme.includes('dist/cli/main.js') || !cliReference.includes('dist/cli/main.js')) { fail('README.md and docs/cli-reference.md must document the built CLI invocation.'); } + const readmeLinkTargets = new Set( + extractMarkdownLinkTargets(readme) + .map(normalizeMarkdownLinkTarget) + .filter((target): target is string => Boolean(target)), + ); for (const requiredLink of [ 'docs/agent-skills.md', 'skills/relay-capture/SKILL.md', 'skills/relay-session-review/SKILL.md', ]) { - if (!readme.includes(requiredLink)) { + if (!readmeLinkTargets.has(requiredLink)) { fail(`README.md must link to ${requiredLink}.`); } } diff --git a/scripts/validate-skill-assets.ts b/scripts/validate-skill-assets.ts index 6d5acbf..ae95eee 100644 --- a/scripts/validate-skill-assets.ts +++ b/scripts/validate-skill-assets.ts @@ -96,6 +96,7 @@ function validateFixtureCoverage(fixturePath: string, cases: readonly SkillFixtu 'REVIEW-SILENT-MUTATION-003', 'REVIEW-TIMER-005', 'REVIEW-SKIP-EMPTY-006', + 'REVIEW-GENERIC-MUTATION-007', ]; for (const id of required) { if (!cases.some((fixtureCase) => fixtureCase.id === id)) @@ -161,7 +162,7 @@ const forbiddenPolicyRules: readonly ForbiddenPolicyRule[] = [ skill: 'review', label: 'skipping the exact-session lookup', pattern: - /(? { + const skillName = canonicalPath.includes('relay-capture') + ? /relay-capture|Relay Capture/i + : /relay-session-review|Relay Session Review/i; + return skillName.test(content); + }); + if (candidates.length === 0) continue; + if (candidates.length > 1) { + fail(`Vendor-specific Relay policy must identify one canonical source: ${path}.`); + } + const canonical = candidates[0]; + if (!canonical) { + fail(`Vendor-specific Relay policy must identify a canonical source: ${path}.`); + } + const canonicalName = canonical.includes('relay-capture') + ? 'relay-capture' + : 'relay-session-review'; + const sourcePattern = new RegExp( + `${canonicalName}/SKILL\\.md|${canonical.replace(/[.*+?^${}()|[\\]\\]/g, '\\$&')}`, + 'i', + ); + if (!sourcePattern.test(content)) { + fail(`Vendor-specific Relay policy must explicitly reference its canonical source: ${path}.`); } } } diff --git a/skills/fixtures/session-review-negative.md b/skills/fixtures/session-review-negative.md index c900ec7..0a1cf74 100644 --- a/skills/fixtures/session-review-negative.md +++ b/skills/fixtures/session-review-negative.md @@ -94,7 +94,7 @@ Skip `session_captures_list` before final completion because the expected result The exact active-session lookup is mandatory and an empty authoritative result is valid. -## REVIEW-GENERIC-MUTATION-006 +## REVIEW-GENERIC-MUTATION-007 Expected: REJECT diff --git a/tests/unit/scripts/validate-repository-assets.test.ts b/tests/unit/scripts/validate-repository-assets.test.ts index 6cd63d6..efc63f5 100644 --- a/tests/unit/scripts/validate-repository-assets.test.ts +++ b/tests/unit/scripts/validate-repository-assets.test.ts @@ -109,6 +109,7 @@ function createFixtureRoot(): string { 'REVIEW-SILENT-MUTATION-003', 'REVIEW-TIMER-005', 'REVIEW-SKIP-EMPTY-006', + 'REVIEW-GENERIC-MUTATION-007', ], ][index]; if (!requiredIds) throw new Error(`Missing required fixture IDs for ${filename}`); @@ -223,4 +224,26 @@ describe('validateRepositoryAssets', () => { expect(() => validateRepositoryAssets({ rootDir })).toThrow(/README.*agent-skills/i); }); + + it('does not count plain text or fenced code mentions as README links', () => { + const rootDir = createFixtureRoot(); + createdRoots.push(rootDir); + writeFileSync( + join(rootDir, 'README.md'), + '# Relay\n\n`node dist/cli/main.js`\n\n```md\n[Agent skills](docs/agent-skills.md)\n[Capture skill](skills/relay-capture/SKILL.md)\n[Review skill](skills/relay-session-review/SKILL.md)\n```\n', + ); + + expect(() => validateRepositoryAssets({ rootDir })).toThrow(/README.*agent-skills/i); + }); + + it('accepts normalized Markdown links to canonical skill guidance', () => { + const rootDir = createFixtureRoot(); + createdRoots.push(rootDir); + writeFileSync( + join(rootDir, 'README.md'), + '# Relay\n\n`node dist/cli/main.js`\n\n[Agent skills](./docs/agent-skills.md#overview)\n[Capture skill](./skills/relay-capture/SKILL.md?source=readme)\n[Review skill](./skills/relay-session-review/SKILL.md)\n', + ); + + expect(() => validateRepositoryAssets({ rootDir })).not.toThrow(); + }); }); diff --git a/tests/unit/scripts/validate-skill-assets.test.ts b/tests/unit/scripts/validate-skill-assets.test.ts index 94d3d29..c539af4 100644 --- a/tests/unit/scripts/validate-skill-assets.test.ts +++ b/tests/unit/scripts/validate-skill-assets.test.ts @@ -50,6 +50,7 @@ function createValidSkillFixtureRoot(): string { 'REVIEW-SILENT-MUTATION-003', 'REVIEW-TIMER-005', 'REVIEW-SKIP-EMPTY-006', + 'REVIEW-GENERIC-MUTATION-007', ], ] as const; for (const [index, path] of fixtureFiles.entries()) { @@ -112,6 +113,23 @@ describe('validateSkillAssets', () => { expect(() => validateSkillAssets({ rootDir })).toThrow(/canonical|vendor-specific/i); }); + it('accepts vendor skills that explicitly reference either canonical policy source', () => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + mkdirSync(join(rootDir, 'integrations/codex/relay-capture'), { recursive: true }); + mkdirSync(join(rootDir, 'integrations/codex/relay-session-review'), { recursive: true }); + writeFileSync( + join(rootDir, 'integrations/codex/relay-capture/SKILL.md'), + '# Relay Capture\n\nSee [canonical source](../../../../skills/relay-capture/SKILL.md).\n', + ); + writeFileSync( + join(rootDir, 'integrations/codex/relay-session-review/SKILL.md'), + '# Relay Session Review\n\nSee [canonical source](../../../../skills/relay-session-review/SKILL.md).\n', + ); + + expect(() => validateSkillAssets({ rootDir })).not.toThrow(); + }); + it('requires caller-owned capture provenance and Relay-owned fields', () => { const rootDir = createValidSkillFixtureRoot(); createdRoots.push(rootDir); @@ -195,4 +213,16 @@ describe('validateSkillAssets', () => { createdRoots.push(rootDir); expect(() => validateSkillAssets({ rootDir })).not.toThrow(); }); + + it.each([ + 'Do not skip the exact session lookup before final completion.', + 'The agent must not omit the exact session lookup before final completion.', + ])('allows exact-session prohibitions: %s', (prohibition) => { + const rootDir = createValidSkillFixtureRoot(); + createdRoots.push(rootDir); + const skillPath = join(rootDir, 'skills/relay-session-review/SKILL.md'); + writeFileSync(skillPath, `${readFileSync(skillPath, 'utf-8')}\n${prohibition}\n`); + + expect(() => validateSkillAssets({ rootDir })).not.toThrow(); + }); });