← Back to PRs

#17715: scaffolds: Phase 1 executor (g-v-p) + gates + manifests

by swmeyer1979 open 2026-02-16 03:36 View on GitHub →
docs agents stale size: XL
## Summary Phase 1 “reasoning scaffolds”: adds manifest-driven **Generate → Verify → Patch** execution for opted-in skills. Key properties: - Executor owns prompts (no app system prompt injection). - Model calls are **tool-less** and **messages[]**-based. - Deterministic local verification (AJV schema + invariant gates) + conservative JSON fence stripping. - Manifest parsing is strict and **fail-closed when executors are enabled**. ## RFC - `docs/scaffolds-rfc.md` ## Activation / behavior - Default behavior is unchanged. - New behavior only activates when: 1) `config.scaffolds.executorsEnabled === true`, **and** 2) the invoked skill has a valid `scaffold.manifest.json` adjacent to `SKILL.md`. - If `executorsEnabled === true` and a manifest is **present-but-invalid** → fail closed with: - `Scaffold error: invalid manifest (E_MANIFEST_INVALID).` ## What’s included - Manifest loading + validation in `src/agents/skills/workspace.ts` - Budgets (`BudgetCounter`) with validation rule `maxLlmCalls >= 1 + maxRetries` - Gate pipeline: - AJV schema validation with normalized, deterministically-sorted failures - invariants: `no_placeholders`, `max_length` - deterministic failure ordering - `g-v-p` executor implementation + tests - Minimal runner wiring in `src/agents/pi-embedded-runner/run/attempt.ts` to route skill-command invocations through the executor when enabled ## Verification - `pnpm lint` - `pnpm check` - `pnpm -s test` <!-- greptile_comment --> <h3>Greptile Summary</h3> Phase 1 scaffolds introducing a manifest-driven **Generate → Verify → Patch** executor for opted-in skills. The implementation adds: - **Manifest-driven activation**: Skills with a valid `scaffold.manifest.json` adjacent to `SKILL.md` are routed through the GVP executor when `config.scaffolds.executorsEnabled === true`. Invalid manifests fail closed. - **GVP executor**: Generates a JSON artifact, validates via AJV schema + invariant gates, and retries with targeted patch prompts up to the budget limit. - **Gate pipeline**: Three-stage deterministic validation (schema → extract → invariants) with sorted failure output. - **Budget enforcement**: `BudgetCounter` with `maxLlmCalls >= 1 + maxRetries` constraint validated at manifest parse time. - **Config/sensitive refactoring**: DRY extraction of sensitive-key patterns into `src/config/sensitive.ts`. Key observations: - Default behavior is preserved; new code paths only activate under explicit opt-in. - All scaffolded LLM calls record **zero token usage** in session messages — usage tracking will under-report for scaffolded executions. - The `callModel` adapter and session message recording blocks in `attempt.ts` are quite verbose (~165 lines) with repeated zero-usage boilerplate that could be extracted. - Unrelated SSRF test additions and search provider doc string update are bundled in the same PR. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge — new behavior only activates under explicit opt-in and default paths are preserved. - The implementation is well-structured with fail-closed semantics, strict manifest parsing, and deterministic validation. The code is gated behind two opt-in conditions (config flag + valid manifest), ensuring no impact on existing behavior. Deductions for: zero token usage tracking in scaffolded executions, verbose boilerplate in attempt.ts, and bundled unrelated changes. No critical logic bugs found beyond what was already flagged in prior review threads. - `src/agents/pi-embedded-runner/run/attempt.ts` — the scaffold integration adds significant branching complexity to an already large file. `src/scaffolds/executors/gvp-executor.ts` — core execution logic. <sub>Last reviewed commit: 65c978d</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs