From bcc5280c9b87d349bdb9268e730da626b1fb31fc Mon Sep 17 00:00:00 2001 From: danielgap Date: Tue, 22 Sep 2026 23:58:49 +0200 Subject: [PATCH 01/13] test(orchestrator): reproduce the parent-inline skill contract gap (#348) Deterministic baseline for issue #348 on unmodified main: a project skill with an ## Output Contract is indexed by the skill registry, yet the composed parent prompt binds only subagents to read the exact indexed SKILL.md. No parent-inline read obligation exists and the contract body never reaches parent context, so contract markers cannot reach parent-inline results. Offline runtime harness per tests/asset-installation-runtime.test.ts: scoped-env child boots the real SDK session (no network, in-memory settings), forces /skill-registry:refresh, asserts indexing, then captures the composed parent prompt via __testing.buildGentlePrompt with a fidelity pin proving the render went through the composition pipeline. The two gap assertions are guarded flip helpers for the fix slice. Design note: issues/348#issuecomment-5784664642 --- ...rchestrator-inline-skill-execution.test.ts | 295 ++++++++++++++++++ 1 file changed, 295 insertions(+) create mode 100644 tests/orchestrator-inline-skill-execution.test.ts diff --git a/tests/orchestrator-inline-skill-execution.test.ts b/tests/orchestrator-inline-skill-execution.test.ts new file mode 100644 index 000000000..8cf140a89 --- /dev/null +++ b/tests/orchestrator-inline-skill-execution.test.ts @@ -0,0 +1,295 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +// --------------------------------------------------------------------------- +// gentle-shell#348 — slice 1 (repro): the parent-inline skill contract gap. +// +// Deterministic reproduction, expected to PASS on unmodified main: +// +// 1. A project skill with an `## Output Contract` body is indexed by the +// gentle-ai skill registry (discovery succeeds — `.atl/skill-registry.md` +// lists it with scope `project`). +// 2. The composed parent prompt (the exact `buildGentlePrompt` composition the +// primary session's `before_agent_start` handler injects) only binds +// SUBAGENTS to read the exact indexed `SKILL.md` ("subagents read those +// `SKILL.md` files first"). It imposes no parent-inline read obligation: +// there is no "Parent inline execution" section and no wording binding the +// parent's own inline path to read the indexed `SKILL.md` in-session. +// 3. Consequently the skill's contract body marker can never reach +// parent-inline results through the composed prompt. +// +// Slice 2 must flip exactly the two guarded helpers below: +// `assertParentPromptHasNoInlineSkillReadObligation` (the c-check) and +// `assertParentPromptOmitsSkillContractBody` (the d-check). +// +// Harness: same accepted offline runtime pattern as +// tests/asset-installation-runtime.test.ts — the parent test builds a throwaway +// HOME + agent dir + project (all under one mkdtemp root), spawns this file as +// a child with a fully scoped env (no network, in-memory settings), and the +// child boots the real SDK session with the gentle-ai extension active. +// --------------------------------------------------------------------------- + +const FIXTURE_MARKER = "FIXTURE348-CONTRACT-MARK"; +const FIXTURE_SKILL_NAME = "fixture-contract-probe"; +const CHILD_FLAG = "GENTLE_PI_348_INLINE_PROBE_CHILD"; + +const FIXTURE_SKILL_SOURCE = `--- +name: ${FIXTURE_SKILL_NAME} +description: Probe skill for gentle-shell#348 inline contract reproduction. +--- + +# Fixture Contract Probe + +## Output Contract + +Every result produced under this skill MUST include the literal marker +${FIXTURE_MARKER} on its own line. This contract lives only in the indexed +SKILL.md body; the registry index carries name, description, and path — +never this body. +`; + +function extensionSourceUrl(name: string): string { + return JSON.stringify(new URL(`../extensions/${name}.ts`, import.meta.url).href); +} + +function buildShimSource(): string { + // Loads only what the repro needs from the packaged extensions directory: + // the gentle-ai extension (primary-session orchestrator prompt + session + // lifecycle) and the skill-registry extension (project skill discovery). + return ` +import { createGentleAiExtension } from ${extensionSourceUrl("gentle-ai")}; +import skillRegistry from ${extensionSourceUrl("skill-registry")}; +export default function (pi) { + createGentleAiExtension({ nativeReviewCli: null, candidateViews: null, processEnv: {} })(pi); + skillRegistry(pi); +} +`; +} + +// --- guarded flip targets for slice 2 ------------------------------------- + +// c-check: the composed parent prompt must not bind the parent's OWN inline +// execution path to reading the indexed `SKILL.md` in-session. On main only +// subagents owe that read (the asymmetry under test). Slice 2 inverts this +// helper (or narrows its patterns) once the prompt carries the obligation. +function assertParentPromptHasNoInlineSkillReadObligation(prompt: string): void { + const patterns: readonly RegExp[] = [ + /parent[ -]inline[ -]execution/i, + /inline[^.\n]{0,120}`SKILL\.md`/i, + /`SKILL\.md`[^.\n]{0,120}inline/i, + ]; + for (const pattern of patterns) { + const match = pattern.exec(prompt); + const line = match === null ? "" : prompt.split("\n").find((l) => pattern.test(l)) ?? ""; + assert.ok( + match === null, + `composed parent prompt binds the inline path to SKILL.md reading via /${pattern.source}/: ${JSON.stringify(line.trim())}`, + ); + } +} + +// d-check: the fixture skill's contract body never reaches the composed parent +// prompt (the registry indexes metadata only). Slice 2 flips this helper if +// contract bodies start being delivered inline; otherwise it stays a guard. +function assertParentPromptOmitsSkillContractBody(prompt: string, marker: string): void { + assert.ok( + !prompt.includes(marker), + `composed parent prompt leaked the indexed SKILL.md contract body (marker ${marker})`, + ); +} + +// --------------------------------------------------------------------------- + +async function proveInlineSkillContractGap(): Promise { + const { + createAgentSessionFromServices, + createAgentSessionRuntime, + createAgentSessionServices, + ModelRuntime, + SessionManager, + SettingsManager, + } = await import("@earendil-works/pi-coding-agent"); + + const projectDir = process.env.GENTLE_PI_348_PROJECT!; + const shimPath = process.env.GENTLE_PI_348_SHIM!; + const agentDir = process.env.PI_CODING_AGENT_DIR!; + const cwd = projectDir; + + const modelRuntime = await ModelRuntime.create({ + authPath: join(agentDir, "empty-auth.json"), + modelsPath: null, + modelsStorePath: join(agentDir, "models-store.json"), + allowModelNetwork: false, + }); + + const runtime = await createAgentSessionRuntime( + async ({ cwd, sessionManager, sessionStartEvent }) => { + const services = await createAgentSessionServices({ + cwd, + agentDir, + modelRuntime, + settingsManager: SettingsManager.inMemory({ + retry: { enabled: false }, + compaction: { enabled: false }, + }), + resourceLoaderOptions: { + noExtensions: true, + noSkills: true, + noPromptTemplates: true, + noThemes: true, + noContextFiles: true, + additionalExtensionPaths: [shimPath], + }, + }); + // e-check: the real session boots with zero extension errors and + // zero diagnostics, so any prompt-shape finding below is not an + // artifact of a broken boot. + assert.deepEqual(services.resourceLoader.getExtensions().errors, []); + assert.deepEqual(services.diagnostics, []); + return { + ...(await createAgentSessionFromServices({ + services, + sessionManager, + sessionStartEvent, + tools: ["read", "bash"], + })), + services, + diagnostics: services.diagnostics, + }; + }, + { cwd, agentDir, sessionManager: SessionManager.inMemory(cwd) }, + ); + + try { + await runtime.session.bindExtensions({ mode: "print" }); + const session = runtime.session; + + // a-check (discovery): force the live extension to regenerate the + // registry, then prove the fixture project skill is indexed. The slash + // command runs without a model turn, exactly like the accepted harness + // pattern's /gentle:install-sdd step. + await session.prompt("/skill-registry:refresh"); + assert.equal(session.messages.length, 0, "slash activation must not start a model turn"); + + const registryPath = join(cwd, ".atl", "skill-registry.md"); + assert.ok(existsSync(registryPath), "skill registry must be regenerated in the project"); + const registry = readFileSync(registryPath, "utf8"); + assert.ok( + registry.includes(`\`${FIXTURE_SKILL_NAME}\``), + `registry must index the fixture skill: ${FIXTURE_SKILL_NAME}`, + ); + assert.ok( + registry.includes(`skills/${FIXTURE_SKILL_NAME}/SKILL.md`), + "registry must carry the fixture SKILL.md path", + ); + assert.match(registry, /\| project \|/, "fixture skill must be scoped to the project"); + + // Fixture self-check: the contract marker lives in the indexed body. + const skillBody = readFileSync( + join(cwd, "skills", FIXTURE_SKILL_NAME, "SKILL.md"), + "utf8", + ); + assert.ok(skillBody.includes(FIXTURE_MARKER), "fixture must carry the contract marker"); + assert.ok( + !registry.includes(FIXTURE_MARKER), + "registry index must not inline skill bodies (it carries metadata only)", + ); + + // Composed parent prompt capture: `buildGentlePrompt` is the exact + // composition the primary session's `before_agent_start` handler + // injects (persona, cwd, active tool names, RDD status line). With + // `nativeReviewCli: null` the resolved RDD line equals the default + // worst-case "unknown" line, so this call reproduces the boot + // composition for the primary session byte-for-byte. The module + // instance is shared with the shim (same absolute path), and + // `getOrchestratorPrompt` memoization is per background-policy key — + // irrelevant here because this repro asserts prompt content, not + // per-cwd variance. + const { __testing } = await import("../extensions/gentle-ai.ts"); + const toolNames = session.agent.state.tools.map((tool) => tool.name); + const prompt = __testing.buildGentlePrompt("gentleman", cwd, toolNames); + + // Fidelity pin: the render actually went through the composition + // pipeline (assets root substitution + background/RDD status block). + assert.match( + prompt, + /Receipt-driven development: unknown \(native status unavailable\)/, + "captured prompt must be the composed render, not raw asset bytes", + ); + assert.doesNotMatch(prompt, /\{\{/, "composed prompt must have every placeholder resolved"); + + // b-check (asymmetry baseline): the subagent-directed read clause IS + // delivered to the parent prompt — subagents owe the exact read. + assert.match( + prompt, + /subagents read those `SKILL\.md` files first/, + "parent prompt must carry the subagent-directed SKILL.md read clause", + ); + + // c-check + d-check: the gap. No parent-inline read obligation, and + // the indexed contract body never reaches parent context. + assertParentPromptHasNoInlineSkillReadObligation(prompt); + assertParentPromptOmitsSkillContractBody(prompt, FIXTURE_MARKER); + + console.log( + "inline-skill-probe: fixture indexed (project) -> composed parent prompt has the subagent-only SKILL.md read clause, no parent-inline obligation, no contract body", + ); + } finally { + await runtime.dispose(); + } +} + +if (process.env[CHILD_FLAG] === "1") { + await proveInlineSkillContractGap(); +} else { + test("gentle-shell#348 slice 1: registry-indexed skill contract cannot reach the parent-inline path", () => { + const root = mkdtempSync(join(tmpdir(), "gentle-pi-348-probe-")); + try { + const home = join(root, "home"); + const projectDir = join(root, "project"); + const agentDir = join(home, ".pi", "agent"); + const skillDir = join(projectDir, "skills", FIXTURE_SKILL_NAME); + for (const path of [projectDir, agentDir, skillDir]) { + mkdirSync(path, { recursive: true }); + } + writeFileSync(join(skillDir, "SKILL.md"), FIXTURE_SKILL_SOURCE); + + const shimPath = join(root, "shim", "extensions.ts"); + mkdirSync(dirname(shimPath), { recursive: true }); + writeFileSync(shimPath, buildShimSource()); + + const result = spawnSync( + process.execPath, + ["--experimental-strip-types", fileURLToPath(import.meta.url)], + { + cwd: projectDir, + encoding: "utf8", + timeout: 120_000, + env: { + PATH: `${dirname(process.execPath)}:/usr/bin:/bin`, + HOME: home, + TMPDIR: root, + PI_CODING_AGENT_DIR: agentDir, + GENTLE_PI_AGENT_HOME: agentDir, + GENTLE_PI_CONFIG_HOME: join(home, "config"), + XDG_CONFIG_HOME: join(home, ".config"), + PI_OFFLINE: "1", + GENTLE_PI_348_PROJECT: projectDir, + GENTLE_PI_348_SHIM: shimPath, + [CHILD_FLAG]: "1", + }, + }, + ); + assert.ifError(result.error); + assert.equal(result.status, 0, `${result.stdout}\n${result.stderr}`); + assert.match(result.stdout, /inline-skill-probe: fixture indexed/); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }); +} From 8383a54e6a87dada65a91c05c065d50fc3964c5d Mon Sep 17 00:00:00 2001 From: danielgap Date: Wed, 23 Sep 2026 00:35:46 +0200 Subject: [PATCH 02/13] fix(orchestrator): bind the parent inline path to the indexed skill read (#348) One clause in the always-on core Skill Registry Protocol extends the exact-file read duty to the parent's own inline path at the composition boundary (+49 B). The new lazy '### Parent inline execution' section in orchestrator-skills.md carries the detail: in-session read, Output Contract markers in the reply, name-only attribution line (Skill applied (inline): ), and the unreadable-path fallback. Budget: canonical always-on budget steps 8192 -> 8320 (next 256-B step). The controlled-long assets root previously held 7 B headroom (frozen, see odd/tasks/sdd-trigger-boundary.md), so the duty clause required the step; margins are now 191 B (short root) and 86 B (long root). Tests: a contract layer with per-clause negative controls and forbidden-token injections binds the lazy section; the offline runtime harness flips to prove composition-boundary delivery (fixture skill indexed, obligation present in the composed prompt, contract body still excluded, boot clean, fidelity pin intact). Full-suite note: tests/inprocess-reviewer.test.ts has a pre-existing cancelled cluster (26 cancelledByParent) that reproduces on clean main without this change; unrelated. All suites covering the touched surfaces pass. Design note: issues/348#issuecomment-5784664642 (addendum follows). --- assets/orchestrator-skills.md | 13 ++ assets/orchestrator.md | 2 +- tests/orchestrator-budget.test.ts | 7 +- ...rchestrator-inline-skill-execution.test.ts | 80 ++++++------ ...rchestrator-skills-inline-contract.test.ts | 119 ++++++++++++++++++ 5 files changed, 177 insertions(+), 44 deletions(-) create mode 100644 tests/orchestrator-skills-inline-contract.test.ts diff --git a/assets/orchestrator-skills.md b/assets/orchestrator-skills.md index 6e3af07c7..71b5cd730 100644 --- a/assets/orchestrator-skills.md +++ b/assets/orchestrator-skills.md @@ -25,6 +25,19 @@ If a subagent reports `skill_resolution`, interpret it as project/user skill res If any subagent reports a fallback instead of `paths-injected`, treat it as an orchestration gap and correct future delegations by passing exact indexed paths directly. +### Parent inline execution + +When the parent executes work inline that a resolved skill covers (Work +Routing Ladder tier 1), it follows the same read discipline as a delegated +subagent: read the exact indexed `SKILL.md` in this session before acting, +and let the skill's declared `## Output Contract` shape the result, so the +reply carries the same contract markers applying the skill directly would +produce. Close with one attribution line naming the applied skill +(`Skill applied (inline): `); if the indexed path is unreadable, +proceed unskilled and say so in that line. Never present contract markers or +attribute a skill without the in-session read. Attribution carries the skill +name only, never paths. + ## Intent-Driven Skill Discovery For skill-shaped requests, do not treat injected `` as complete. Use the registry and filesystem only as a discovery aid; do not let a trigger table override the user's concrete request or turn a small request into a larger workflow. diff --git a/assets/orchestrator.md b/assets/orchestrator.md index 2fb1fc37f..ba853b06d 100644 --- a/assets/orchestrator.md +++ b/assets/orchestrator.md @@ -79,7 +79,7 @@ When memory is available, the parent selects context and subagents save discover ## Skill Registry Protocol -The parent resolves skill paths once per session under `## Skills to load before work`; subagents read those `SKILL.md` files first, or report unavailable paths. Fallback semantics (`paths-injected`/`fallback-registry`/`fallback-path`/`none`) and the SDD-executor distinction: `orchestrator-skills.md`. +The parent resolves skill paths once per session under `## Skills to load before work`; subagents read those `SKILL.md` files first, or report unavailable paths; the parent's own inline path owes the same read. Fallback semantics (`paths-injected`/`fallback-registry`/`fallback-path`/`none`) and the SDD-executor distinction: `orchestrator-skills.md`. ## Intent-Driven Skill Discovery diff --git a/tests/orchestrator-budget.test.ts b/tests/orchestrator-budget.test.ts index aae003f96..903a334cd 100644 --- a/tests/orchestrator-budget.test.ts +++ b/tests/orchestrator-budget.test.ts @@ -30,7 +30,10 @@ import test, { after } from "node:test"; const REPO_ROOT = join(import.meta.dirname, ".."); const REAL_ASSETS_DIR = join(REPO_ROOT, "assets"); const FIXTURE_PATH = join(import.meta.dirname, "fixtures", "orchestrator.pre-diet.md"); -const BUDGET_BYTES = 8192; +// Canonical always-on budget. gentle-shell#348: the parent-inline skill-read +// duty clause costs +49 B and the controlled-long assets root previously had +// 7 B headroom; the step 8192 -> 8320 restores ~86 B margin. +const BUDGET_BYTES = 8320; const MIN_CONTROLLED_LONG_ASSETS_ROOT_CHARS = 93; const LAZY_ASSET_NAMES = [ @@ -123,7 +126,7 @@ function measureOrchestratorPromptBytes(assetsDir: string): number { // measuring a shorter render again. const RDD_WORST_CASE_LINE = "Receipt-driven development: unknown (native status unavailable)"; -test("getOrchestratorPrompt return value stays within the canonical 8,192 B budget at a short assets root", () => { +test("getOrchestratorPrompt return value stays within the canonical 8,320 B budget at a short assets root", () => { const rendered = __testing.renderOrchestratorPrompt(representativeProductionAssetsDir); assert.ok( rendered.includes(RDD_WORST_CASE_LINE), diff --git a/tests/orchestrator-inline-skill-execution.test.ts b/tests/orchestrator-inline-skill-execution.test.ts index 8cf140a89..8dbab9af0 100644 --- a/tests/orchestrator-inline-skill-execution.test.ts +++ b/tests/orchestrator-inline-skill-execution.test.ts @@ -7,25 +7,26 @@ import test from "node:test"; import { fileURLToPath } from "node:url"; // --------------------------------------------------------------------------- -// gentle-shell#348 — slice 1 (repro): the parent-inline skill contract gap. +// gentle-shell#348 — slice 1 (repro, extended by slice 3): the parent-inline +// skill contract. // -// Deterministic reproduction, expected to PASS on unmodified main: +// What this test proves on the current tree: // // 1. A project skill with an `## Output Contract` body is indexed by the // gentle-ai skill registry (discovery succeeds — `.atl/skill-registry.md` // lists it with scope `project`). // 2. The composed parent prompt (the exact `buildGentlePrompt` composition the -// primary session's `before_agent_start` handler injects) only binds -// SUBAGENTS to read the exact indexed `SKILL.md` ("subagents read those -// `SKILL.md` files first"). It imposes no parent-inline read obligation: -// there is no "Parent inline execution" section and no wording binding the -// parent's own inline path to read the indexed `SKILL.md` in-session. -// 3. Consequently the skill's contract body marker can never reach -// parent-inline results through the composed prompt. -// -// Slice 2 must flip exactly the two guarded helpers below: -// `assertParentPromptHasNoInlineSkillReadObligation` (the c-check) and -// `assertParentPromptOmitsSkillContractBody` (the d-check). +// primary session's `before_agent_start` handler injects) delivers BOTH +// halves of the read duty at the composition boundary: the subagent-directed +// clause ("subagents read those `SKILL.md` files first") AND the parent's +// own inline duty ("the parent's own inline path owes the same read"). The +// always-on core clause is the delivery path; the execution detail +// (attribution line, contract markers, unreadable-path fallback) lives in +// the lazy `assets/orchestrator-skills.md` `### Parent inline execution` +// section and is bound by tests/orchestrator-skills-inline-contract.test.ts. +// 3. The skill's contract body itself still never reaches the composed prompt +// (the registry indexes metadata only; the in-session read delivers the +// body at runtime). // // Harness: same accepted offline runtime pattern as // tests/asset-installation-runtime.test.ts — the parent test builds a throwaway @@ -71,31 +72,27 @@ export default function (pi) { `; } -// --- guarded flip targets for slice 2 ------------------------------------- +// --- composition-boundary delivery guards --------------------------------- -// c-check: the composed parent prompt must not bind the parent's OWN inline -// execution path to reading the indexed `SKILL.md` in-session. On main only -// subagents owe that read (the asymmetry under test). Slice 2 inverts this -// helper (or narrows its patterns) once the prompt carries the obligation. -function assertParentPromptHasNoInlineSkillReadObligation(prompt: string): void { - const patterns: readonly RegExp[] = [ - /parent[ -]inline[ -]execution/i, - /inline[^.\n]{0,120}`SKILL\.md`/i, - /`SKILL\.md`[^.\n]{0,120}inline/i, - ]; - for (const pattern of patterns) { - const match = pattern.exec(prompt); - const line = match === null ? "" : prompt.split("\n").find((l) => pattern.test(l)) ?? ""; - assert.ok( - match === null, - `composed parent prompt binds the inline path to SKILL.md reading via /${pattern.source}/: ${JSON.stringify(line.trim())}`, - ); - } +// c-check: the composed parent prompt must deliver the core clause binding the +// parent's OWN inline path to the same exact-file read subagents owe (landed +// in assets/orchestrator.md, Skill Registry Protocol sentence). Anchored on +// the exact landed wording; the execution detail (attribution line, contract +// markers, unreadable-path fallback) is delivered via the lazy +// `orchestrator-skills.md` section bound by the slice-2 contract test. +function assertParentPromptCarriesInlineSkillReadObligation(prompt: string): void { + assert.match( + prompt, + /; the parent's own inline path owes the same read\./, + "composed parent prompt must bind the parent's own inline path to the same exact-file SKILL.md read subagents owe", + ); } -// d-check: the fixture skill's contract body never reaches the composed parent -// prompt (the registry indexes metadata only). Slice 2 flips this helper if -// contract bodies start being delivered inline; otherwise it stays a guard. +// d-check: the fixture skill's contract body still never reaches the composed +// parent prompt (the registry indexes metadata only; the obligation directs +// the parent to read the indexed file in-session, which delivers the body at +// runtime, not through the prompt). Keeping this guard prevents a future +// "fix" that re-imports skill bodies into the always-on prompt. function assertParentPromptOmitsSkillContractBody(prompt: string, marker: string): void { assert.ok( !prompt.includes(marker), @@ -223,7 +220,7 @@ async function proveInlineSkillContractGap(): Promise { ); assert.doesNotMatch(prompt, /\{\{/, "composed prompt must have every placeholder resolved"); - // b-check (asymmetry baseline): the subagent-directed read clause IS + // b-check (subagent half): the subagent-directed read clause stays // delivered to the parent prompt — subagents owe the exact read. assert.match( prompt, @@ -231,13 +228,14 @@ async function proveInlineSkillContractGap(): Promise { "parent prompt must carry the subagent-directed SKILL.md read clause", ); - // c-check + d-check: the gap. No parent-inline read obligation, and - // the indexed contract body never reaches parent context. - assertParentPromptHasNoInlineSkillReadObligation(prompt); + // c-check + d-check: composition-boundary delivery of the parent's own + // inline duty (core clause), with the contract body itself still kept + // out of the prompt (runtime in-session read delivers it). + assertParentPromptCarriesInlineSkillReadObligation(prompt); assertParentPromptOmitsSkillContractBody(prompt, FIXTURE_MARKER); console.log( - "inline-skill-probe: fixture indexed (project) -> composed parent prompt has the subagent-only SKILL.md read clause, no parent-inline obligation, no contract body", + "inline-skill-probe: fixture indexed (project) -> composed parent prompt carries the subagent SKILL.md read clause AND the parent's own inline duty; contract body stays out of the prompt", ); } finally { await runtime.dispose(); @@ -247,7 +245,7 @@ async function proveInlineSkillContractGap(): Promise { if (process.env[CHILD_FLAG] === "1") { await proveInlineSkillContractGap(); } else { - test("gentle-shell#348 slice 1: registry-indexed skill contract cannot reach the parent-inline path", () => { + test("gentle-shell#348: the parent-inline skill read duty is delivered at the composition boundary", () => { const root = mkdtempSync(join(tmpdir(), "gentle-pi-348-probe-")); try { const home = join(root, "home"); diff --git a/tests/orchestrator-skills-inline-contract.test.ts b/tests/orchestrator-skills-inline-contract.test.ts new file mode 100644 index 000000000..2c31e7560 --- /dev/null +++ b/tests/orchestrator-skills-inline-contract.test.ts @@ -0,0 +1,119 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import test from "node:test"; + +// --------------------------------------------------------------------------- +// gentle-shell#348 — slice 2, contract layer. +// +// The `### Parent inline execution` paragraph in the lazy +// `assets/orchestrator-skills.md` is the narrowest execution boundary of the +// inline skill contract (design note: issues/348#issuecomment-5784664642). +// This test binds every obligation clause of the landed paragraph so no +// wording drift can silently weaken it: each required clause is asserted, the +// section must stay free of delegation-scope tokens and absolute-path +// phrasing, and a negative control per clause proves the assertion function +// rejects mutated copies. +// --------------------------------------------------------------------------- + +const repoRoot = join(import.meta.dirname, ".."); +const asset = readFileSync(join(repoRoot, "assets", "orchestrator-skills.md"), "utf8"); + +const SECTION_HEADING = "### Parent inline execution"; + +// Extracts the `### Parent inline execution` section: from its heading to the +// next markdown heading (any level) or EOF, whichever comes first. +function extractParentInlineExecutionSection(source: string): string { + const start = source.indexOf(SECTION_HEADING); + assert.ok(start >= 0, "the skills asset must carry a `### Parent inline execution` section"); + const remainder = source.slice(start + SECTION_HEADING.length); + const nextHeading = /^#{1,6} .*/m.exec(remainder); + return remainder.slice(0, nextHeading === null ? remainder.length : nextHeading.index); +} + +// Requires every obligation clause of the landed paragraph, and forbids the +// delegation-scope vocabulary and absolute-path phrasing the design note +// keeps out of the inline boundary. +function assertInlineExecutionContract(section: string): void { + assert.match(section, /read the exact indexed `SKILL\.md` in this session/); + assert.match(section, /let the skill's declared `## Output Contract` shape the result/); + assert.match(section, /same contract markers applying the skill directly would\s+produce/); + assert.match(section, /`Skill applied \(inline\): `/); + assert.match(section, /proceed unskilled/); + assert.match(section, /Never present contract markers/); + assert.match(section, /name only, never paths/); + + assert.doesNotMatch(section, /paths-injected|paths-invalid|fallback-/); + assert.doesNotMatch(section, /\/home\//); + assert.doesNotMatch(section, /~\/\.pi/); +} + +test("the landed parent-inline execution paragraph carries the full inline skill contract", () => { + const section = extractParentInlineExecutionSection(asset); + assertInlineExecutionContract(section); +}); + +test("every obligation clause is bound: mutating it makes the contract assertion throw", () => { + const section = extractParentInlineExecutionSection(asset); + const clauseMutations: ReadonlyArray<{ label: string; mutated: string }> = [ + { + label: "in-session read of the exact indexed SKILL.md", + mutated: section.replace( + /read the exact indexed `SKILL\.md` in this session/, + "read the skill whenever convenient", + ), + }, + { + label: "declared ## Output Contract shapes the result", + mutated: section.replace(/shape the result/, "be ignored"), + }, + { + label: "same contract markers applying the skill directly would produce", + mutated: section.replace( + /same contract markers applying the skill directly would\s+produce/, + "whatever markers the model improvises", + ), + }, + { + label: "attribution line format", + mutated: section.replace(/`Skill applied \(inline\): `/, "`some attribution`"), + }, + { + label: "unreadable-path fallback", + mutated: section.replace(/proceed unskilled/, "improvise from the registry description"), + }, + { + label: "never present markers without the in-session read", + mutated: section.replace(/Never present contract markers/, ""), + }, + { + label: "name-only attribution", + mutated: section.replace(/name only, never paths/, "name and, when helpful, its path"), + }, + ]; + for (const { label, mutated } of clauseMutations) { + assert.notEqual(mutated, section, `mutation must change the section: ${label}`); + assert.throws(() => assertInlineExecutionContract(mutated), undefined, label); + } +}); + +test("the inline boundary stays free of delegation-scope tokens and absolute paths", () => { + const section = extractParentInlineExecutionSection(asset); + const forbiddenAdditions: ReadonlyArray<{ label: string; mutated: string }> = [ + { + label: "delegation skill_resolution token", + mutated: `${section}\nReported as skill_resolution: paths-injected.`, + }, + { + label: "absolute home path phrasing", + mutated: `${section}\nRead it from /home/dev/.pi/skills/probe/SKILL.md.`, + }, + { + label: "tilde agent-dir path phrasing", + mutated: `${section}\nRead it from ~/.pi/skills/probe/SKILL.md.`, + }, + ]; + for (const { label, mutated } of forbiddenAdditions) { + assert.throws(() => assertInlineExecutionContract(mutated), undefined, label); + } +}); From cb4d4aaf82d71cb4812e20f6a86d7bb98478cc7c Mon Sep 17 00:00:00 2001 From: danielgap Date: Wed, 23 Sep 2026 01:38:53 +0200 Subject: [PATCH 03/13] test(orchestrator): assert the inline-skill gap through the registered before_agent_start handler (#348) CodeRabbit review follow-up on #1357: the repro previously reconstructed the parent prompt via __testing.buildGentlePrompt and asserted against that reconstruction. It now registers the real gentle-ai extension on a minimal capture-pi (accepted tests/runtime-harness.mjs surface), fires the registered before_agent_start handler with the project cwd, and asserts the b/c/d gap checks against the handler-returned systemPrompt, so the repro binds the prompt the primary session actually delivers. Telemetry is deterministically skipped via the GENTLE_AI_TELEMETRY kill switch; the fail-closed unknown RDD line stays pinned. --- ...rchestrator-inline-skill-execution.test.ts | 71 +++++++++++++------ 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/tests/orchestrator-inline-skill-execution.test.ts b/tests/orchestrator-inline-skill-execution.test.ts index 8cf140a89..c68953ea8 100644 --- a/tests/orchestrator-inline-skill-execution.test.ts +++ b/tests/orchestrator-inline-skill-execution.test.ts @@ -14,12 +14,14 @@ import { fileURLToPath } from "node:url"; // 1. A project skill with an `## Output Contract` body is indexed by the // gentle-ai skill registry (discovery succeeds — `.atl/skill-registry.md` // lists it with scope `project`). -// 2. The composed parent prompt (the exact `buildGentlePrompt` composition the -// primary session's `before_agent_start` handler injects) only binds -// SUBAGENTS to read the exact indexed `SKILL.md` ("subagents read those -// `SKILL.md` files first"). It imposes no parent-inline read obligation: -// there is no "Parent inline execution" section and no wording binding the -// parent's own inline path to read the indexed `SKILL.md` in-session. +// 2. The parent prompt the registered `before_agent_start` handler +// actually returns (persona, cwd, active tool names, RDD status line, +// research capabilities — the real production composition path) only +// binds SUBAGENTS to read the exact indexed `SKILL.md` ("subagents read +// those `SKILL.md` files first"). It imposes no parent-inline read +// obligation: there is no "Parent inline execution" section and no +// wording binding the parent's own inline path to read the indexed +// `SKILL.md` in-session. // 3. Consequently the skill's contract body marker can never reach // parent-inline results through the composed prompt. // @@ -200,22 +202,51 @@ async function proveInlineSkillContractGap(): Promise { "registry index must not inline skill bodies (it carries metadata only)", ); - // Composed parent prompt capture: `buildGentlePrompt` is the exact - // composition the primary session's `before_agent_start` handler - // injects (persona, cwd, active tool names, RDD status line). With - // `nativeReviewCli: null` the resolved RDD line equals the default - // worst-case "unknown" line, so this call reproduces the boot - // composition for the primary session byte-for-byte. The module - // instance is shared with the shim (same absolute path), and - // `getOrchestratorPrompt` memoization is per background-policy key — - // irrelevant here because this repro asserts prompt content, not - // per-cwd variance. - const { __testing } = await import("../extensions/gentle-ai.ts"); + // Parent prompt capture (CodeRabbit review fix): assert against the + // prompt the REGISTERED `before_agent_start` handler returns, not a + // `buildGentlePrompt` reconstruction. The handler is the production + // composition path (persona from ctx.cwd, active tool names via + // pi.getActiveTools, the fail-closed RDD line, research capabilities), + // so the gap assertions below bind the prompt the primary session + // would actually deliver. The capture-pi mirrors the accepted minimal + // registration surface from tests/runtime-harness.mjs; telemetry is + // deterministically skipped through the GENTLE_AI_TELEMETRY kill + // switch, and `nativeReviewCli: null` resolves the worst-case + // "unknown" RDD line exactly like the shim's boot. + const { createGentleAiExtension } = await import("../extensions/gentle-ai.ts"); const toolNames = session.agent.state.tools.map((tool) => tool.name); - const prompt = __testing.buildGentlePrompt("gentleman", cwd, toolNames); + const captureHooks = new Map Promise>>(); + const noop = () => {}; + const capturePi = { + events: { on: () => noop, emit: noop }, + on(name: string, handler: (event: unknown, ctx: unknown) => Promise) { + captureHooks.set(name, [...(captureHooks.get(name) ?? []), handler]); + }, + registerCommand: noop, + registerFlag: noop, + registerTool: noop, + getFlag: () => false, + getCommands: () => [], + getActiveTools: () => toolNames, + getAllTools: () => toolNames.map((name) => ({ name })), + }; + createGentleAiExtension({ + nativeReviewCli: null, + candidateViews: null, + processEnv: { GENTLE_AI_TELEMETRY: "0" }, + })(capturePi as never); + const promptHandler = captureHooks.get("before_agent_start")?.[0]; + assert.ok(promptHandler, "the gentle-ai extension must register a before_agent_start handler"); + const handlerResult = (await promptHandler( + { systemPrompt: "PROBE-BASE" }, + { cwd, hasUI: false, sessionManager: { getSessionId: () => "inline-skill-probe" } }, + )) as { systemPrompt: string }; + const prompt = handlerResult.systemPrompt; - // Fidelity pin: the render actually went through the composition - // pipeline (assets root substitution + background/RDD status block). + // Fidelity pins: the handler result really went through the production + // composition pipeline (base prompt carried through, assets-root + // substitution, background/RDD status block with the fail-closed line). + assert.ok(prompt.startsWith("PROBE-BASE"), "the handler must compose over the base system prompt"); assert.match( prompt, /Receipt-driven development: unknown \(native status unavailable\)/, From e2cf573241fd6d41c306aa27f5aa23bf8217ca20 Mon Sep 17 00:00:00 2001 From: danielgap Date: Fri, 25 Sep 2026 20:34:08 +0200 Subject: [PATCH 04/13] ci(windows): raise candidate-view git timeout for cold runners The Windows candidate view regression fails its first owner-preparation git command with candidate-owner-preparation-failed (ETIMEDOUT) when a cold windows-latest runner exceeds the 10s CANDIDATE_GIT_TIMEOUT_MS cap (#990, #1009 family). Set GENTLE_PI_CANDIDATE_GIT_TIMEOUT_MS=60000 on that step using the override the cap already provides (max 120s) rather than retrying: a git timeout is a diagnosable candidate state, not a transient error. --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1deed4170..84f23223b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -104,6 +104,8 @@ jobs: - name: Run Windows candidate view regression shell: pwsh + env: + GENTLE_PI_CANDIDATE_GIT_TIMEOUT_MS: "60000" run: | $output = & pnpm exec node --experimental-strip-types --test --test-reporter=tap --test-name-pattern "candidate owner|candidate contributor root|candidate executable-mode validation|candidate registry forwards explicit Windows mode validation" tests/review-candidate-view.test.ts 2>&1 $status = $LASTEXITCODE From 091fe3b4843daf3981b9dfcce26eda8179c75a1b Mon Sep 17 00:00:00 2001 From: danielgap Date: Tue, 29 Sep 2026 01:19:26 +0200 Subject: [PATCH 05/13] test(orchestrator): adapt the #348 probe to the appendSystemPrompt handler contract Main's gentle-shell#1485 changed before_agent_start to compose through the mutable systemPromptOptions.appendSystemPrompt section (pi-claude-bridge drops handler-returned replacement prompts), which broke the probe's handlerResult.systemPrompt capture after the main merge. The probe now passes the structured options and reconstructs the delivered prompt as base plus appended block; the gap assertions are unchanged and the repro still passes on unmodified main. --- ...rchestrator-inline-skill-execution.test.ts | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/orchestrator-inline-skill-execution.test.ts b/tests/orchestrator-inline-skill-execution.test.ts index c68953ea8..99d6cf678 100644 --- a/tests/orchestrator-inline-skill-execution.test.ts +++ b/tests/orchestrator-inline-skill-execution.test.ts @@ -237,16 +237,23 @@ async function proveInlineSkillContractGap(): Promise { })(capturePi as never); const promptHandler = captureHooks.get("before_agent_start")?.[0]; assert.ok(promptHandler, "the gentle-ai extension must register a before_agent_start handler"); - const handlerResult = (await promptHandler( - { systemPrompt: "PROBE-BASE" }, - { cwd, hasUI: false, sessionManager: { getSessionId: () => "inline-skill-probe" } }, - )) as { systemPrompt: string }; - const prompt = handlerResult.systemPrompt; + // gentle-shell#1485: pi-claude-bridge forwards only the structured + // systemPromptOptions and drops a handler-returned replacement prompt, + // so the production handler now composes through the mutable + // appendSystemPrompt section and returns undefined. The probe passes the + // structured options and reconstructs the delivered prompt as the base + // prompt plus the appended block, exactly what the harness would send. + const systemPromptOptions = { appendSystemPrompt: "" }; + await promptHandler( + { systemPrompt: "PROBE-BASE", systemPromptOptions }, + { cwd, hasUI: false, sessionManager: { getSessionId: () => "inline-skill-probe" } }, + ); + const prompt = `PROBE-BASE\n\n${systemPromptOptions.appendSystemPrompt}`; - // Fidelity pins: the handler result really went through the production - // composition pipeline (base prompt carried through, assets-root + // Fidelity pins: the captured composition really went through the + // production pipeline (base prompt carried through, assets-root // substitution, background/RDD status block with the fail-closed line). - assert.ok(prompt.startsWith("PROBE-BASE"), "the handler must compose over the base system prompt"); + assert.ok(prompt.startsWith("PROBE-BASE"), "the probe must compose over the base system prompt"); assert.match( prompt, /Receipt-driven development: unknown \(native status unavailable\)/, From 9c120379f3be1d8fb4b247a1de3cf5f5264d4857 Mon Sep 17 00:00:00 2001 From: danielgap Date: Thu, 1 Oct 2026 01:18:14 +0200 Subject: [PATCH 06/13] test(orchestrator): re-measure the canonical prompt budget after the refresh merge (#348) --- tests/orchestrator-budget.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/orchestrator-budget.test.ts b/tests/orchestrator-budget.test.ts index 997679d3e..4f200066b 100644 --- a/tests/orchestrator-budget.test.ts +++ b/tests/orchestrator-budget.test.ts @@ -31,7 +31,10 @@ const REPO_ROOT = join(import.meta.dirname, ".."); const REAL_ASSETS_DIR = join(REPO_ROOT, "assets"); const FIXTURE_PATH = join(import.meta.dirname, "fixtures", "orchestrator.pre-diet.md"); // Canonical always-on budget; re-measured at the controlled 128-char assets-root worst case after main's SDD/openspec removal shrank the core (prior pin 8320 pre-dated that drift). -const BUDGET_BYTES = 7131; +// Canonical worst case (controlled long assets root), re-measured after the +// 2026-09-30 refresh merge: main grew the shared prompt ~380 B past the +// pre-merge 7131 B canonical. Hard cap stays 8192 B. +const BUDGET_BYTES = 7560; const MIN_CONTROLLED_LONG_ASSETS_ROOT_CHARS = 93; const LAZY_ASSET_NAMES = [ From 12a2ae51f8a0457393b777c760f4c9e305feb567 Mon Sep 17 00:00:00 2001 From: danielgap Date: Mon, 5 Oct 2026 18:46:39 +0200 Subject: [PATCH 07/13] test(orchestrator): re-pin the canonical prompt budget to the measured 8,501 B after the 2026-10-05 refresh merge (#348) --- tests/orchestrator-budget.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/orchestrator-budget.test.ts b/tests/orchestrator-budget.test.ts index 8bef05c6f..01c5d4103 100644 --- a/tests/orchestrator-budget.test.ts +++ b/tests/orchestrator-budget.test.ts @@ -33,9 +33,12 @@ const FIXTURE_PATH = join(import.meta.dirname, "fixtures", "orchestrator.pre-die // Canonical always-on budget; re-measured at the controlled 128-char assets-root worst case after main's SDD/openspec removal shrank the core (prior pin 8320 pre-dated that drift). // Canonical worst case (controlled long assets root), re-measured after the // 2026-09-30 refresh merge: main grew the shared prompt ~380 B past the -// pre-merge 7131 B canonical. After the 2026-10-05 refresh merge this branch's -// slim core keeps the 7560 B pin (main's 8,400 B covers its fatter core, #1731 T10). -const BUDGET_BYTES = 7560; +// pre-merge 7131 B canonical. Re-measured after the 2026-10-05 refresh merge: +// this branch's #348 contract text (parent inline path owes the same read) plus +// main's grown shared prompt put the controlled worst case at 8,501 B, past +// main's #1731 T10 cap (8,400 B). Pinned to the measured value; whether the +// contract prose should be trimmed back under 8,400 B is a review decision. +const BUDGET_BYTES = 8501; const MIN_CONTROLLED_LONG_ASSETS_ROOT_CHARS = 93; const LAZY_ASSET_NAMES = [ From d91531bbaec2a6aa02ec69ace1ba3e9371eada46 Mon Sep 17 00:00:00 2001 From: danielgap Date: Mon, 5 Oct 2026 19:24:39 +0200 Subject: [PATCH 08/13] fix(orchestrator): reconcile the #348 clause with main's tightened phrasing and re-pin the canonical budget at 8,457 B --- assets/orchestrator.md | 2 +- tests/orchestrator-budget.test.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/assets/orchestrator.md b/assets/orchestrator.md index d41df9350..24d60a041 100644 --- a/assets/orchestrator.md +++ b/assets/orchestrator.md @@ -70,7 +70,7 @@ With memory, the parent selects context; subagents save discoveries before retur ## Skill Registry Protocol -The parent resolves skill paths once per session under `## Skills to load before work`; subagents read those `SKILL.md` files first, or report unavailable paths; the parent's own inline path owes the same read. Fallback semantics (`paths-injected`/`fallback-registry`/`fallback-path`/`none`): `orchestrator-skills.md`. +The parent resolves skill paths once per session under `## Skills to load before work`; subagents read those `SKILL.md` files first (`paths-injected`) or report unavailable paths; the parent's own inline path owes the same read. Fallback semantics: `orchestrator-skills.md`. ## Intent-Driven Skill Discovery diff --git a/tests/orchestrator-budget.test.ts b/tests/orchestrator-budget.test.ts index 01c5d4103..df0af96ce 100644 --- a/tests/orchestrator-budget.test.ts +++ b/tests/orchestrator-budget.test.ts @@ -34,11 +34,11 @@ const FIXTURE_PATH = join(import.meta.dirname, "fixtures", "orchestrator.pre-die // Canonical worst case (controlled long assets root), re-measured after the // 2026-09-30 refresh merge: main grew the shared prompt ~380 B past the // pre-merge 7131 B canonical. Re-measured after the 2026-10-05 refresh merge: -// this branch's #348 contract text (parent inline path owes the same read) plus -// main's grown shared prompt put the controlled worst case at 8,501 B, past -// main's #1731 T10 cap (8,400 B). Pinned to the measured value; whether the -// contract prose should be trimmed back under 8,400 B is a review decision. -const BUDGET_BYTES = 8501; +// the #348 clause (the parent's own inline path owes the same read) rides on +// main's tightened phrasing, putting the controlled long-assets-root worst +// case at 8,457 B. The separate 8,400 B core cap in +// orchestrator-rdd-ownership.test.ts still holds. +const BUDGET_BYTES = 8457; const MIN_CONTROLLED_LONG_ASSETS_ROOT_CHARS = 93; const LAZY_ASSET_NAMES = [ From 4736a680d6b332c75c9225c8e2cd97559248adb1 Mon Sep 17 00:00:00 2001 From: danielgap Date: Fri, 9 Oct 2026 16:00:36 +0200 Subject: [PATCH 09/13] fix(orchestrator): keep the #348 inline clause within #1731's 8,400 B default-render guard Merge-forward of origin/main (552 commits) exposed a real budget bust: the branch's #348 skill-registry clause on top of main's #1731-narrowed core pushed the default-root parent prompt to 8,413 B, over the #1731 T10 user-decision 8,400 B guard in tests/orchestrator-rdd-ownership.test.ts. Compress the clause to main's compact shape plus only the guarded parent-inline duty sentence (-53 B): the default-root render measures 8,360 B, so the 8,400 guard passes untouched; the fallback-semantics enumeration tokens stay out of the lazy section (orchestrator-skills-inline-contract guard). The canonical worst-case budget re-measures at 8,448 B at the controlled 128-char assets root and is re-pinned with provenance; the `paths-injected` token the skill-registry test pins stays in the core. --- assets/orchestrator.md | 2 +- tests/orchestrator-budget.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/orchestrator.md b/assets/orchestrator.md index d41df9350..6c12d9633 100644 --- a/assets/orchestrator.md +++ b/assets/orchestrator.md @@ -70,7 +70,7 @@ With memory, the parent selects context; subagents save discoveries before retur ## Skill Registry Protocol -The parent resolves skill paths once per session under `## Skills to load before work`; subagents read those `SKILL.md` files first, or report unavailable paths; the parent's own inline path owes the same read. Fallback semantics (`paths-injected`/`fallback-registry`/`fallback-path`/`none`): `orchestrator-skills.md`. +The parent resolves skill paths once per session under `## Skills to load before work`; subagents read those `SKILL.md` files first (`paths-injected`) or report unavailable paths; the parent's own inline path owes the same read. Fallbacks: `orchestrator-skills.md`. ## Intent-Driven Skill Discovery diff --git a/tests/orchestrator-budget.test.ts b/tests/orchestrator-budget.test.ts index 5694134d0..78cea4bb7 100644 --- a/tests/orchestrator-budget.test.ts +++ b/tests/orchestrator-budget.test.ts @@ -30,8 +30,8 @@ import test, { after } from "node:test"; const REPO_ROOT = join(import.meta.dirname, ".."); const REAL_ASSETS_DIR = join(REPO_ROOT, "assets"); const FIXTURE_PATH = join(import.meta.dirname, "fixtures", "orchestrator.pre-diet.md"); -// Canonical always-on budget; re-measured at the controlled 128-char assets-root worst case (merge of main #1731 narrowing + this branch's #348 clause). -const BUDGET_BYTES = 7131; +// Canonical always-on budget; re-measured at the controlled 128-char assets-root worst case: main's #1731 narrowed core (8,400) plus this branch's compact #348 parent-inline duty clause (+48 B). Default-root render stays within #1731's 8,400 guard (orchestrator-rdd-ownership.test.ts). +const BUDGET_BYTES = 8448; const MIN_CONTROLLED_LONG_ASSETS_ROOT_CHARS = 93; const LAZY_ASSET_NAMES = [ From 26a7a6dbd1f088705411f508c7c34b1a117e8da7 Mon Sep 17 00:00:00 2001 From: danielgap Date: Fri, 9 Oct 2026 16:00:50 +0200 Subject: [PATCH 10/13] test(orchestrator): platform-correct child PATH and offline provider-backed inline turn Resolve both CodeRabbit actionables on #1358 (review 2026-10-09T09:43Z): - Child PATH now joins the Node executable directory and the current platform's system directories (win32: SystemRoot\System32) with node:path's delimiter instead of a hardcoded ':'-separated Unix PATH. - Add the execution half of the #348 contract through the existing offline provider seam (modelRuntime.registerProvider + streamSimple with createAssistantMessageEventStream, as in bridge-wake-lifecycle): turn 1 issues the read tool call on the exact registry-indexed SKILL.md path, the tool result carries the contract body, turn 2 applies the marker and closes with the 'Skill applied (inline): fixture-contract-probe' attribution line. The turn provably runs under the composed prompt that carries the read duty. The prompt-only guards (assertParentPromptCarriesInlineSkillReadObligation, assertParentPromptOmitsSkillContractBody) are unchanged. Evidence: focused run green; negative probe (scripted model that skips the read) fails the test, proving the new assertions bind. --- ...rchestrator-inline-skill-execution.test.ts | 166 +++++++++++++++++- 1 file changed, 162 insertions(+), 4 deletions(-) diff --git a/tests/orchestrator-inline-skill-execution.test.ts b/tests/orchestrator-inline-skill-execution.test.ts index 8dbab9af0..fb8de5a91 100644 --- a/tests/orchestrator-inline-skill-execution.test.ts +++ b/tests/orchestrator-inline-skill-execution.test.ts @@ -1,10 +1,16 @@ import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; -import { dirname, join } from "node:path"; +import { platform, tmpdir } from "node:os"; +import { delimiter, dirname, join } from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; +import type { + AssistantMessage, + ToolCall, + ToolResultMessage, + TranscriptContext, +} from "@earendil-works/pi-ai"; // --------------------------------------------------------------------------- // gentle-shell#348 — slice 1 (repro, extended by slice 3): the parent-inline @@ -27,6 +33,15 @@ import { fileURLToPath } from "node:url"; // 3. The skill's contract body itself still never reaches the composed prompt // (the registry indexes metadata only; the in-session read delivers the // body at runtime). +// 4. Execution half, via the accepted offline provider seam +// (`modelRuntime.registerProvider` + `streamSimple` with +// `createAssistantMessageEventStream`, as in bridge-wake-lifecycle): a +// scripted turn complies with the parent-inline contract the lazy +// `orchestrator-skills.md` `### Parent inline execution` section specifies +// — the parent reads the exact indexed `SKILL.md` path through the read +// tool, the tool result carries the contract body, and the final reply +// applies the contract (marker) and closes with the +// `Skill applied (inline): ` attribution line. // // Harness: same accepted offline runtime pattern as // tests/asset-installation-runtime.test.ts — the parent test builds a throwaway @@ -38,6 +53,8 @@ import { fileURLToPath } from "node:url"; const FIXTURE_MARKER = "FIXTURE348-CONTRACT-MARK"; const FIXTURE_SKILL_NAME = "fixture-contract-probe"; const CHILD_FLAG = "GENTLE_PI_348_INLINE_PROBE_CHILD"; +const PROVIDER_ID = "gentle-348-fixture"; +const MODEL_ID = "inline-probe"; const FIXTURE_SKILL_SOURCE = `--- name: ${FIXTURE_SKILL_NAME} @@ -124,6 +141,85 @@ async function proveInlineSkillContractGap(): Promise { allowModelNetwork: false, }); + // Offline scripted provider turn (existing seam, same shape as + // tests/bridge-wake-lifecycle.test.ts): two deterministic responses. Turn 1 + // issues the read tool call on the exact registry-indexed SKILL.md path; + // turn 2 applies the contract body that came back and closes with the + // attribution line the lazy `orchestrator-skills.md` section requires. + const { createAssistantMessageEventStream, getCurrentSystemPrompt } = await import("@earendil-works/pi-ai"); + const providerRequests: TranscriptContext[] = []; + let turnStep = 0; + modelRuntime.registerProvider(PROVIDER_ID, { + api: "gentle-348-offline", + apiKey: "fixture-only", + baseUrl: "http://invalid.local", + models: [ + { + id: MODEL_ID, + name: "Gentle #348 inline probe", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 100_000, + maxTokens: 4_096, + }, + ], + streamSimple: (model, context) => { + providerRequests.push(structuredClone(context)); + const stream = createAssistantMessageEventStream(); + const readStep = turnStep++ === 0; + const usage = { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }; + const message: AssistantMessage = { + role: "assistant", + api: model.api, + provider: model.provider, + model: model.id, + content: [], + stopReason: readStep ? "toolUse" : "stop", + timestamp: Date.now(), + usage, + }; + queueMicrotask(() => { + stream.push({ type: "start", partial: message }); + if (readStep) { + const toolCall: ToolCall = { + type: "toolCall", + id: "call-348-skill-read", + name: "read", + arguments: { path: join(cwd, "skills", FIXTURE_SKILL_NAME, "SKILL.md") }, + }; + message.content.push(toolCall); + stream.push({ type: "toolcall_start", contentIndex: 0, partial: message }); + stream.push({ + type: "toolcall_delta", + contentIndex: 0, + delta: JSON.stringify(toolCall.arguments), + partial: message, + }); + stream.push({ type: "toolcall_end", contentIndex: 0, toolCall, partial: message }); + } else { + const text = `Probe result under the fixture contract.\n${FIXTURE_MARKER}\nSkill applied (inline): ${FIXTURE_SKILL_NAME}`; + message.content.push({ type: "text", text }); + stream.push({ type: "text_start", contentIndex: 0, partial: message }); + stream.push({ type: "text_delta", contentIndex: 0, delta: text, partial: message }); + stream.push({ type: "text_end", contentIndex: 0, content: text, partial: message }); + } + stream.push({ type: "done", reason: message.stopReason, message }); + stream.end(); + }); + return stream; + }, + }); + const model = modelRuntime.getModel(PROVIDER_ID, MODEL_ID); + assert.ok(model, "scripted offline provider model must resolve"); + const runtime = await createAgentSessionRuntime( async ({ cwd, sessionManager, sessionStartEvent }) => { const services = await createAgentSessionServices({ @@ -154,6 +250,7 @@ async function proveInlineSkillContractGap(): Promise { sessionManager, sessionStartEvent, tools: ["read", "bash"], + model, })), services, diagnostics: services.diagnostics, @@ -234,8 +331,61 @@ async function proveInlineSkillContractGap(): Promise { assertParentPromptCarriesInlineSkillReadObligation(prompt); assertParentPromptOmitsSkillContractBody(prompt, FIXTURE_MARKER); + // f-check (execution half): one scripted offline provider turn walks the + // full parent-inline contract: the model reads the exact indexed + // SKILL.md path through the read tool, the tool result carries the + // contract body, and the final reply applies the contract and closes + // with the attribution line. + await session.prompt("Produce the fixture probe result inline now, applying the fixture-contract-probe skill."); + assert.ok(session.isIdle, "scripted turn must settle"); + assert.equal(providerRequests.length, 2, "scripted turn must be exactly two provider calls"); + + // f1: the turn actually ran under the composed prompt that carries the + // duty (same prompt-only guards, applied to the captured request). + const turnSystemPrompt = getCurrentSystemPrompt(providerRequests[0].messages); + assertParentPromptCarriesInlineSkillReadObligation(turnSystemPrompt); + + // f2: the parent read the exact registry-indexed SKILL.md path. + const assistantMessages = session.messages.filter( + (message): message is AssistantMessage => message.role === "assistant", + ); + const skillRead = assistantMessages + .flatMap((message) => message.content) + .find((block): block is ToolCall => block.type === "toolCall" && block.name === "read"); + assert.ok(skillRead, "parent turn must issue a read tool call"); + const indexedSkillPath = join(cwd, "skills", FIXTURE_SKILL_NAME, "SKILL.md"); + assert.equal( + (skillRead.arguments as { path?: string }).path, + indexedSkillPath, + "parent must read the exact registry-indexed SKILL.md path", + ); + + // f3: the read executed against the real indexed file — the tool result + // delivers the contract body. + const skillReadResult = session.messages.find( + (message): message is ToolResultMessage => + message.role === "toolResult" && message.toolCallId === skillRead.id, + ); + assert.ok(skillReadResult, "read tool call must have a tool result"); + assert.ok(!skillReadResult.isError, "indexed SKILL.md read must not error"); + assert.ok( + JSON.stringify(skillReadResult.content).includes(FIXTURE_MARKER), + "tool result must deliver the indexed contract body", + ); + + // f4: the final reply applies the contract and attributes the skill. + const finalText = (assistantMessages.at(-1)?.content ?? []) + .filter((block): block is { type: "text"; text: string } => block.type === "text") + .map((block) => block.text) + .join("\n"); + assert.ok(finalText.includes(FIXTURE_MARKER), "final reply must carry the skill's contract marker"); + assert.ok( + finalText.includes(`Skill applied (inline): ${FIXTURE_SKILL_NAME}`), + "final reply must close with the inline attribution line", + ); + console.log( - "inline-skill-probe: fixture indexed (project) -> composed parent prompt carries the subagent SKILL.md read clause AND the parent's own inline duty; contract body stays out of the prompt", + "inline-skill-probe: fixture indexed (project) -> composed parent prompt carries the subagent SKILL.md read clause AND the parent's own inline duty; contract body stays out of the prompt; scripted turn read the indexed SKILL.md and applied the contract inline", ); } finally { await runtime.dispose(); @@ -269,7 +419,15 @@ if (process.env[CHILD_FLAG] === "1") { encoding: "utf8", timeout: 120_000, env: { - PATH: `${dirname(process.execPath)}:/usr/bin:/bin`, + // CR fix: platform-correct child PATH — node:path `delimiter` plus + // the current platform's system directories, keeping the Node + // executable directory first. + PATH: [ + dirname(process.execPath), + ...(platform() === "win32" + ? [join(process.env.SystemRoot ?? "C:\\Windows", "System32")] + : ["/usr/bin", "/bin"]), + ].join(delimiter), HOME: home, TMPDIR: root, PI_CODING_AGENT_DIR: agentDir, From b8f3996426b3bff54a97bb5fdedda0ec938df2d9 Mon Sep 17 00:00:00 2001 From: danielgap Date: Fri, 9 Oct 2026 16:31:03 +0200 Subject: [PATCH 11/13] test(orchestrator): satisfy the done-event reason type in the #348 turn script The scripted provider's done event narrows reason to the no-error literal union; share one stopReason const between the AssistantMessage field and the event so typecheck records no new diagnostic (TS2322 at the done push). --- tests/orchestrator-inline-skill-execution.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/orchestrator-inline-skill-execution.test.ts b/tests/orchestrator-inline-skill-execution.test.ts index fb8de5a91..85c1f59a6 100644 --- a/tests/orchestrator-inline-skill-execution.test.ts +++ b/tests/orchestrator-inline-skill-execution.test.ts @@ -168,6 +168,9 @@ async function proveInlineSkillContractGap(): Promise { providerRequests.push(structuredClone(context)); const stream = createAssistantMessageEventStream(); const readStep = turnStep++ === 0; + // The done event's `reason` excludes "error"; share one literal union + // between the message field and the event so both typecheck. + const stopReason = readStep ? ("toolUse" as const) : ("stop" as const); const usage = { input: 0, output: 0, @@ -182,7 +185,7 @@ async function proveInlineSkillContractGap(): Promise { provider: model.provider, model: model.id, content: [], - stopReason: readStep ? "toolUse" : "stop", + stopReason, timestamp: Date.now(), usage, }; @@ -211,7 +214,7 @@ async function proveInlineSkillContractGap(): Promise { stream.push({ type: "text_delta", contentIndex: 0, delta: text, partial: message }); stream.push({ type: "text_end", contentIndex: 0, content: text, partial: message }); } - stream.push({ type: "done", reason: message.stopReason, message }); + stream.push({ type: "done", reason: stopReason, message }); stream.end(); }); return stream; From 670f84665346304aeebc513dfeac66c9caba73f9 Mon Sep 17 00:00:00 2001 From: danielgap Date: Thu, 15 Oct 2026 01:18:34 +0200 Subject: [PATCH 12/13] test(orchestrator): bind inline skill probe reply to the read result (#348) The scripted second provider turn emitted the contract marker unconditionally, so the final-reply assertion could pass even if the SKILL.md read never delivered it. Emit the marker only when the read tool result carries it, and assert the final provider call received that result. Also require the inline attribution to be the reply's last line and to appear exactly once, instead of a substring match. --- ...rchestrator-inline-skill-execution.test.ts | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/tests/orchestrator-inline-skill-execution.test.ts b/tests/orchestrator-inline-skill-execution.test.ts index 85c1f59a6..abd4f6ae9 100644 --- a/tests/orchestrator-inline-skill-execution.test.ts +++ b/tests/orchestrator-inline-skill-execution.test.ts @@ -208,7 +208,14 @@ async function proveInlineSkillContractGap(): Promise { }); stream.push({ type: "toolcall_end", contentIndex: 0, toolCall, partial: message }); } else { - const text = `Probe result under the fixture contract.\n${FIXTURE_MARKER}\nSkill applied (inline): ${FIXTURE_SKILL_NAME}`; + // Emit the marker only when the read tool result actually delivered it, + // so the final reply depends on the executed read, not on the script. + const readResult = context.messages.find( + (entry): entry is ToolResultMessage => + entry.role === "toolResult" && entry.toolCallId === "call-348-skill-read", + ); + const deliveredMarker = readResult && JSON.stringify(readResult.content).includes(FIXTURE_MARKER) ? FIXTURE_MARKER : "contract marker unavailable"; + const text = `Probe result under the fixture contract.\n${deliveredMarker}\nSkill applied (inline): ${FIXTURE_SKILL_NAME}`; message.content.push({ type: "text", text }); stream.push({ type: "text_start", contentIndex: 0, partial: message }); stream.push({ type: "text_delta", contentIndex: 0, delta: text, partial: message }); @@ -381,10 +388,21 @@ async function proveInlineSkillContractGap(): Promise { .filter((block): block is { type: "text"; text: string } => block.type === "text") .map((block) => block.text) .join("\n"); - assert.ok(finalText.includes(FIXTURE_MARKER), "final reply must carry the skill's contract marker"); + const secondRequestRead = providerRequests[1].messages.find( + (entry): entry is ToolResultMessage => entry.role === "toolResult" && entry.toolCallId === skillRead.id, + ); assert.ok( - finalText.includes(`Skill applied (inline): ${FIXTURE_SKILL_NAME}`), - "final reply must close with the inline attribution line", + secondRequestRead && JSON.stringify(secondRequestRead.content).includes(FIXTURE_MARKER), + "the final provider call must receive the read result carrying the contract marker", + ); + assert.ok(finalText.includes(FIXTURE_MARKER), "final reply must carry the skill's contract marker"); + const attribution = `Skill applied (inline): ${FIXTURE_SKILL_NAME}`; + const finalLines = finalText.trimEnd().split(/\r?\n/); + assert.equal(finalLines.at(-1), attribution, "final reply must close with the inline attribution line"); + assert.equal( + finalLines.filter((line) => line === attribution).length, + 1, + "the inline attribution line must appear exactly once", ); console.log( From 9433f6eb46546e5a69c8d00fb4a25b575548cd56 Mon Sep 17 00:00:00 2001 From: danielgap Date: Sun, 18 Oct 2026 00:57:49 +0200 Subject: [PATCH 13/13] test(orchestrator): raise the core prompt budget to 9,150 B for the inline skill read (#348) The Skill Registry Protocol sentence that binds the parent's own inline path to the same SKILL.md read adds 50 B, which takes the rendered core to 9,144 B at the controlled 128-char assets root after merging main. Raise the budget to 9,150 B in both tests that pin it and regenerate the primary-session prompt golden, whose only change is that sentence. --- tests/fixtures/primary-session-prompt/gentleman.golden.md | 2 +- tests/orchestrator-budget.test.ts | 4 +++- tests/orchestrator-rdd-ownership.test.ts | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/fixtures/primary-session-prompt/gentleman.golden.md b/tests/fixtures/primary-session-prompt/gentleman.golden.md index 30cd8995e..ed85d9e39 100644 --- a/tests/fixtures/primary-session-prompt/gentleman.golden.md +++ b/tests/fixtures/primary-session-prompt/gentleman.golden.md @@ -123,7 +123,7 @@ With memory, the parent selects context; subagents save discoveries before retur ## Skill Registry Protocol -The parent resolves skill paths once per session under `## Skills to load before work`; subagents read those `SKILL.md` files first (`paths-injected`) or report unavailable paths; fallbacks: `orchestrator-skills.md`. +The parent resolves skill paths once per session under `## Skills to load before work`; subagents read those `SKILL.md` files first (`paths-injected`) or report unavailable paths; the parent's own inline path owes the same read. Fallbacks: `orchestrator-skills.md`. ## Intent-Driven Skill Discovery diff --git a/tests/orchestrator-budget.test.ts b/tests/orchestrator-budget.test.ts index 68f46a4b4..01c80e0be 100644 --- a/tests/orchestrator-budget.test.ts +++ b/tests/orchestrator-budget.test.ts @@ -33,7 +33,9 @@ const FIXTURE_PATH = join(import.meta.dirname, "fixtures", "orchestrator.pre-die // gentle-shell#1731 T10 (user decision): 8,192 -> 8,400 B for the narrowed high-risk items 1 and 3. // Verification by named risk (escalate-only rule 3) and the writer-rule fix (triggers hold only when // their condition holds; a unit is independent behavior over a size floor), user decision 2026-10-14: 8,400 -> 9,100 B. -const BUDGET_BYTES = 9100; +// gentle-shell#348: the Skill Registry Protocol binds the parent's own inline path to the same SKILL.md read +// (one sentence, 50 B), user decision 2026-10-18: 9,100 -> 9,150 B. +const BUDGET_BYTES = 9150; const MIN_CONTROLLED_LONG_ASSETS_ROOT_CHARS = 93; const LAZY_ASSET_NAMES = [ diff --git a/tests/orchestrator-rdd-ownership.test.ts b/tests/orchestrator-rdd-ownership.test.ts index bd1c0ac26..1d0517c5d 100644 --- a/tests/orchestrator-rdd-ownership.test.ts +++ b/tests/orchestrator-rdd-ownership.test.ts @@ -72,7 +72,7 @@ test("rendered parent prompt keeps the RDD boundary while omitting lifecycle mir rendered.includes("Receipt-driven development: unknown (native status unavailable)"), "the default render must include the worst-case RDD status line", ); - assert.ok(Buffer.byteLength(rendered, "utf8") <= 9100, "the rendered parent prompt must stay within the 9,100 B core budget"); + assert.ok(Buffer.byteLength(rendered, "utf8") <= 9150, "the rendered parent prompt must stay within the 9,150 B core budget"); }); test("static prompts retain ODD and delegated-work guidance without SDD", () => {