Held for review: agent placeholder substitution, reviewer recapture contract, base-directory script form (#544)

* Resolve {{scripts_path}} in the agent bodies Codex ships

Three code paths emit an agent body: the degraded fallback reference, the
.toml nested inside the skill for Codex, and the native agent file. Only the
nested .toml skipped placeholder substitution and rule-marker stripping, so
the codex and .agents dists shipped `node {{scripts_path}}/embed-prompt.mjs`
verbatim in the asset producer, and every caller had to substitute the token
itself at load time.

All three now render through renderAgentBody(), and the new regression test
asserts a runnable embed-prompt command on each emitted surface plus a
synthetic agent proving markers and placeholders resolve in the nested .toml.

Prepared by an AI agent (Claude Code) under pbakaus's instruction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Give the finish reviewer's screenshots one fixed address

The Input Contract asked for "desktop and mobile screenshot paths captured by
the parent" and named none, so each session invented a filename and the
verdict pass went looking for a recapture that was never written there. Two
reviewer passes burned on that in the eval runs.

The parent now captures and recaptures to .impeccable/review/desktop.png and
.impeccable/review/mobile.png, and the reviewer reads those two first,
treating a brief-named path as the fallback for a parent that wrote elsewhere.

Prepared by an AI agent (Claude Code) under pbakaus's instruction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Lead Setup with the base directory the runtime reports

The rendered claude and codex skills opened with
`node .claude/skills/impeccable/scripts/context.mjs`, a project-relative path
that resolves in this repo and in nothing a user installs: a personal or
plugin install puts the scripts outside the project entirely. The working form
was already in the text, parenthesized, after the one that fails.

Setup now leads with `node <skill-base-dir>/scripts/context.mjs` and says once
that the base directory resolves every scripts-path command in the skill and
its references, leaving the project-relative path as the fallback for runtimes
that report no base directory.

Prepared by an AI agent (Claude Code) under pbakaus's instruction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Answer the Copilot review: brittle model assertion, missing review dir

Assert that {{model}} resolved rather than that it resolved to "GPT", which
belongs to PROVIDER_PLACEHOLDERS and can change without touching what the test
guards. And have the parent create .impeccable/review/ when the harness does
not, so a fresh project's first capture has somewhere to land.

Prepared by an AI agent (Claude Code) under pbakaus's instruction.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Make the review-screenshot contract directory-based, not web-viewport-named

Two amendments to the recapture contract from review feedback:

1. The canonical location is the directory .impeccable/review/, one file
   per captured viewport; desktop.png and mobile.png are the web case,
   not the contract. Baking web-viewport names into the reviewer's spec
   would have hardened a web assumption into paths that a native
   (ios/android/adaptive) build cannot honestly write.

2. Precedence restored to explicit-beats-convention: paths the calling
   brief names are authoritative when the files exist; the canonical
   directory is where the reviewer looks when the brief names none or a
   named path is missing. This avoids stale canonical files from an
   earlier run silently winning over fresh explicit paths. The observed
   failure (the verdict round inventing a round-stamped filename) stays
   fixed: recapture happens over the same files, and invented filenames
   are still called out as pointing at nothing.

Assisted-by: Claude Code

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
2026-08-08 19:26:43 -07:00
committed by GitHub
co-authored by Claude Fable 5
parent 5c8652b019
commit 045865918a
5 changed files with 107 additions and 14 deletions
+19 -10
View File
@@ -176,6 +176,22 @@ function buildCursorAgent(agent, body) {
return `${generateYamlFrontmatter(frontmatter)}\n${body.trim()}\n`;
}
/**
* Render an agent's markdown body for one provider.
*
* Every surface that ships an agent body (the degraded fallback reference, the
* Codex .toml nested inside the skill, and the native agent file) goes through
* here, so all three resolve provider blocks, {{placeholders}}, rule markers,
* and {{scripts_path}} the same way. The nested Codex .toml used to skip the
* last two and shipped `node {{scripts_path}}/embed-prompt.mjs` literally.
*/
function renderAgentBody(agent, { providerTags, placeholderKey, allSkillNames, scriptsPath }) {
let body = compileProviderBlocks(agent.body, providerTags);
body = replacePlaceholders(body, placeholderKey, [], allSkillNames);
body = stripRuleMarkers(body);
return body.replace(/\{\{scripts_path\}\}/g, scriptsPath);
}
function buildAgentFile(config, agent, body) {
if (config.agentFormat === 'codex-toml') {
return {
@@ -330,10 +346,7 @@ export function createTransformer(config) {
ensureDir(degradedDir);
for (const agent of skill.agents) {
const role = agent.name.replace(/^impeccable-/, '');
let body = compileProviderBlocks(agent.body, providerTags);
body = replacePlaceholders(body, placeholderKey, [], allSkillNames);
body = stripRuleMarkers(body);
body = body.replace(/\{\{scripts_path\}\}/g, scriptsPath);
const body = renderAgentBody(agent, { providerTags, placeholderKey, allSkillNames, scriptsPath });
const content = `${DEGRADED_PREAMBLE}\n\n${body.replace(/^\s+/, '')}`;
writeFile(path.join(degradedDir, `${role}.md`), content);
refCount++;
@@ -358,8 +371,7 @@ export function createTransformer(config) {
if (CODEX_SKILL_PROVIDERS.has(provider)) {
for (const agent of skill.agents || []) {
if (agent.providers && !agent.providers.includes('codex')) continue;
let agentBody = compileProviderBlocks(agent.body, providerTags);
agentBody = replacePlaceholders(agentBody, placeholderKey, [], allSkillNames);
const agentBody = renderAgentBody(agent, { providerTags, placeholderKey, allSkillNames, scriptsPath });
const filename = `${agent.codexName || agent.name.replace(/-/g, '_')}.toml`;
ensureDir(path.join(skillDir, 'agents'));
writeFile(path.join(skillDir, 'agents', filename), buildCodexAgent(agent, agentBody));
@@ -375,10 +387,7 @@ export function createTransformer(config) {
// Agents can declare `providers: <list>` to limit which harnesses
// they emit to. Default (no field) ships everywhere with agentFormat.
if (agent.providers && !agent.providers.includes(provider)) continue;
let body = compileProviderBlocks(agent.body, providerTags);
body = replacePlaceholders(body, placeholderKey, [], allSkillNames);
body = stripRuleMarkers(body);
body = body.replace(/\{\{scripts_path\}\}/g, scriptsPath);
const body = renderAgentBody(agent, { providerTags, placeholderKey, allSkillNames, scriptsPath });
const agentFile = buildAgentFile(config, agent, body);
if (!agentFile) continue;
ensureDir(agentsDir);