diff --git a/docs/design/feedback-pipeline-explainer.html b/docs/design/feedback-pipeline-explainer.html new file mode 100644 index 000000000..252029f96 --- /dev/null +++ b/docs/design/feedback-pipeline-explainer.html @@ -0,0 +1,214 @@ + + + + + + Feedback Pipeline Architecture Explainer + + + +
+
+

Feedback Pipeline Explainer

+

+ Durable architecture reference for the report -> issue -> agent-session -> PR pipeline, + including fork-aware execution and governance controls. +

+
+ +
+
+

Formal Pipeline

+
    +
  1. Capture and validate report payload.
    2. +
  2. Resolve issue via dedupe markers (create or update/comment).
    3. +
  3. Plan follow-up: issue-only, issue+PR, or issue+fork.
    4. +
  4. Execute direct SCM operations or spawn agent session for code changes.
    5. +
  5. Link issue/fork/PR and update operation journal.
    6. +
+
+ +
+

Execution Boundaries

+
    +
  • Orchestrator is the only component allowed to perform SCM side effects.
    • +
  • Optional subagent skill can provide recommendation signals only.
    • +
  • Policy fallback remains deterministic when subagent output is invalid.
    • +
+
+
+ +
+
+

Trigger Conditions

+
    +
  • Valid report in mode=scm.
    • +
  • Confidence thresholds met.
    • +
  • Governance allows attempted mutation path.
    • +
+
+ +
+

Target Selection

+
    +
  • Prefer upstream when writable and policy allows.
    • +
  • Use fork path when upstream writes are blocked.
    • +
  • Downgrade to issue-only when neither path is permitted.
    • +
+
+ +
+

Idempotency

+
    +
  • dedupeKey for issue identity.
    • +
  • operationKey per mutation stage.
    • +
  • Find-or-create semantics before every mutation.
    • +
+
+
+ +
+
+

Consent Gates (Default Policy)

+
    +
  • Outside AO dogfooding, human consent is required for fork creation.
    • +
  • Outside AO dogfooding, human consent is required for PR creation.
    • +
  • Outside AO dogfooding, human consent is required before upstream/fork target switch.
    • +
  • Project-level bypass is allowed only when owner explicitly enables it.
    • +
+
+ +
+

Journal (Plain Language)

+
    +
  • The journal is a progress log for every report.
    • +
  • Each mutation updates one tracked record, not separate duplicates.
    • +
  • Retries keep the same keys and increment attempt metadata.
    • +
  • Consent outcomes are stored for auditability.
    • +
+
{
+  "dedupeKey": "f4d7dbe5b0f8...",
+  "stage": "create_pr",
+  "status": "failed",
+  "attempt": 2,
+  "issueUrl": ".../issues/399",
+  "prUrl": null
+}
+
+
+ +
+
+

PR Requirements

+
    +
  • Issue reference must exist in PR metadata/body.
    • +
  • Stable lineage markers link issue, session, and PR.
    • +
  • Existing PR for same dedupe context must be linked, not duplicated.
    • +
+
+ +
+

Governance Hooks

+
    +
  • canCreateIssue / canCreateFork / canCreatePR / canSpawnSession checks.
    • +
  • Per-fork-owner allow/deny and optional approval gates.
    • +
  • Denied operations are journaled with explicit reason codes.
    • +
