Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. 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

- Node.js `24.x` (see `.nvmrc`)
Expand Down
9 changes: 9 additions & 0 deletions docs/agent-skills.md
Original file line number Diff line number Diff line change
@@ -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 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.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
```

Expand Down
117 changes: 117 additions & 0 deletions docs/superpowers/plans/2026-07-28-pr-32-review-remediation.md
Original file line number Diff line number Diff line change
@@ -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.
23 changes: 21 additions & 2 deletions scripts/validate-repository-assets.ts
Original file line number Diff line number Diff line change
@@ -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}`);
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -190,18 +198,29 @@ 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}.`);
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const allFiles = walkFiles(rootDir);

validateSkillAssets({ rootDir });

validateJsonFiles(allFiles);
validatePlaceholders(allFiles);
validateMarkdownLinks(
join(rootDir, 'README.md'),
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}`);
Expand Down
Loading
Loading