port: sync from Mike/pygienium@595ce48f
This commit is contained in:
omp-port
204
README.md
204
README.md
@@ -1,207 +1,11 @@
|
||||
# Pygienium
|
||||
# Pygienium (omp port)
|
||||
|
||||
Code hygiene for [omp](https://github.com/oh-my-pi) — isolated
|
||||
sub-agent checks that scan a target, apply fixes, and emit a findings + changes
|
||||
report. Inspired by piolium's sub-agent loops.
|
||||
|
||||
Pygienium runs **highly-structured hygiene passes** over a repo to clean up the
|
||||
common quality issues LLM-generated code accumulates: restating comments,
|
||||
shallow pass-through modules, dead exports/files, and redundant defensive
|
||||
guards. Each check is an isolated, resumable sub-agent run whose progress lands
|
||||
in a single inspectable run-state file.
|
||||
Code hygiene extension for omp — isolated sub-agent checks that scan a target, apply fixes, and emit a findings+changes report.
|
||||
|
||||
## Install
|
||||
|
||||
Pygienium is a local omp extension under `~/.omp/agent/extensions/pygienium/`.
|
||||
Omp loads it via `omp.extensions` in `package.json` (entry `./src/index.ts`).
|
||||
|
||||
```sh
|
||||
# from the extension root
|
||||
bun install # optional: only needed for typecheck/test dev deps
|
||||
bun run typecheck
|
||||
bun test
|
||||
omp install @mikefreno/omp-pygienium
|
||||
```
|
||||
|
||||
Omp auto-discovers the extension from this location via the `omp.extensions` entry in `package.json` — no settings.json config is needed. On load it emits a TUI notification `Pygienium loaded. Run /pygienium-help for available checks and flags.` (only when a dialog-capable UI is available).
|
||||
|
||||
## Configuration
|
||||
|
||||
Pygienium reads its chat-rendering style from omp's `settings.json` (`~/.omp/agent/settings.json`) under a `pygienium` key:
|
||||
|
||||
```json
|
||||
{
|
||||
"pygienium": {
|
||||
"chatStyle": "verbose"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
| Setting | Default | Values | Description |
|
||||
| --- | --- | --- | --- |
|
||||
| `pygienium.chatStyle` | `"verbose"` | `"verbose"` \| `"compact"` | Chat rendering for sub-agent tool calls. **verbose** (piolium-style) streams each tool event live as its own chat line (`[Comments: Scanning] → bash ...` / `← (ok)`). **compact** (ralpi-style) suppresses the per-event stream and shows only the final completion message with its expandable phase tree. |
|
||||
|
||||
No entry means `"verbose"` (the default). An unreadable or missing `settings.json` also falls back to `"verbose"`.
|
||||
|
||||
## Commands
|
||||
|
||||
Every command accepts a `[path]` target (default: the current directory) and is
|
||||
resumable — progress is persisted to `<cwd>/.pygienium/run-state.json`.
|
||||
|
||||
| Command | What it does |
|
||||
| --- | --- |
|
||||
| `/pygienium-help` | Print every command, shipped check, and flag. |
|
||||
| `/pygienium-<check> [path] [--fix]` | Run one isolated sub-agent that scans a target, applies fixes with `--fix`, and emits a findings + changes report. |
|
||||
| `/pygienium-all [path] [--fix]` | Run every registered check in sequence under one resumable run-state. |
|
||||
| `/pygienium-status [path]` | Show per-check progress, artifact line counts, and errors for the latest run. |
|
||||
| `/pygienium-resume [path] [--fresh]` | Resume the latest in-progress/failed/partial run; complete/skipped checks skip unless `--fresh`. |
|
||||
| `/pygienium-export [path] [--check=] [--status=] [--out=md\|json]` | Bundle every check's `findings.md` + `changes.md` into `pygienium/export.{md\|json}`. |
|
||||
|
||||
## Flags
|
||||
|
||||
| Flag | Scope | Description |
|
||||
| --- | --- | --- |
|
||||
| `[path]` | all check commands | Target file or directory to scan (default: current dir). |
|
||||
| `--fix` | `<check>`, `all`, `resume` | Apply fixes (default: scan-only; emits findings only). |
|
||||
| `--fresh` | `resume` | Re-dispatch completed checks too — reset their run-state entries and re-run. |
|
||||
| `--check=` | `export` | Comma-separated check names to include in the bundle. |
|
||||
| `--status=` | `export` | Comma-separated statuses to include (e.g. `complete,failed,skipped`). |
|
||||
| `--out=` | `export` | Bundle format: `md` (default) or `json`. |
|
||||
|
||||
## Checks
|
||||
|
||||
The five shipped checks live in [`src/checks/`](./src/checks/) and self-register
|
||||
on load. `/pygienium-help` lists whichever checks are currently registered, so
|
||||
this table and the live help always agree on the registered set.
|
||||
|
||||
| Command | Check | Agent | What it fixes |
|
||||
| --- | --- | --- | --- |
|
||||
| `/pygienium-comments` | comments | `scanner` / `fixer` | Remove low-value/restating comments, tighten verbose ones, keep "why" comments. |
|
||||
| `/pygienium-deep-modules` | deep-modules | `deep-modules` | Detect shallow modules (pass-throughs, trivial wrappers, re-export barrels) and consolidate the safe ones. |
|
||||
| `/pygienium-dead-code` | dead-code | `scanner` / `fixer` | Find unreferenced exports, dead files, obsolete compat shims, migration helpers, and unused dependencies; remove clearly-dead items and flag dynamic ones. |
|
||||
| `/pygienium-defensive-guards` | defensive-guards | `defensive-guards` | Remove redundant defensive guarding (null checks on non-nullable types, swallowing try/catch, masking fallbacks) while keeping boundary guards (IO, parsing, untrusted input). |
|
||||
| `/pygienium-todos` | todos | `todos` / `fixer` | Inventory TODO/FIXME markers and stub implementations; with `--fix`, convert silent stubs (placeholder returns, empty bodies) into loud failures — never implementing TODOs or deleting markers. |
|
||||
|
||||
## Artifacts
|
||||
|
||||
Every check writes its reports under `<cwd>/.pygienium/checks/<name>/` (run
|
||||
state lives at `<cwd>/.pygienium/run-state.json`):
|
||||
|
||||
- `findings.md` — what the scan found (per-file line refs).
|
||||
- `changes.md` — what the fix phase changed + anything deferred for human review.
|
||||
|
||||
`/pygienium-export` merges every check's artifacts into one
|
||||
`.pygienium/export.md` (or `export.json`).
|
||||
|
||||
On first run in a git work tree, pygienium appends `.pygienium/` to the
|
||||
target repo's `.gitignore` so a run never stages its own state/artifacts into
|
||||
git (opt out with `--no-gitignore`).
|
||||
|
||||
## Adding a check
|
||||
|
||||
One file + one `registerCheck()` call. **No `index.ts` command-wiring changes.**
|
||||
`index.ts` auto-discovers every `checks/*.ts` (except the registry barrel) at
|
||||
startup, so a new file self-registers and `/pygienium-<name>` appears
|
||||
automatically.
|
||||
|
||||
1. Create `src/checks/<name>.ts` from the template below.
|
||||
2. Edit the `name`, `label`, `description`, the rubric in the scan/fix task
|
||||
builders, and the `gate` precondition.
|
||||
3. Keep the trailing `registerCheck(<name>Check)`. Done.
|
||||
|
||||
```ts
|
||||
import { registerCheck, type CheckScope } from "./registry.js";
|
||||
|
||||
export const myCheck = {
|
||||
name: "my-check",
|
||||
label: "My check",
|
||||
description: "What it fixes (shown in /pygienium-help).",
|
||||
agentName: "scanner", // reuse a shipped agent, or add agents/<name>.md
|
||||
fixAgentName: "fixer",
|
||||
phaseId: "my-check",
|
||||
buildScanTask: (_cwd: string, scope: CheckScope) => `# Task: my-check scan\n…`,
|
||||
buildFixTask: (_cwd: string, scope: CheckScope, findings: string) => `# Task: my-check fix\n…`,
|
||||
gate: (cwd: string) => undefined,
|
||||
} as const;
|
||||
|
||||
registerCheck(myCheck);
|
||||
```
|
||||
|
||||
Reload omp (or `/reload`) and run `/pygienium-help` — `/pygienium-my-check` is
|
||||
listed and runnable. A `CheckDefinition` supplies the task builders and gate;
|
||||
the generic check-runner wires the phases together, so a new check never
|
||||
touches command plumbing.
|
||||
|
||||
## Architecture
|
||||
|
||||
Each check is a "mode" running a fixed phase pipeline:
|
||||
|
||||
```
|
||||
registerCheck(def) ← checks/*.ts self-register on load
|
||||
│
|
||||
▼
|
||||
/pygienium-<check> ─► runCheck(def) (src/modes/check-runner.ts)
|
||||
│
|
||||
├─ Q0 recon (shared, run once per run) (src/recon.ts)
|
||||
│ git state + source-file inventory → .pygienium/recon.json
|
||||
├─ analysis sub-agent (buildScanTask) ← scanner/<check> agent
|
||||
│ writes .pygienium/checks/<name>/findings.md
|
||||
├─ fix sub-agent (buildFixTask) ← fixer, only with --fix
|
||||
│ writes .pygienium/checks/<name>/changes.md
|
||||
├─ verify gate (re-runs check.gate)
|
||||
└─ cleanup (drops transient scratch artifacts)
|
||||
```
|
||||
|
||||
- **Sub-agents** are isolated in-memory `AgentSession`s scoped to the target
|
||||
`cwd` (see `src/agent-runner.ts`), with the agent definition's system prompt
|
||||
and tool allowlist applied. Agent definitions are plain editable markdown in
|
||||
[`agents/`](./agents/) (frontmatter `name` + `allowedTools`, body = system
|
||||
prompt) — tuning a sub-agent never needs TypeScript changes. A scanned
|
||||
project can ship its own `agents/*.md` at its root: those are loaded as
|
||||
overrides (repo agent wins on name collision), so teams can tune prompts or
|
||||
add project-specific agents without touching the extension.
|
||||
- **Run-state** is a single JSON file at `<cwd>/.pygienium/run-state.json`
|
||||
(`src/run-state.ts`): per-check phase progress, captured findings/changes
|
||||
text, and recon status. `/pygienium-status`, `/pygienium-resume`, and
|
||||
`/pygienium-export` are pure reads over it; `/pygienium-all` shares one
|
||||
`RunState` across every check so phases accumulate in one record.
|
||||
- **The registry** (`src/checks/registry.ts`) is the extensibility seam: a
|
||||
module-level `Map` of `CheckDefinition`s. `index.ts` iterates it and binds
|
||||
one `/pygienium-<name>` command per entry, so adding a check is a file +
|
||||
one `registerCheck()` line.
|
||||
- **The footer** (`src/footer.ts`) is the piolium-style pipeline-overview
|
||||
status strip: a single static line in the TUI footer (via
|
||||
`ui.setStatus(key, text)`) listing the full ordered pipeline with the cursor
|
||||
on the current phase and what's to come. For a single check the items are
|
||||
the phases; for `/pygienium-all` they're the checks (the full todo list),
|
||||
and the per-check footer is suppressed so two overviews never compete over
|
||||
the same status slot. The chat widget (`phases.ts`) remains the animated
|
||||
detail view (spinner + tool-call tree + completion tree); the footer is the
|
||||
overview — the two never overlap. In print/JSON mode the footer is a no-op.
|
||||
|
||||
## Layout
|
||||
|
||||
```
|
||||
pygienium/
|
||||
├─ src/
|
||||
│ ├─ index.ts ← entry: auto-discover checks, bind commands
|
||||
│ ├─ commands.ts ← slash-command handlers (thin binders)
|
||||
│ ├─ help.ts ← COMMANDS + CLI_FLAGS → /pygienium-help output
|
||||
│ ├─ agent-runner.ts ← isolated sub-agent sessions (injectable for tests)
|
||||
│ ├─ agents.ts ← markdown agent-definition loader
|
||||
│ ├─ recon.ts ← shared Q0 reconnaissance snapshot
|
||||
│ ├─ run-state.ts ← persistent, resumable run-state model
|
||||
│ ├─ status.ts ← /pygienium-status formatter (pure)
|
||||
│ ├─ export.ts ← /pygienium-export gatherer + md/json renderer
|
||||
│ ├─ phases.ts ← live chat progress widget + completion-tree helpers
|
||||
│ ├─ footer.ts ← pipeline-overview status strip (TUI footer)
|
||||
│ ├─ modes/check-runner.ts ← the per-check phase pipeline
|
||||
│ └─ checks/ ← one file per check, self-registering
|
||||
│ ├─ registry.ts ← CheckDefinition + registerCheck
|
||||
│ ├─ comments.ts deep-modules.ts dead-code.ts
|
||||
│ ├─ defensive-guards.ts
|
||||
└─ agents/ ← scanner.md fixer.md deep-modules.md defensive-guards.md
|
||||
```
|
||||
|
||||
## License
|
||||
|
||||
MIT
|
||||
This is the omp port of [Mike/pygienium](https://git.freno.me/Mike/pygienium), regenerated automatically from the source repo. See the source repo for full documentation.
|
||||
|
||||
@@ -17,9 +17,32 @@
|
||||
|
||||
import { mkdir, writeFile } from "node:fs/promises";
|
||||
import { dirname, isAbsolute, join } from "node:path";
|
||||
import type { AgentSessionEvent } from "@oh-my-pi/pi-coding-agent";
|
||||
import type {
|
||||
AgentSession,
|
||||
AgentSessionEvent,
|
||||
} from "@oh-my-pi/pi-coding-agent";
|
||||
import { loadAgents, extensionRoot, type AgentDef } from "./agents.js";
|
||||
|
||||
/**
|
||||
* Env var overriding the per-agent settle timeout (ms). Guards against a
|
||||
* sub-agent session that never settles (stalled provider stream, hung retry /
|
||||
* auto-compaction after the final tool call), which otherwise leaves the run
|
||||
* stuck mid-phase with no state save, no error, and no completion message.
|
||||
*/
|
||||
export const AGENT_TIMEOUT_ENV = "PYGIENIUM_AGENT_TIMEOUT_MS";
|
||||
|
||||
/** Default settle timeout per agent phase: 60 minutes. */
|
||||
const DEFAULT_AGENT_TIMEOUT_MS = 60 * 60_000;
|
||||
|
||||
/** Resolve the per-agent settle timeout, honouring the env override. */
|
||||
export function agentTimeoutMs(): number {
|
||||
const raw = process.env[AGENT_TIMEOUT_ENV];
|
||||
if (raw && /^\d+$/.test(raw.trim()) && Number(raw.trim()) > 0) {
|
||||
return Number(raw.trim());
|
||||
}
|
||||
return DEFAULT_AGENT_TIMEOUT_MS;
|
||||
}
|
||||
|
||||
export interface AgentTaskOptions {
|
||||
/** Absolute working directory for the sub-agent. */
|
||||
cwd: string;
|
||||
@@ -37,6 +60,62 @@ export interface AgentTaskOptions {
|
||||
* turns tool_execution_start/end + assistant turns into chat messages.
|
||||
*/
|
||||
onEvent?: (event: AgentSessionEvent) => void;
|
||||
/**
|
||||
* Maximum time the agent run may take before it is aborted and the phase
|
||||
* fails loudly (default {@link agentTimeoutMs}). A session that never
|
||||
* settles — stalled provider stream, hung retry/compaction after its last
|
||||
* tool call — would otherwise hang the check run silently with no state
|
||||
* update and no completion message.
|
||||
*/
|
||||
timeoutMs?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Race `promise` against a settle deadline. Returns `{ value }` on success or
|
||||
* `{ error }` when the deadline elapsed first (the caller aborts the work).
|
||||
* `promise` is still awaited-then-ignored afterwards so late rejections can
|
||||
* never surface as unhandled.
|
||||
*/
|
||||
export async function withSettleTimeout<T>(
|
||||
promise: Promise<T>,
|
||||
timeoutMs: number,
|
||||
label: string,
|
||||
): Promise<{ value: T } | { error: string }> {
|
||||
if (timeoutMs <= 0) {
|
||||
try {
|
||||
return { value: await promise };
|
||||
} catch (err) {
|
||||
return { error: err instanceof Error ? err.message : String(err) };
|
||||
}
|
||||
}
|
||||
const settled = promise.then(
|
||||
(value) => ({ value }) as { value: T },
|
||||
(err) =>
|
||||
({ error: err instanceof Error ? err.message : String(err) }) as {
|
||||
error: string;
|
||||
},
|
||||
);
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
const deadline = new Promise<{ error: string }>((resolve) => {
|
||||
timer = setTimeout(() => {
|
||||
// ~`1m 30s` / `45s` for the message (timeoutMs is ms; tests use small values).
|
||||
const totalSec = Math.round(timeoutMs / 1000);
|
||||
const duration =
|
||||
totalSec >= 60
|
||||
? `${Math.floor(totalSec / 60)}m${totalSec % 60 ? ` ${totalSec % 60}s` : ""}`
|
||||
: `${totalSec}s`;
|
||||
resolve({
|
||||
error: `${label} did not settle within ${duration}; aborted. Check model/provider connectivity, then resume with /pygienium-resume.`,
|
||||
});
|
||||
}, timeoutMs);
|
||||
// Never hold the process open just because a deadline is pending.
|
||||
timer.unref?.();
|
||||
});
|
||||
try {
|
||||
return await Promise.race([settled, deadline]);
|
||||
} finally {
|
||||
if (timer) clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
|
||||
export interface AgentRunResult {
|
||||
@@ -54,8 +133,21 @@ export type AgentRunner = (opts: AgentTaskOptions) => Promise<AgentRunResult>;
|
||||
let currentRunner: AgentRunner = defaultAgentRunner;
|
||||
|
||||
/** Entry point used by the check-runner. */
|
||||
export function runAgentTask(opts: AgentTaskOptions): Promise<AgentRunResult> {
|
||||
return currentRunner(opts);
|
||||
export async function runAgentTask(
|
||||
opts: AgentTaskOptions,
|
||||
): Promise<AgentRunResult> {
|
||||
// Backstop: even a third-party/custom runner must not be able to hang the
|
||||
// run forever. The default runner additionally aborts its session on
|
||||
// timeout (see defaultAgentRunner); this race covers every other runner.
|
||||
const settled = await withSettleTimeout(
|
||||
currentRunner(opts),
|
||||
opts.timeoutMs ?? agentTimeoutMs(),
|
||||
`sub-agent "${opts.agentName}"`,
|
||||
);
|
||||
if ("error" in settled) {
|
||||
return { ok: false, text: "", error: settled.error };
|
||||
}
|
||||
return settled.value;
|
||||
}
|
||||
|
||||
/** Override the active agent runner (primarily for tests). */
|
||||
@@ -124,43 +216,40 @@ export async function defaultAgentRunner(
|
||||
agentRegistry: new AgentRegistry(),
|
||||
});
|
||||
|
||||
// A session that never settles (stalled provider stream, hung retry /
|
||||
// auto-compaction after its last tool call) must fail the phase loudly
|
||||
// instead of hanging the run mid-transition with zero diagnostics. On
|
||||
// timeout the session is disposed, which aborts the in-flight run.
|
||||
const settled = await withSettleTimeout(
|
||||
runSessionToCompletion(session, opts),
|
||||
opts.timeoutMs ?? agentTimeoutMs(),
|
||||
`sub-agent "${opts.agentName}"`,
|
||||
);
|
||||
if ("error" in settled) {
|
||||
// Abort the still-running session so it can't keep burning provider
|
||||
// calls; the background settle path then finishes and disposes too.
|
||||
try {
|
||||
session.dispose();
|
||||
} catch {
|
||||
/* ignore dispose errors */
|
||||
}
|
||||
return { ok: false, text: "", error: settled.error };
|
||||
}
|
||||
return settled.value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Run an in-memory agent session to completion, streaming tool events to the
|
||||
* chat forwarder, and return the agent's final text. Owns session cleanup.
|
||||
*/
|
||||
async function runSessionToCompletion(
|
||||
session: AgentSession,
|
||||
opts: AgentTaskOptions,
|
||||
): Promise<AgentRunResult> {
|
||||
const acc: SessionEventAccumulator = { text: "" };
|
||||
try {
|
||||
let text = "";
|
||||
let stopReason: string | undefined;
|
||||
let errorMessage: string | undefined;
|
||||
const unsubscribe = session.subscribe((event: AgentSessionEvent) => {
|
||||
if (
|
||||
event.type === "message_update" &&
|
||||
event.assistantMessageEvent.type === "text_delta"
|
||||
) {
|
||||
text += event.assistantMessageEvent.delta;
|
||||
}
|
||||
if (event.type === "message_end") {
|
||||
// Capture the full assistant text from the finalized message —
|
||||
// models that don't stream text_delta (or truncate) still surface
|
||||
// their output here. Prefer the streamed text when non-empty.
|
||||
const message = event.message as {
|
||||
role?: string;
|
||||
content?: unknown;
|
||||
stopReason?: string;
|
||||
errorMessage?: string;
|
||||
};
|
||||
if (message.stopReason) stopReason = message.stopReason;
|
||||
if (message.errorMessage) errorMessage = message.errorMessage;
|
||||
if (message.role === "assistant") {
|
||||
const full = extractAssistantText(message.content).trim();
|
||||
if (full && !text.trim()) text = full;
|
||||
}
|
||||
}
|
||||
// Forward the stream-driving events to the chat forwarder; it turns
|
||||
// each into its own `pygienium-stream` message (see index.ts).
|
||||
if (
|
||||
event.type === "tool_execution_start" ||
|
||||
event.type === "tool_execution_end" ||
|
||||
event.type === "message_end"
|
||||
) {
|
||||
opts.onEvent?.(event);
|
||||
}
|
||||
applySessionEvent(acc, event, opts.onEvent);
|
||||
});
|
||||
await session.prompt(opts.task, { expandPromptTemplates: false });
|
||||
// Ensure the agent has fully settled (tool calls may still be in-flight
|
||||
@@ -171,17 +260,17 @@ export async function defaultAgentRunner(
|
||||
// Surface session errors that didn't throw but left no useful output.
|
||||
// A session ending with stopReason "error" and no text means the model
|
||||
// call failed silently — treat that as a failed run, not ok:true.
|
||||
if (errorMessage) {
|
||||
return { ok: false, text, error: errorMessage };
|
||||
if (acc.errorMessage) {
|
||||
return { ok: false, text: acc.text, error: acc.errorMessage };
|
||||
}
|
||||
if (!text.trim() && stopReason === "error") {
|
||||
if (!acc.text.trim() && acc.stopReason === "error") {
|
||||
return {
|
||||
ok: false,
|
||||
text,
|
||||
text: acc.text,
|
||||
error: "sub-agent session ended in error with no output.",
|
||||
};
|
||||
}
|
||||
return { ok: true, text };
|
||||
return { ok: true, text: acc.text };
|
||||
} catch (err) {
|
||||
return {
|
||||
ok: false,
|
||||
@@ -197,6 +286,83 @@ export async function defaultAgentRunner(
|
||||
}
|
||||
}
|
||||
|
||||
/** Running capture state while a session streams assistant output. */
|
||||
export interface SessionEventAccumulator {
|
||||
/** Joined assistant text seen so far (text_delta stream). */
|
||||
text: string;
|
||||
/** stopReason of the final assistant message, when reported. */
|
||||
stopReason?: string;
|
||||
/** errorMessage of the final assistant message, when reported. */
|
||||
errorMessage?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Interpret one session event into the running accumulator and forward the
|
||||
* stream-driving events to the chat.
|
||||
*
|
||||
* MUST never throw: it runs synchronously inside the SDK's event pipeline
|
||||
* (`session.subscribe` listeners are invoked from `processEvents`/`_emit`).
|
||||
* A throw there is not contained — the SDK's run-failure path re-emits
|
||||
* failure events through the same callback, so a callback that always throws
|
||||
* recurses until stack overflow and crashes the host, stranding run-state at
|
||||
* the phase boundary with no save and no completion. Guard every shape (the
|
||||
* runtime events are partial/streaming and weaker than their types) and never
|
||||
* let the cosmetic chat forwarder take the run down.
|
||||
*/
|
||||
export function applySessionEvent(
|
||||
acc: SessionEventAccumulator,
|
||||
event: AgentSessionEvent,
|
||||
forward?: (event: AgentSessionEvent) => void,
|
||||
): void {
|
||||
if (!event) return;
|
||||
try {
|
||||
if (
|
||||
event.type === "message_update" &&
|
||||
event.assistantMessageEvent?.type === "text_delta"
|
||||
) {
|
||||
acc.text += event.assistantMessageEvent.delta ?? "";
|
||||
}
|
||||
if (event.type === "message_end") {
|
||||
// Capture the full assistant text from the finalized message —
|
||||
// models that don't stream text_delta (or truncate) still surface
|
||||
// their output here. Prefer the streamed text when non-empty.
|
||||
const message = event.message as
|
||||
| {
|
||||
role?: string;
|
||||
content?: unknown;
|
||||
stopReason?: string;
|
||||
errorMessage?: string;
|
||||
}
|
||||
| undefined;
|
||||
if (message) {
|
||||
if (message.stopReason) acc.stopReason = message.stopReason;
|
||||
if (message.errorMessage) acc.errorMessage = message.errorMessage;
|
||||
if (message.role === "assistant") {
|
||||
const full = extractAssistantText(message.content).trim();
|
||||
if (full && !acc.text.trim()) acc.text = full;
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
// A malformed event must not crash the SDK event pipeline; log and skip.
|
||||
console.error("pygienium: error processing sub-agent event", err);
|
||||
}
|
||||
// Forward the stream-driving events to the chat forwarder; it turns each
|
||||
// into its own `pygienium-stream` message (see index.ts). Cosmetic chat
|
||||
// UI: a broken forwarder must not fail or hang the agent run.
|
||||
if (
|
||||
event.type === "tool_execution_start" ||
|
||||
event.type === "tool_execution_end" ||
|
||||
event.type === "message_end"
|
||||
) {
|
||||
try {
|
||||
forward?.(event);
|
||||
} catch (err) {
|
||||
console.error("pygienium: error forwarding sub-agent event", err);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract joined text from an assistant message's content blocks.
|
||||
* Mirrors piolium's `extractAssistantText`.
|
||||
|
||||
@@ -92,8 +92,11 @@ ${scopeRulesMarkdown()}
|
||||
3. Apply the rubric below to each comment and classify it: RESTATE, VERBOSE,
|
||||
WHY, or OK.
|
||||
4. Write a findings report to \`${findingsFile}\` with per-file line refs.
|
||||
5. Return the findings report text as your final message (same content as the
|
||||
file). The host captures it as the analysis-phase findings.
|
||||
5. Return a ONE-LINE summary as your final message, e.g.
|
||||
\`<count> comment smell(s) across <files> file(s).\` The full report lives
|
||||
in findings.md — do NOT regenerate the report text in your final message
|
||||
(regenerating a large report doubles the fragile output right after the
|
||||
file write and can stall the run).
|
||||
|
||||
${RUBRIC}
|
||||
|
||||
@@ -110,7 +113,8 @@ ${RUBRIC}
|
||||
\`\`\`
|
||||
|
||||
If no smells are found, write \`# comments — findings\n\n0 comment smell(s).\`
|
||||
and return that text. Always create findings.md so the run has an artifact.
|
||||
and return \`0 comment smell(s).\` as your final message. Always create
|
||||
findings.md so the run has an artifact.
|
||||
|
||||
Write the report under \`${outDir}\` (create directories as needed).
|
||||
`;
|
||||
@@ -128,6 +132,7 @@ export function buildCommentsFixTask(
|
||||
): string {
|
||||
const outDir = commentsArtifactDir(scope);
|
||||
const changesFile = changesPath(scope);
|
||||
const findingsFile = findingsPath(scope);
|
||||
return `# Task: comments hygiene fix
|
||||
|
||||
You are running the **comments** hygiene fix phase.
|
||||
@@ -136,6 +141,9 @@ You are running the **comments** hygiene fix phase.
|
||||
- Fix target: \`${scope.target}\`
|
||||
|
||||
## Input: scan findings
|
||||
Read the detailed per-file findings from \`${findingsFile}\` (created by the
|
||||
scan phase). If that file is missing or unreadable, fall back to the findings
|
||||
text below:
|
||||
${findings.trim().length > 0 ? findings : "(no findings text provided)"}
|
||||
|
||||
## What to do
|
||||
|
||||
@@ -142,7 +142,12 @@ export async function runCheck(
|
||||
): Promise<CheckRunOutcome> {
|
||||
const startMs = Date.now();
|
||||
const outcome = await runCheckImpl(opts);
|
||||
postCheckCompletion(opts, outcome, Date.now() - startMs);
|
||||
try {
|
||||
postCheckCompletion(opts, outcome, Date.now() - startMs);
|
||||
} catch {
|
||||
// Completion posting is best-effort chat UI: a renderer or send
|
||||
// failure must never reject the run after its state was persisted.
|
||||
}
|
||||
return outcome;
|
||||
}
|
||||
|
||||
@@ -383,7 +388,16 @@ async function runCheckImplInner(
|
||||
error = err instanceof Error ? err.message : String(err);
|
||||
markCheckStatus(state, check.name, "failed", error);
|
||||
markRunStatus(state, reconcileRunStatus(state));
|
||||
await saveRunState(state);
|
||||
try {
|
||||
await saveRunState(state);
|
||||
} catch (saveErr) {
|
||||
// A failing state save inside the error path must not mask the
|
||||
// original error or escape as an unhandled rejection (which would
|
||||
// kill the run with nothing persisted or reported).
|
||||
error += `; (also failed to persist run-state: ${
|
||||
saveErr instanceof Error ? saveErr.message : String(saveErr)
|
||||
})`;
|
||||
}
|
||||
return { status: "failed", error, findings, changes, state };
|
||||
} finally {
|
||||
strip.done();
|
||||
|
||||
137
tests/agent-runner.test.ts
Normal file
137
tests/agent-runner.test.ts
Normal file
@@ -0,0 +1,137 @@
|
||||
/**
|
||||
* agent-runner.test.ts — unit tests for the session event accumulator.
|
||||
*
|
||||
* `applySessionEvent` runs synchronously inside the SDK's event pipeline; a
|
||||
* throw there crashes the host (the SDK's run-failure path re-emits events
|
||||
* through the same callback → stack overflow), stranding run-state at the
|
||||
* phase boundary. These tests pin the handler to never throw on the partial /
|
||||
* malformed event shapes streaming sessions actually emit.
|
||||
*/
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import {
|
||||
applySessionEvent,
|
||||
type SessionEventAccumulator,
|
||||
} from "../src/agent-runner.js";
|
||||
import type { AgentSessionEvent } from "@oh-my-pi/pi-coding-agent";
|
||||
|
||||
function fresh(): SessionEventAccumulator {
|
||||
return { text: "" };
|
||||
}
|
||||
|
||||
/** Build a typed-as-unknown event so malformed shapes compile in tests. */
|
||||
function event(shape: unknown): AgentSessionEvent {
|
||||
return shape as AgentSessionEvent;
|
||||
}
|
||||
|
||||
describe("applySessionEvent", () => {
|
||||
it("accumulates text_delta stream events in order", () => {
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "foo" },
|
||||
}),
|
||||
);
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "bar" },
|
||||
}),
|
||||
);
|
||||
expect(acc.text).toBe("foobar");
|
||||
});
|
||||
|
||||
it("does not throw on a message_update with no assistantMessageEvent", () => {
|
||||
const acc = fresh();
|
||||
expect(() =>
|
||||
applySessionEvent(acc, event({ type: "message_update" })),
|
||||
).not.toThrow();
|
||||
expect(acc.text).toBe("");
|
||||
});
|
||||
|
||||
it("does not throw on a message_update with an unknown event shape", () => {
|
||||
const acc = fresh();
|
||||
expect(() => applySessionEvent(acc, event({ type: "bogus_event" }))).not.toThrow();
|
||||
expect(() => applySessionEvent(acc, event(null))).not.toThrow();
|
||||
expect(() => applySessionEvent(acc, event(undefined))).not.toThrow();
|
||||
expect(acc.text).toBe("");
|
||||
});
|
||||
|
||||
it("captures full text + stopReason from message_end when nothing streamed", () => {
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({
|
||||
type: "message_end",
|
||||
message: {
|
||||
role: "assistant",
|
||||
stopReason: "stop",
|
||||
content: [{ type: "text", text: "full report" }],
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect(acc.text).toBe("full report");
|
||||
expect(acc.stopReason).toBe("stop");
|
||||
});
|
||||
|
||||
it("does not throw on a message_end with no message", () => {
|
||||
const acc = fresh();
|
||||
expect(() => applySessionEvent(acc, event({ type: "message_end" }))).not.toThrow();
|
||||
expect(acc.text).toBe("");
|
||||
});
|
||||
|
||||
it("records an errorMessage surfaced on the final message", () => {
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({
|
||||
type: "message_end",
|
||||
message: { role: "assistant", errorMessage: "upstream 529" },
|
||||
}),
|
||||
);
|
||||
expect(acc.errorMessage).toBe("upstream 529");
|
||||
});
|
||||
|
||||
it("forwards stream-driving events and swallows a throwing forwarder", () => {
|
||||
const originalError = console.error;
|
||||
console.error = () => {};
|
||||
try {
|
||||
const forwarded: string[] = [];
|
||||
const forward = (ev: AgentSessionEvent) => {
|
||||
forwarded.push(ev.type);
|
||||
if (ev.type === "tool_execution_end") throw new Error("renderer boom");
|
||||
};
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({ type: "tool_execution_start", toolName: "bash" }),
|
||||
forward,
|
||||
);
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({ type: "tool_execution_end", toolName: "bash" }),
|
||||
forward,
|
||||
);
|
||||
expect(forwarded).toEqual(["tool_execution_start", "tool_execution_end"]);
|
||||
} finally {
|
||||
console.error = originalError;
|
||||
}
|
||||
});
|
||||
|
||||
it("does not forward non-stream-driving events", () => {
|
||||
const forwarded: string[] = [];
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({ type: "message_update", assistantMessageEvent: { type: "text_delta", delta: "x" } }),
|
||||
(ev) => forwarded.push(ev.type),
|
||||
);
|
||||
applySessionEvent(acc, event({ type: "agent_settled" }), (ev) =>
|
||||
forwarded.push(ev.type),
|
||||
);
|
||||
expect(forwarded).toEqual([]);
|
||||
expect(acc.text).toBe("x");
|
||||
});
|
||||
});
|
||||
@@ -20,6 +20,8 @@ import {
|
||||
setAgentRunner,
|
||||
resetAgentRunner,
|
||||
fakeAgentRunner,
|
||||
AGENT_TIMEOUT_ENV,
|
||||
type AgentRunResult,
|
||||
} from "../src/agent-runner.js";
|
||||
import { handleCheckCommand, type PygieniumCtx } from "../src/commands.js";
|
||||
import { loadRunState, runStatePath } from "../src/run-state.js";
|
||||
@@ -128,4 +130,29 @@ describe("check-runner integration", () => {
|
||||
expect(state?.checks.gated.status).toBe("skipped");
|
||||
expect(state?.checks.gated.error).toBe("no source files matched");
|
||||
});
|
||||
|
||||
it("fails the check loudly when the agent never settles", async () => {
|
||||
// Models the silent hang: the sub-agent session never resolves after
|
||||
// its last tool call (stalled provider stream / hung retry), leaving
|
||||
// the run stuck mid-phase with no state save and no completion. The
|
||||
// watchdog must abort the phase with a visible error instead.
|
||||
setAgentRunner(() => new Promise<AgentRunResult>(() => {}));
|
||||
const prev = process.env[AGENT_TIMEOUT_ENV];
|
||||
process.env[AGENT_TIMEOUT_ENV] = "50";
|
||||
try {
|
||||
await handleCheckCommand(smokeCheck(), "", stubCtx(cwd));
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env[AGENT_TIMEOUT_ENV];
|
||||
else process.env[AGENT_TIMEOUT_ENV] = prev;
|
||||
}
|
||||
|
||||
const state = await loadRunState(cwd);
|
||||
expect(state?.checks.smoke.status).toBe("failed");
|
||||
expect(state?.checks.smoke.error).toContain("did not settle within");
|
||||
expect(state?.checks.smoke.error).toContain("/pygienium-resume");
|
||||
expect(
|
||||
state?.checks.smoke.phases.find((p) => p.id === "analysis")?.status,
|
||||
).toBe("failed");
|
||||
expect(state?.status).toBe("failed");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -30,6 +30,8 @@ import {
|
||||
commentsCheck,
|
||||
findingsPath,
|
||||
changesPath,
|
||||
buildCommentsScanTask,
|
||||
buildCommentsFixTask,
|
||||
} from "../src/checks/comments.js";
|
||||
import type { CheckScope } from "../src/checks/registry.js";
|
||||
|
||||
@@ -256,4 +258,31 @@ describe("comments check (end-to-end)", () => {
|
||||
expect(check!.findings).toBeDefined();
|
||||
expect(check!.changes).toBeDefined();
|
||||
});
|
||||
|
||||
it("scan task keeps the full report in findings.md, not the final message", () => {
|
||||
// Regression: the scan task used to demand the agent regenerate the
|
||||
// whole report as its final message right after writing findings.md —
|
||||
// a second huge output that stalled the phase transition (observed in
|
||||
// freno-dev twice). The final message must stay a one-line summary.
|
||||
const scope: CheckScope = {
|
||||
cwd,
|
||||
target,
|
||||
fix: false,
|
||||
rest: [],
|
||||
};
|
||||
const task = buildCommentsScanTask(cwd, scope);
|
||||
expect(task).toContain("ONE-LINE summary");
|
||||
expect(task).toContain("do NOT regenerate the report text");
|
||||
expect(task).not.toContain(
|
||||
"Return the findings report text as your final message",
|
||||
);
|
||||
expect(task).not.toContain("same content as the file");
|
||||
|
||||
// The fix phase must read the detailed findings from the artifact so a
|
||||
// one-line scan summary can't starve it.
|
||||
const fixTask = buildCommentsFixTask(cwd, scope, "fallback-findings-text");
|
||||
expect(fixTask).toContain("Read the detailed per-file findings");
|
||||
expect(fixTask).toContain(findingsPath(scope));
|
||||
expect(fixTask).toContain("fallback-findings-text");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user