+
+
+
+ + diff --git a/docs/design/feedback-routing-and-followup-design.md b/docs/design/feedback-routing-and-followup-design.md new file mode 100644 index 000000000..52c7351f4 --- /dev/null +++ b/docs/design/feedback-routing-and-followup-design.md @@ -0,0 +1,290 @@ +# Feedback Routing and Follow-up Design (Formalized Pipeline v2) + +## Status + +This is a design formalization update after PR #403 discussion. + +- PR #403 remains implementation-scoped to feedback contracts/validation/storage. +- This document defines the next-step architecture for report -> issue -> agent-session -> PR. +- This update is design-only (no new runtime behavior introduced by this document itself). + +## Scope and Decisions + +1. Feedback routing mode is exclusive: `local` OR `scm` (never both). +2. Privacy guardrails are intentionally deferred to a dedicated follow-up PR. +3. Side effects stay deterministic in orchestrator control code. +4. Optional subagent/skill can recommend decisions, but cannot execute SCM mutations. + +## Formal Pipeline + +1. **Report capture**: validate feedback tool payload and compute dedupe key. +2. **Issue resolution**: find existing issue by markers; create or comment/update. +3. **Follow-up planning**: decide issue-only vs issue+PR vs issue+fork from policy + context. +4. **Execution**: + - direct SCM action path (issue/fork/PR metadata operations), or + - agent-session path (spawn session to produce code changes). +5. **Linking and journal update**: persist outcome state and references. + +## 1) Trigger Conditions + +The pipeline is triggered when all of the following are true: + +1. A valid `bug_report` or `improvement_suggestion` is captured. +2. Routing mode is `scm` for the active project. +3. Confidence threshold is met for that report type. +4. Governance policy allows target/fork mutation for this actor/project. + +Decision triggers for follow-up action: + +1. `self_blocking_now = false` -> issue-only. +2. `self_blocking_now = true` + ready branch/commits -> issue + PR link/create path. +3. `self_blocking_now = true` + no writable upstream path -> issue + fork path. +4. `self_blocking_now = true` + no code yet -> spawn agent-session path. + +## 2) Session Spawning Contract + +When follow-up requires code (not just metadata operations), orchestrator spawns a worker session. + +### Inputs + +1. `reportId`, `dedupeKey`, `issueUrl` (resolved in prior stage). +2. `targetRepo` and `targetBranchPolicy`. +3. `followUpIntent` (`fix_now`, `draft_solution`, etc.). +4. Optional `forkContext` (fork owner/repo/branch if fork path selected). + +### Preconditions + +1. Issue is already resolved/created and has stable URL. +2. Spawn policy permits automatic coding session for this project. +3. Repo target has been selected (upstream or fork). + +### Required outputs + +1. Session ID. +2. Branch reference used by the session. +3. Optional PR URL if created by orchestrator or agent flow. +4. Terminal state recorded in publish journal (`done`, `failed`, `cancelled`). + +## 3) Target Selection (Upstream vs Fork) + +Target selection is deterministic and policy-driven: + +1. If upstream write is allowed and policy is `upstream`, target upstream. +2. If upstream write is blocked and policy allows fork, target fork. +3. If fork exists and policy says reuse, reuse existing fork. +4. If fork missing and policy allows creation, create fork and continue. +5. If neither upstream nor fork is allowed, downgrade to issue-only and mark follow-up blocked. + +Example policy knobs: + +```yaml +feedback: + mode: scm + scm: + provider: github + targetRepo: auto # auto | upstream | fork + forkStrategy: upstream # upstream | fork | skip +``` + +## Consent Gates (Default Policy) + +For projects other than AO dogfooding, these are hard defaults: + +1. Explicit human consent is required before creating a fork. +2. Explicit human consent is required before creating a PR. +3. Explicit human consent is required before switching execution target between upstream and fork. +4. No silent infrastructure flip is allowed by default. + +Override model: + +1. Project-level override is optional and must be explicitly enabled by project owner. +2. Overrides are scoped per operation (`createFork`, `createPR`, `switchTarget`) and must be auditable. +3. Without explicit owner override, consent gate defaults remain enforced. + +## 4) PR Creation/Linking Requirements + +For any PR action, orchestrator enforces: + +1. Issue URL exists and is referenced in PR body. +2. Dedupe marker is present in issue/PR metadata for traceability. +3. If PR already exists for dedupe key + branch, link existing PR rather than creating duplicate. +4. If fork path is used, PR must include fork repo/branch references. +5. Issue must be updated with final PR URL and state transitions. + +Canonical markers in issue/PR body: + +1. `` +2. `` +3. `` (if session spawned) + +## 5) Idempotency and Retry Semantics + +Idempotency keys: + +1. `dedupeKey` for issue-level identity. +2. `operationKey` for each side effect (create issue, create fork, create PR, add comment). + +Retry semantics: + +1. Retry only retryable transport/server failures. +2. Exponential backoff with bounded attempts. +3. Non-retryable errors transition to terminal failure with actionable reason. + +At-least-once safety: + +1. Replays must first check existing issue/fork/PR using markers before creating anything. +2. Side effects must be written as "find-or-create" operations. + +Journal semantics: + +1. Minimal journal record per report in `scm` mode: + - `dedupeKey`, `stage`, `status`, `issueUrl`, `prUrl`, `targetRepo`, `lastError`. +2. Journal drives recovery and prevents duplicate creation on restart. + +Plain-language journal behavior: + +1. Think of the journal as a progress log for each report. +2. Before each mutation attempt, orchestrator writes what it is about to do. +3. After attempt completion, orchestrator updates the same record with success/failure and links. +4. On retry, orchestrator keeps the same identity keys and increments attempt metadata instead of creating a parallel track. +5. Consent decisions (approved/denied) are written so operators can audit why a path was or was not taken. + +Minimal journal schema example: + +```json +{ + "reportId": "fr_01HT2H2F3H4A5", + "dedupeKey": "f4d7dbe5b0f8...", + "mode": "scm", + "stage": "create_pr", + "status": "failed", + "attempt": 2, + "operationKey": "create_pr:f4d7dbe5b0f8:upstream", + "targetRepo": "ComposioHQ/agent-orchestrator", + "issueUrl": "https://github.com/ComposioHQ/agent-orchestrator/issues/399", + "prUrl": null, + "consent": { + "createFork": "approved", + "createPR": "approved", + "switchTarget": "not-needed" + }, + "lastError": { + "code": "FORBIDDEN", + "message": "PR creation blocked by repository policy" + }, + "updatedAt": "2026-03-10T15:45:00Z" +} +``` + +## 6) Governance Hooks Per Fork Owner Policy + +Governance is evaluated before each mutating operation. + +Policy hooks: + +1. `canCreateIssue(project, actor, targetRepo)` +2. `canCreateFork(project, actor, forkOwner)` +3. `canCreatePR(project, actor, targetRepo, sourceRepo)` +4. `canSpawnSession(project, actor, followUpIntent)` + +Per-fork-owner controls: + +1. Allowed fork owner list / deny list. +2. Require human approval for fork creation under selected owners. +3. Optional restriction to pre-registered fork remotes. + +If governance denies an operation: + +1. Do not attempt mutation. +2. Downgrade path if possible (e.g., issue-only). +3. Record explicit denial reason in journal. + +## Responsibilities: Orchestrator vs Subagent Skill + +### Orchestrator (required) + +1. Owns all SCM side effects. +2. Owns retries, idempotency checks, and journal updates. +3. Enforces governance hooks and policy fallbacks. + +### Optional ephemeral subagent skill + +1. Produces recommendation payload only: + - `self_blocking_now` + - `recommended_action` + - `reason` + - `confidence` +2. Must return strict JSON schema. +3. If invalid/low confidence, orchestrator falls back to deterministic rules. + +## Proposed Components + +1. `FeedbackRouter`: local vs scm dispatch. +2. `IssueResolver`: dedupe-aware issue create/update/comment. +3. `FollowUpPlanner`: issue-only vs issue+PR vs issue+fork decision. +4. `TargetResolver`: upstream/fork target determination. +5. `FollowUpExecutor`: direct SCM or agent-session execution path. +6. `FeedbackPublishJournal`: status, links, retries, recovery metadata. + +## Config Proposal (Extended) + +```yaml +feedback: + mode: scm # local | scm + scm: + provider: github # github | gitlab + targetRepo: auto # auto | upstream | fork + forkStrategy: upstream # upstream | fork | skip + prReference: if_present # required | if_present | never + minConfidence: + bug_report: 0.6 + improvement_suggestion: 0.75 + followUp: + enableAgentSession: true + requireIssueBeforeSession: true + consent: + defaultPolicy: require_human_for_major_mutations # hard default outside AO dogfooding + requireFor: + createFork: true + createPR: true + switchTarget: true + projectOverride: + enabled: false # must be explicitly enabled by project owner + governance: + allowedForkOwners: [""] + requireApprovalForForkCreation: true +``` + +## Testing Strategy + +### Unit tests + +1. Trigger matrix and planner decision table. +2. Target resolver behavior for upstream/fork permutations. +3. Marker generation and dedupe matching. +4. Governance hook allow/deny behavior. +5. Retry classifier and idempotent replay logic. + +### Integration tests + +1. Issue create/update on GitHub and GitLab. +2. Fork ensure/reuse path under different policies. +3. PR create/link and issue back-link updates. +4. Agent-session contract handoff and journal transitions. + +### End-to-end tests + +1. report -> issue-only path. +2. report -> issue -> session -> PR path. +3. report -> issue -> fork -> session -> PR-from-fork path. + +## Rollout + +1. Ship with `mode: local` default and `scm` opt-in. +2. Enable on a small set of projects first. +3. Track duplicate suppression rate, retry outcomes, and failure classes. +4. Expand once deterministic behavior and governance policy outcomes are stable. + +## Summary + +The formal pipeline is now explicit: report -> issue -> (optional) agent-session -> PR, with fork-aware execution and governance hooks. Orchestrator remains the only executor of side effects; agentic skill remains advisory. diff --git a/docs/reviews/ao22-design-iteration-summary.md b/docs/reviews/ao22-design-iteration-summary.md new file mode 100644 index 000000000..830777325 --- /dev/null +++ b/docs/reviews/ao22-design-iteration-summary.md @@ -0,0 +1,33 @@ +# AO22 Design Iteration Summary + +## What was updated + +1. `docs/design/feedback-routing-and-followup-design.md` + - Formalized the full pipeline: report -> issue -> agent-session -> PR. + - Added explicit sections for: + - trigger conditions, + - session spawning contract, + - target selection (upstream vs fork), + - PR creation/linking requirements, + - idempotency/retry semantics, + - governance hooks per fork-owner policy. + +2. `docs/design/feedback-pipeline-explainer.html` + - Added a durable architecture explainer page (non-PR-specific). + - Mirrors the same six design contracts and the formal pipeline. + +## Intent + +This iteration is design formalization only. No runtime/code behavior was introduced by this request. + +## Feedback incorporated + +- Added a dedicated `Consent Gates (Default Policy)` section with hard defaults for non-AO dogfooding: + - explicit approval required for `createFork`, `createPR`, and upstream/fork target switching. +- Clarified that project-level override is optional and only active when explicitly enabled by the owner. +- Expanded journal behavior in plain language to describe how records are written/updated/retried. +- Added a minimal journal schema example for the feedback pipeline. + +## Ready for review + +The docs now provide a deterministic contract for implementing fork-aware report->issue->session->PR execution in follow-up PRs. diff --git a/packages/core/README.md b/packages/core/README.md index 29d6c34eb..82f4167b8 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -139,6 +139,51 @@ Loads and validates `agent-orchestrator.yaml`: 2. Wire it up in the polling loop 3. Add config schema in `src/config.ts` if new reaction type +### Feedback Tools (v1) + +`@composio/ao-core` exports two structured feedback tool contracts: + +- `bug_report` +- `improvement_suggestion` + +Both share the same required input fields: + +- `title` +- `body` +- `evidence` (array of strings) +- `session` +- `source` +- `confidence` (0..1) + +Example: + +```ts +import { FEEDBACK_TOOL_NAMES, FeedbackReportStore, getFeedbackReportsDir } from "@composio/ao-core"; + +const reportsDir = getFeedbackReportsDir(configPath, projectPath); +const store = new FeedbackReportStore(reportsDir); + +const saved = store.persist(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + title: "SSO login loop", + body: "Google SSO redirects back to /login repeatedly.", + evidence: ["trace_id=abc123", "screenshot: login-loop.png"], + session: "ao-22", + source: "agent", + confidence: 0.84, +}); +``` + +Storage format: + +- Reports are persisted under `~/.agent-orchestrator/{hash}-{projectId}/feedback-reports` +- Each report is a typed key=value file (`report__.kv`) for easy inspection +- A deterministic dedupe key (`sha256`, 16 hex chars) is generated from normalized tool+content + +Migration notes: + +- No migration needed for existing AO installs +- The `feedback-reports` directory is created lazily on first persisted report + ## Testing ```bash diff --git a/packages/core/src/__tests__/feedback-tools.test.ts b/packages/core/src/__tests__/feedback-tools.test.ts new file mode 100644 index 000000000..ca05fd063 --- /dev/null +++ b/packages/core/src/__tests__/feedback-tools.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, it, beforeEach, afterEach } from "vitest"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { randomUUID } from "node:crypto"; +import { tmpdir } from "node:os"; +import { + FEEDBACK_TOOL_CONTRACTS, + FEEDBACK_TOOL_NAMES, + FeedbackReportStore, + generateFeedbackDedupeKey, + validateFeedbackToolInput, +} from "../feedback-tools.js"; + +const validPayload = { + title: "Login failure for SSO users", + body: "Users with Google SSO are looped back to login.", + evidence: ["trace_id=abc123", "Video capture from session"], + session: "ao-22", + source: "agent", + confidence: 0.82, +}; + +describe("feedback tool contracts", () => { + it("defines both v1 tool contracts", () => { + expect(Object.keys(FEEDBACK_TOOL_CONTRACTS).sort()).toEqual([ + FEEDBACK_TOOL_NAMES.BUG_REPORT, + FEEDBACK_TOOL_NAMES.IMPROVEMENT_SUGGESTION, + ]); + }); + + it("validates required fields for bug_report", () => { + expect(() => + validateFeedbackToolInput(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + title: "", + }), + ).toThrow(); + + expect(() => + validateFeedbackToolInput(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + body: "", + }), + ).toThrow(); + + expect(() => + validateFeedbackToolInput(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + evidence: [], + }), + ).toThrow(); + + expect(() => + validateFeedbackToolInput(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + session: "", + }), + ).toThrow(); + + expect(() => + validateFeedbackToolInput(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + source: "", + }), + ).toThrow(); + }); + + it("rejects malformed confidence", () => { + expect(() => + validateFeedbackToolInput(FEEDBACK_TOOL_NAMES.IMPROVEMENT_SUGGESTION, { + ...validPayload, + confidence: -0.1, + }), + ).toThrow(); + + expect(() => + validateFeedbackToolInput(FEEDBACK_TOOL_NAMES.IMPROVEMENT_SUGGESTION, { + ...validPayload, + confidence: 1.1, + }), + ).toThrow(); + + expect(() => + validateFeedbackToolInput(FEEDBACK_TOOL_NAMES.IMPROVEMENT_SUGGESTION, { + ...validPayload, + confidence: Number.NaN, + }), + ).toThrow(); + }); +}); + +describe("feedback dedupe key", () => { + it("is stable for whitespace/case differences and evidence ordering", () => { + const keyA = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + title: " Login failure FOR SSO users ", + body: "Users with Google SSO are looped back to login.", + evidence: ["Video capture from session", "trace_id=abc123"], + source: "Agent", + }); + + const keyB = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, validPayload); + + expect(keyA).toBe(keyB); + }); + + it("is stable when case-only evidence changes alter default sort order", () => { + const keyA = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + evidence: ["BETA", "alpha"], + }); + const keyB = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + evidence: ["beta", "alpha"], + }); + + expect(keyA).toBe(keyB); + }); + + it("is stable when only confidence changes", () => { + const keyA = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + confidence: 0.21, + }); + const keyB = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + confidence: 0.99, + }); + + expect(keyA).toBe(keyB); + }); + + it("changes when normalized content differs", () => { + const keyA = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, validPayload); + const keyB = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + body: "Users with Google SSO see a 500 after callback.", + }); + + expect(keyA).not.toBe(keyB); + }); + + it("does not collide when one evidence item contains pipes", () => { + const keyA = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + evidence: ["a|b"], + }); + const keyB = generateFeedbackDedupeKey(FEEDBACK_TOOL_NAMES.BUG_REPORT, { + ...validPayload, + evidence: ["a", "b"], + }); + + expect(keyA).not.toBe(keyB); + }); +}); + +describe("feedback report store", () => { + let reportsDir: string; + + beforeEach(() => { + reportsDir = join(tmpdir(), `ao-feedback-${randomUUID()}`); + mkdirSync(reportsDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(reportsDir, { recursive: true, force: true }); + }); + + it("persists and reads structured feedback reports", () => { + const store = new FeedbackReportStore(reportsDir); + + const saved = store.persist(FEEDBACK_TOOL_NAMES.BUG_REPORT, validPayload); + const records = store.list(); + + expect(records).toHaveLength(1); + expect(records[0]).toEqual(saved); + expect(records[0]?.tool).toBe(FEEDBACK_TOOL_NAMES.BUG_REPORT); + expect(records[0]?.dedupeKey).toMatch(/^[a-f0-9]{16}$/); + }); + + it("does not allow invalid reports to be persisted", () => { + const store = new FeedbackReportStore(reportsDir); + + expect(() => + store.persist(FEEDBACK_TOOL_NAMES.IMPROVEMENT_SUGGESTION, { + ...validPayload, + evidence: [], + }), + ).toThrow(); + }); + + it("skips corrupt report files and returns valid ones", () => { + const store = new FeedbackReportStore(reportsDir); + + const saved = store.persist(FEEDBACK_TOOL_NAMES.BUG_REPORT, validPayload); + writeFileSync( + join(reportsDir, "report_2026-03-10T00-00-00-000Z_bad.kv"), + "version=1\nid=report_2026-03-10T00-00-00-000Z_bad\ntool=bug_report\n", + "utf-8", + ); + + const records = store.list(); + expect(records).toEqual([saved]); + }); +}); diff --git a/packages/core/src/__tests__/paths.test.ts b/packages/core/src/__tests__/paths.test.ts index 95ae0bc18..64721d0f0 100644 --- a/packages/core/src/__tests__/paths.test.ts +++ b/packages/core/src/__tests__/paths.test.ts @@ -23,6 +23,7 @@ import { getProjectBaseDir, getSessionsDir, getWorktreesDir, + getFeedbackReportsDir, getArchiveDir, getOriginFilePath, generateSessionName, @@ -271,6 +272,12 @@ describe("Path Construction", () => { expect(worktreesDir).toMatch(/\.agent-orchestrator\/[a-f0-9]{12}-integrator\/worktrees$/); }); + it("getFeedbackReportsDir returns {baseDir}/feedback-reports", () => { + const reportsDir = getFeedbackReportsDir(configPath, "/repos/integrator"); + + expect(reportsDir).toMatch(/\.agent-orchestrator\/[a-f0-9]{12}-integrator\/feedback-reports$/); + }); + it("getArchiveDir returns {baseDir}/sessions/archive", () => { const archiveDir = getArchiveDir(configPath, "/repos/integrator"); @@ -287,10 +294,12 @@ describe("Path Construction", () => { const baseDir = getProjectBaseDir(configPath, "/repos/integrator"); const sessionsDir = getSessionsDir(configPath, "/repos/integrator"); const worktreesDir = getWorktreesDir(configPath, "/repos/integrator"); + const reportsDir = getFeedbackReportsDir(configPath, "/repos/integrator"); const archiveDir = getArchiveDir(configPath, "/repos/integrator"); expect(sessionsDir).toContain(baseDir); expect(worktreesDir).toContain(baseDir); + expect(reportsDir).toContain(baseDir); expect(archiveDir).toContain(baseDir); }); }); diff --git a/packages/core/src/atomic-write.ts b/packages/core/src/atomic-write.ts new file mode 100644 index 000000000..287de327a --- /dev/null +++ b/packages/core/src/atomic-write.ts @@ -0,0 +1,11 @@ +import { renameSync, writeFileSync } from "node:fs"; + +/** + * Atomically write a file by writing to a temp file then renaming. + * rename() is atomic on POSIX, so concurrent writers never produce torn data. + */ +export function atomicWriteFileSync(filePath: string, content: string): void { + const tmpPath = `${filePath}.tmp.${process.pid}.${Date.now()}`; + writeFileSync(tmpPath, content, "utf-8"); + renameSync(tmpPath, filePath); +} diff --git a/packages/core/src/feedback-tools.ts b/packages/core/src/feedback-tools.ts new file mode 100644 index 000000000..a11b49f0b --- /dev/null +++ b/packages/core/src/feedback-tools.ts @@ -0,0 +1,204 @@ +import { createHash, randomUUID } from "node:crypto"; +import { existsSync, mkdirSync, readFileSync, readdirSync, statSync } from "node:fs"; +import { join } from "node:path"; +import { z } from "zod"; +import { atomicWriteFileSync } from "./atomic-write.js"; +import { parseKeyValueContent } from "./key-value.js"; + +export const FEEDBACK_TOOL_NAMES = { + BUG_REPORT: "bug_report", + IMPROVEMENT_SUGGESTION: "improvement_suggestion", +} as const; + +export type FeedbackToolName = (typeof FEEDBACK_TOOL_NAMES)[keyof typeof FEEDBACK_TOOL_NAMES]; + +const normalizeText = (value: string): string => value.trim().replace(/\s+/g, " "); + +const NonEmptyTextSchema = z.string().transform(normalizeText).pipe(z.string().min(1)); + +const FeedbackInputSchema = z + .object({ + title: NonEmptyTextSchema, + body: NonEmptyTextSchema, + evidence: z.array(NonEmptyTextSchema).min(1), + session: NonEmptyTextSchema, + source: NonEmptyTextSchema, + confidence: z.number().finite().min(0).max(1), + }) + .strict(); + +export const BugReportSchema = FeedbackInputSchema; +export const ImprovementSuggestionSchema = FeedbackInputSchema; + +export type BugReportInput = z.infer; +export type ImprovementSuggestionInput = z.infer; +export type FeedbackToolInput = BugReportInput | ImprovementSuggestionInput; + +export interface FeedbackToolContract { + name: FeedbackToolName; + description: string; + schema: typeof FeedbackInputSchema; +} + +export const FEEDBACK_TOOL_CONTRACTS: Record = { + bug_report: { + name: FEEDBACK_TOOL_NAMES.BUG_REPORT, + description: "Capture reproducible bugs found while working in AO sessions.", + schema: BugReportSchema, + }, + improvement_suggestion: { + name: FEEDBACK_TOOL_NAMES.IMPROVEMENT_SUGGESTION, + description: "Capture actionable improvement suggestions discovered in AO sessions.", + schema: ImprovementSuggestionSchema, + }, +}; + +export interface PersistedFeedbackReport extends FeedbackToolInput { + id: string; + tool: FeedbackToolName; + createdAt: string; + dedupeKey: string; +} + +const ReportIdSchema = z.string().regex(/^report_[A-Za-z0-9_-]+$/); +const ISODateSchema = z.string().datetime({ offset: true }); +const DedupeKeySchema = z.string().regex(/^[a-f0-9]{16}$/); + +export function validateFeedbackToolInput( + tool: FeedbackToolName, + input: FeedbackToolInput, +): FeedbackToolInput { + return FEEDBACK_TOOL_CONTRACTS[tool].schema.parse(input); +} + +export function generateFeedbackDedupeKey( + tool: FeedbackToolName, + input: FeedbackToolInput, +): string { + const canonicalEvidence = [...input.evidence] + .map((item) => normalizeText(item).toLowerCase()) + .sort(); + const canonical = JSON.stringify({ + tool, + title: normalizeText(input.title).toLowerCase(), + body: normalizeText(input.body).toLowerCase(), + session: normalizeText(input.session).toLowerCase(), + source: normalizeText(input.source).toLowerCase(), + evidence: canonicalEvidence, + }); + + return createHash("sha256").update(canonical).digest("hex").slice(0, 16); +} + +function serializeReport(report: PersistedFeedbackReport): string { + const lines: string[] = [ + "version=1", + `id=${report.id}`, + `tool=${report.tool}`, + `createdAt=${report.createdAt}`, + `dedupeKey=${report.dedupeKey}`, + `title=${report.title}`, + `body=${report.body}`, + `session=${report.session}`, + `source=${report.source}`, + `confidence=${report.confidence}`, + ]; + + for (const [index, evidenceItem] of report.evidence.entries()) { + lines.push(`evidence.${index}=${evidenceItem}`); + } + + return `${lines.join("\n")}\n`; +} + +function parseReportFile(content: string): PersistedFeedbackReport { + const raw = parseKeyValueContent(content); + + const evidence = Object.entries(raw) + .filter(([key]) => key.startsWith("evidence.")) + .map(([key, value]) => ({ + index: Number.parseInt(key.split(".")[1] ?? "0", 10), + value, + })) + .sort((a, b) => a.index - b.index) + .map((item) => item.value); + + const tool = raw["tool"]; + if ( + tool !== FEEDBACK_TOOL_NAMES.BUG_REPORT && + tool !== FEEDBACK_TOOL_NAMES.IMPROVEMENT_SUGGESTION + ) { + throw new Error(`Invalid feedback report tool type: ${tool ?? "unknown"}`); + } + + const parsedInput = FEEDBACK_TOOL_CONTRACTS[tool].schema.parse({ + title: raw["title"] ?? "", + body: raw["body"] ?? "", + evidence, + session: raw["session"] ?? "", + source: raw["source"] ?? "", + confidence: Number(raw["confidence"]), + }); + + return { + id: ReportIdSchema.parse(raw["id"] ?? ""), + tool, + createdAt: ISODateSchema.parse(raw["createdAt"] ?? ""), + dedupeKey: DedupeKeySchema.parse(raw["dedupeKey"] ?? ""), + ...parsedInput, + }; +} + +function isReportFileName(fileName: string): boolean { + return /^report_[A-Za-z0-9_-]+\.kv$/.test(fileName); +} + +export class FeedbackReportStore { + constructor(private readonly reportsDir: string) {} + + persist(tool: FeedbackToolName, input: FeedbackToolInput): PersistedFeedbackReport { + const validated = validateFeedbackToolInput(tool, input); + const createdAt = new Date().toISOString(); + const dedupeKey = generateFeedbackDedupeKey(tool, validated); + const id = `report_${createdAt.replace(/[:.]/g, "-")}_${randomUUID().slice(0, 8)}`; + + const report: PersistedFeedbackReport = { + id, + tool, + createdAt, + dedupeKey, + ...validated, + }; + + mkdirSync(this.reportsDir, { recursive: true }); + const filePath = join(this.reportsDir, `${id}.kv`); + atomicWriteFileSync(filePath, serializeReport(report)); + + return report; + } + + list(): PersistedFeedbackReport[] { + if (!existsSync(this.reportsDir)) return []; + + const reports: PersistedFeedbackReport[] = []; + for (const name of readdirSync(this.reportsDir)) { + if (!isReportFileName(name)) continue; + const reportPath = join(this.reportsDir, name); + try { + if (!statSync(reportPath).isFile()) continue; + } catch { + continue; + } + + try { + const content = readFileSync(reportPath, "utf-8"); + const parsed = parseReportFile(content); + reports.push(parsed); + } catch { + continue; + } + } + + return reports.sort((a, b) => a.createdAt.localeCompare(b.createdAt)); + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9526c66c2..481bd35d2 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -99,6 +99,25 @@ export { asValidOpenCodeSessionId } from "./opencode-session-id.js"; export { normalizeOrchestratorSessionStrategy } from "./orchestrator-session-strategy.js"; export type { NormalizedOrchestratorSessionStrategy } from "./orchestrator-session-strategy.js"; +// Feedback tools — contracts, validation, and report storage +export { + FEEDBACK_TOOL_NAMES, + FEEDBACK_TOOL_CONTRACTS, + BugReportSchema, + ImprovementSuggestionSchema, + validateFeedbackToolInput, + generateFeedbackDedupeKey, + FeedbackReportStore, +} from "./feedback-tools.js"; +export type { + FeedbackToolName, + FeedbackToolContract, + BugReportInput, + ImprovementSuggestionInput, + FeedbackToolInput, + PersistedFeedbackReport, +} from "./feedback-tools.js"; + // Path utilities — hash-based directory structure export { generateConfigHash, @@ -108,6 +127,7 @@ export { getProjectBaseDir, getSessionsDir, getWorktreesDir, + getFeedbackReportsDir, getArchiveDir, getOriginFilePath, generateSessionName, diff --git a/packages/core/src/key-value.ts b/packages/core/src/key-value.ts new file mode 100644 index 000000000..27ad3dba3 --- /dev/null +++ b/packages/core/src/key-value.ts @@ -0,0 +1,18 @@ +/** + * Parse a key=value file into a record. + * Lines starting with # are comments. Empty lines are skipped. + * Only the first "=" is used as the delimiter (values can contain "="). + */ +export function parseKeyValueContent(content: string): Record { + const result: Record = {}; + for (const line of content.split("\n")) { + const trimmed = line.trim(); + if (!trimmed || trimmed.startsWith("#")) continue; + const eqIndex = trimmed.indexOf("="); + if (eqIndex === -1) continue; + const key = trimmed.slice(0, eqIndex).trim(); + const value = trimmed.slice(eqIndex + 1).trim(); + if (key) result[key] = value; + } + return result; +} diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index 1247b9e9c..7010e3f88 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -22,7 +22,6 @@ import { readFileSync, writeFileSync, - renameSync, existsSync, mkdirSync, unlinkSync, @@ -34,25 +33,8 @@ import { } from "node:fs"; import { join, dirname } from "node:path"; import type { SessionId, SessionMetadata } from "./types.js"; - -/** - * Parse a key=value metadata file into a record. - * Lines starting with # are comments. Empty lines are skipped. - * Only the first `=` is used as the delimiter (values can contain `=`). - */ -function parseMetadataFile(content: string): Record { - const result: Record = {}; - for (const line of content.split("\n")) { - const trimmed = line.trim(); - if (!trimmed || trimmed.startsWith("#")) continue; - const eqIndex = trimmed.indexOf("="); - if (eqIndex === -1) continue; - const key = trimmed.slice(0, eqIndex).trim(); - const value = trimmed.slice(eqIndex + 1).trim(); - if (key) result[key] = value; - } - return result; -} +import { atomicWriteFileSync } from "./atomic-write.js"; +import { parseKeyValueContent } from "./key-value.js"; /** Serialize a record back to key=value format. */ function serializeMetadata(data: Record): string { @@ -64,16 +46,6 @@ function serializeMetadata(data: Record): string { ); } -/** - * Atomically write a file by writing to a temp file then renaming. - * rename() is atomic on POSIX, so concurrent writers never produce torn data. - */ -function atomicWriteFileSync(filePath: string, content: string): void { - const tmpPath = `${filePath}.tmp.${process.pid}.${Date.now()}`; - writeFileSync(tmpPath, content, "utf-8"); - renameSync(tmpPath, filePath); -} - /** Validate sessionId to prevent path traversal. */ const VALID_SESSION_ID = /^[a-zA-Z0-9_-]+$/; @@ -97,7 +69,7 @@ export function readMetadata(dataDir: string, sessionId: SessionId): SessionMeta if (!existsSync(path)) return null; const content = readFileSync(path, "utf-8"); - const raw = parseMetadataFile(content); + const raw = parseKeyValueContent(content); return { worktree: raw["worktree"] ?? "", @@ -133,7 +105,7 @@ export function readMetadataRaw( ): Record | null { const path = metadataPath(dataDir, sessionId); if (!existsSync(path)) return null; - return parseMetadataFile(readFileSync(path, "utf-8")); + return parseKeyValueContent(readFileSync(path, "utf-8")); } /** @@ -187,7 +159,7 @@ export function updateMetadata( let existing: Record = {}; if (existsSync(path)) { - existing = parseMetadataFile(readFileSync(path, "utf-8")); + existing = parseKeyValueContent(readFileSync(path, "utf-8")); } // Merge updates — remove keys set to empty string @@ -254,7 +226,7 @@ export function readArchivedMetadataRaw( if (!latest) return null; try { - return parseMetadataFile(readFileSync(join(archiveDir, latest), "utf-8")); + return parseKeyValueContent(readFileSync(join(archiveDir, latest), "utf-8")); } catch { return null; } @@ -284,7 +256,7 @@ export function updateArchivedMetadata( const archivePath = join(archiveDir, latest); let existing: Record; try { - existing = parseMetadataFile(readFileSync(archivePath, "utf-8")); + existing = parseKeyValueContent(readFileSync(archivePath, "utf-8")); } catch { return false; } diff --git a/packages/core/src/paths.ts b/packages/core/src/paths.ts index 12a91d7c9..49d8b3f51 100644 --- a/packages/core/src/paths.ts +++ b/packages/core/src/paths.ts @@ -102,6 +102,14 @@ export function getWorktreesDir(configPath: string, projectPath: string): string return join(getProjectBaseDir(configPath, projectPath), "worktrees"); } +/** + * Get the feedback reports directory for a project. + * Format: ~/.agent-orchestrator/{hash}-{projectId}/feedback-reports + */ +export function getFeedbackReportsDir(configPath: string, projectPath: string): string { + return join(getProjectBaseDir(configPath, projectPath), "feedback-reports"); +} + /** * Get the archive directory for a project. * Format: ~/.agent-orchestrator/{hash}-{projectId}/archive