feat(setup-pstack): validate and merge model aliases with apply-models.mjs

.github/workflows/gate.yml

@@ -36,6 +36,8 @@ jobs:
       - name: Script tests
         working-directory: plugins/pstack/skills/poteto-mode/scripts
         run: bun run test
+      - name: Setup script tests
+        run: bun test plugins/pstack/skills/setup-pstack/scripts
       - name: Script typecheck
         working-directory: plugins/pstack/skills/poteto-mode/scripts
         run: bun run typecheck

PORTING.md

@@ -251,6 +251,13 @@ you how much upstream moved underneath you. A non-zero count on `skills/setup-ps
 the selection procedure by hand before trusting the skill; the two other owned paths have no
 upstream counterpart and always report as port-only.
 
+`skills/setup-pstack/scripts/apply-models.mjs` makes the config write. `omp config set` stores a
+selector the catalog does not have and an alias no role defines without complaint, so a bad value
+used to surface only at spawn. The script checks every alias, effort, and agent value against
+`omp models --json` and the merged `modelRoles` before its first write, preserves every entry pstack
+does not own, and converges on a rerun. Its tests run in the gate against a stub `omp` on `PATH`,
+and the `setup config contract` check asserts the skill still routes through it.
+
 Known deltas the port mirrors faithfully and will not diverge on. The guide says the verification feature map lives at `references/features` while both trees write `features/`. The guide recommends a daily `/maintain-verification-skill` run while both trees state no cadence.
 
 ## The one thing that needed new code

omp-port/check-port.sh

@@ -316,9 +316,7 @@ runtime_contract() {
 	grep -Fxq 'Preserve every unrelated entry in both maps.' "$setup_skill" || bad="$bad"'unrelated setup ownership preservation is missing'$'\n'
 	grep -Fq 'capability, exact discovered agent name' "$setup_skill" || bad="$bad"'setup capability mapping is missing'$'\n'
 	grep -Fq 'The effort ladder is `max > xhigh > high > medium > low > minimal`.' "$setup_skill" || bad="$bad"'setup effort ladder is missing minimal or ordering'$'\n'
-	grep -Fq 'Strip only that supported suffix and require the remaining base to equal a selector from `omp models --json`.' "$setup_skill" || bad="$bad"'setup suffix or base-selector validation is missing'$'\n'
-	grep -Fq 'Never strip an arbitrary colon fragment.' "$setup_skill" || bad="$bad"'setup allows arbitrary suffix stripping'$'\n'
-	grep -Fq '`:low`, or `:minimal` suffix' "$setup_skill" || bad="$bad"'setup supported suffix list is missing minimal'$'\n'
+	grep -Fq 'APPLY=$(realpath skill://setup-pstack)/scripts/apply-models.mjs' "$setup_skill" && grep -Fq -- '--dry-run' "$setup_skill" && [ -f skills/setup-pstack/scripts/apply-models.mjs ] || bad="$bad"'setup does not plan and write through scripts/apply-models.mjs'$'\n'
 	grep -Fq 'selected model' "$setup_skill" && grep -Fq '`thinking.efforts`' "$setup_skill" || bad="$bad"'setup does not validate supported model efforts'$'\n'
 	grep -Fq 'An omitted override uses the discovered agent' "$setup_skill" && grep -Fq "frontmatter model first, then the parent session's active or default model" "$setup_skill" || bad="$bad"'setup omission precedence is missing'$'\n'
 	grep -Fq 'Family diversity is optional' "$setup_skill" || bad="$bad"'setup family fallback is missing'$'\n'
@@ -331,7 +329,7 @@ runtime_contract() {
 		report "setup config contract" "FAIL"
 		while read -r line; do [ -n "$line" ] && violate "$line"; done <<<"$bad"
 	else
-		report "setup config contract" "PASS  ownership, aliases, model efforts, suffix, and base selector"
+		report "setup config contract" "PASS  ownership, aliases, model efforts, suffix, base selector, and the apply-models lever"
 	fi
 }
 runtime_contract

omp-port/test-gate-mutations.sh

@@ -56,12 +56,15 @@ mutate_alias_shape() {
 }
 
 mutate_supported_suffixes() {
-	sed -i 's/`:low`, or `:minimal`/`:low`/' "$FIXTURE/skills/setup-pstack/SKILL.md"
 	sed -i 's/, and `minimal`//' "$FIXTURE/skills/omp-mechanics/references/setup-pstack-config.md"
 }
 
 mutate_base_selector() {
-	sed -i 's/require the remaining base to equal a selector from `omp models --json`/accept any remaining base/' "$FIXTURE/skills/setup-pstack/SKILL.md"
+	sed -i 's/require the remaining base to equal a selector from `omp models --json`/accept any remaining base/' "$FIXTURE/skills/omp-mechanics/references/setup-pstack-config.md"
+}
+
+mutate_setup_apply_script() {
+	sed -i '/^APPLY=/d' "$FIXTURE/skills/setup-pstack/SKILL.md"
 }
 
 mutate_setup_lever() {
@@ -109,6 +112,7 @@ expect_failure 'per-agent ownership' 'setup config contract' mutate_override_own
 expect_failure 'alias shape' 'setup config contract' mutate_alias_shape
 expect_failure 'supported suffix list' 'setup config contract' mutate_supported_suffixes
 expect_failure 'base selector validation' 'setup config contract' mutate_base_selector
+expect_failure 'setup apply script' 'setup config contract' mutate_setup_apply_script
 expect_failure 'setup model lever' 'omp model levers' mutate_setup_lever
 expect_failure 'router model lever' 'omp model levers' mutate_router_lever
 expect_failure 'agent hub api' 'agent hub api' mutate_hub_api

plugins/pstack/skills/setup-pstack/SKILL.md

@@ -44,17 +44,25 @@ Prefer `ask`. Offer these exact labels and name the current budget when recorded
 
 The effort ladder is `max > xhigh > high > medium > low > minimal`. Keep the detected selector as the base. Choose the requested effort only when it appears in that model's `thinking.efforts` list. Otherwise choose the highest listed effort below it. If none exists, mark that alias unresolved. `unlimited` uses the highest listed effort. Never write a requested suffix that the selected model does not support.
 
-### 4. Validate and confirm
+### 4. Plan with the script and confirm
 
-For each `modelRoles` value, allow an exact detected selector, or one supported `:max`, `:xhigh`, `:high`, `:medium`, `:low`, or `:minimal` suffix. Strip only that supported suffix and require the remaining base to equal a selector from `omp models --json`. Require the suffix to appear in the selected model's `thinking.efforts` list. Never strip an arbitrary colon fragment. Every `task.agentModelOverrides` value must be an exact detected selector or an `@role` alias already present in the current `modelRoles` map. Show the exact mapping table and ask whether to accept it. Mark unavailable agents and unresolved selectors. Offer only detected selectors, configured aliases, or omission with the exact agent-then-parent resolution above.
+`skill://` paths are not executable. Resolve the script once in the bash tool, which expands `skill://` inside `realpath`:
+
+```bash
+APPLY=$(realpath skill://setup-pstack)/scripts/apply-models.mjs
+```
+
+Turn the table into arguments: `--alias <alias>=<selector>[:<effort>]` per alias, `--agent <exact agent name>=@<alias>` per mapped agent, and `--unset-agent <exact agent name>` for an agent still bound to a pstack alias but no longer selected. Run `node "$APPLY" --dry-run <arguments>`; `bun` runs it too. It prints the planned changes, every pstack-owned agent with the model it resolves to, and the family count. Show that output with the table, mark unavailable agents and unresolved selectors, and ask whether to accept it. Offer only detected selectors, configured aliases, or omission with the exact agent-then-parent resolution above.
+
+The script refuses and writes nothing when an alias is not one of the three, a base is not a selector from `omp models --json`, an effort is off the ladder or missing from the selected model's `thinking.efforts` list, or an agent value is not a `@pstack_*` alias defined in `modelRoles`. It never strips an arbitrary colon fragment. It also refuses to change or remove an agent entry pstack does not own, and any `pstack_*` agent key. Fix the row and rerun the dry run. Never edit `config.yml` by hand to get past a refusal.
 
 ### 5. Write the config
 
-Use the shape in `skill://omp-mechanics/references/setup-pstack-config.md`. Write only the three pstack aliases and exact discovered agent keys selected in the table. Update entries currently bound to a pstack alias idempotently, and remove stale entries only when they still point to a pstack alias and their exact agent is no longer selected. Preserve unrelated aliases, overrides, and operator entries. Never write a `pstack_*` agent key.
+On acceptance, run the same command without `--dry-run`. It merges onto the current records, writes `modelRoles` before `task.agentModelOverrides`, and preserves every entry pstack does not own. A rerun with the same arguments writes nothing and says so. If it reports that `modelRoles` was written and `task.agentModelOverrides` was not, rerun the same command to finish.
 
 ### 6. Confirm
 
-Report the exact aliases and agent keys written, the selectors and effort suffixes used, any unavailable capability, and the family count. State that the entries apply to new sessions and that rerunning updates them.
+Report the exact aliases and agent keys written, the selectors and effort suffixes used, any unavailable capability, and the family count from the script's output. One family prints `weaker diversity: one family`; report it as information. State that the entries apply to new sessions and that rerunning updates them. `node "$APPLY" --show` prints the current aliases, agent mappings, and family count.
 
 ### 7. Offer a verification skill (optional)
 

plugins/pstack/skills/setup-pstack/scripts/apply-models.mjs

@@ -0,0 +1,150 @@
+#!/usr/bin/env node
+// Applies setup-pstack's model choices to omp config, checked against `omp models --json` first.
+// `omp config set` stores any string, so an unknown selector or alias otherwise fails only at spawn.
+//
+// Data shape.
+//   OWNED_ALIASES are the only modelRoles keys this script writes.
+//   A Selection is { alias, base, effort }: base is a `selector` from the catalog, effort is null or
+//   an EFFORTS member that the model's `thinking` list names. It is written as base or base:effort.
+//   A task.agentModelOverrides entry is pstack-owned only when its key is an exact agent name and
+//   its value is one of OWNED_VALUES. Every other entry in both records keeps its key, value, and order.
+import { spawnSync } from "node:child_process";
+
+const OWNED_ALIASES = ["pstack_fast_code", "pstack_judgment", "pstack_instruction"];
+const OWNED_VALUES = OWNED_ALIASES.map((alias) => `@${alias}`);
+const EFFORTS = ["max", "xhigh", "high", "medium", "low", "minimal"];
+const RECORDS = { roles: "modelRoles", overrides: "task.agentModelOverrides" };
+const USAGE = `usage: apply-models.mjs --show
+       apply-models.mjs [--dry-run] [--alias <pstack alias>=<provider/model>[:effort]]...
+                        [--agent <exact agent name>=@<pstack alias>]... [--unset-agent <exact agent name>]...`;
+
+const fail = (message) => {
+	throw new Error(message);
+};
+
+function omp(...args) {
+	const run = spawnSync("omp", args, { encoding: "utf8" });
+	if (run.error) fail(`cannot run omp: ${run.error.message}`);
+	if (run.status !== 0) fail(`omp ${args.slice(0, 3).join(" ")} exited ${run.status}: ${(run.stderr || run.stdout).trim()}`);
+	return run.stdout;
+}
+
+function parseArgs(argv) {
+	const request = { show: false, dryRun: false, aliases: new Map(), agents: new Map() };
+	const add = (map, key, value) => (map.has(key) ? fail(`${key} is given twice`) : map.set(key, value));
+	for (let i = 0; i < argv.length; i++) {
+		const flag = argv[i];
+		const value = () => {
+			const next = argv[++i];
+			return next && !next.startsWith("--") ? next : fail(`${flag} needs a value\n${USAGE}`);
+		};
+		const pair = () => {
+			const next = value();
+			const eq = next.indexOf("=");
+			return eq > 0 && eq < next.length - 1 ? [next.slice(0, eq), next.slice(eq + 1)] : fail(`${flag} expects <key>=<value>, got "${next}"`);
+		};
+		if (flag === "--show") request.show = true;
+		else if (flag === "--dry-run") request.dryRun = true;
+		else if (flag === "--alias") add(request.aliases, ...pair());
+		else if (flag === "--agent") add(request.agents, ...pair());
+		else if (flag === "--unset-agent") add(request.agents, value(), null);
+		else fail(`unknown argument "${flag}"\n${USAGE}`);
+	}
+	const edits = request.aliases.size + request.agents.size;
+	if (request.show && (edits || request.dryRun)) fail("--show takes no other argument");
+	if (!request.show && !edits) fail(`nothing to do\n${USAGE}`);
+	return request;
+}
+
+function readCatalog() {
+	const { models } = JSON.parse(omp("models", "--json"));
+	return new Map(models.map((model) => [model.selector, Array.isArray(model.thinking) ? model.thinking : []]));
+}
+
+// A value that is itself a selector wins, so a colon inside a selector is never read as an effort.
+// Otherwise only a known effort suffix is split off; any other colon fragment stays part of the base.
+function parseSelection(alias, value, catalog) {
+	if (!OWNED_ALIASES.includes(alias)) fail(`--alias ${alias}: not a pstack alias, use one of ${OWNED_ALIASES.join(", ")}`);
+	if (catalog.has(value)) return { alias, base: value, effort: null };
+	const cut = value.lastIndexOf(":");
+	const base = value.slice(0, cut);
+	const effort = value.slice(cut + 1);
+	const suffixed = cut > 0 && EFFORTS.includes(effort);
+	if (!suffixed || !catalog.has(base)) fail(`--alias ${alias}=${value}: ${suffixed ? base : value} is not a selector in \`omp models --json\``);
+	const supported = catalog.get(base);
+	if (!supported.includes(effort)) fail(`--alias ${alias}=${value}: ${base} supports ${supported.join(", ") || "no effort suffix"}, not ${effort}`);
+	return { alias, base, effort };
+}
+
+function plan(current, request, catalog) {
+	const roles = { ...current.roles };
+	for (const [alias, value] of request.aliases) {
+		const { base, effort } = parseSelection(alias, value, catalog);
+		roles[alias] = effort ? `${base}:${effort}` : base;
+	}
+	const overrides = { ...current.overrides };
+	for (const [agent, value] of request.agents) {
+		const held = overrides[agent];
+		if (held !== undefined && !OWNED_VALUES.includes(held)) fail(`${agent} maps to ${JSON.stringify(held)}, which pstack does not own; change it with \`omp config\` yourself`);
+		if (value === null) {
+			delete overrides[agent];
+			continue;
+		}
+		if (agent.startsWith("pstack_")) fail(`--agent ${agent}: a pstack alias is not an agent name`);
+		if (!OWNED_VALUES.includes(value)) fail(`--agent ${agent}=${value}: use one of ${OWNED_VALUES.join(", ")}`);
+		const alias = value.slice(1);
+		if (!Object.hasOwn(roles, alias)) fail(`--agent ${agent}=${value}: modelRoles has no ${alias}, add --alias ${alias}=<selector>`);
+		overrides[agent] = value;
+	}
+	return { roles, overrides };
+}
+
+function describe({ roles, overrides }) {
+	const lines = OWNED_ALIASES.map((alias) => `${alias} = ${roles[alias] ?? "(unset)"}`);
+	for (const [agent, value] of Object.entries(overrides)) {
+		if (OWNED_VALUES.includes(value)) lines.push(`${agent} -> ${value} -> ${roles[value.slice(1)] ?? "(unset)"}`);
+	}
+	const families = new Set(OWNED_ALIASES.filter((alias) => roles[alias]).map((alias) => String(roles[alias]).split("/")[0]));
+	lines.push(`model families: ${families.size}${families.size === 1 ? ", weaker diversity: one family" : ""}`);
+	return lines.join("\n");
+}
+
+function changes(before, after, record) {
+	return [...new Set([...Object.keys(before), ...Object.keys(after)])]
+		.filter((key) => before[key] !== after[key])
+		.map((key) => `  ${record}.${key}: ${before[key] ?? "(unset)"} -> ${after[key] ?? "(removed)"}`);
+}
+
+function main(argv) {
+	const request = parseArgs(argv);
+	const current = {};
+	for (const [field, record] of Object.entries(RECORDS)) current[field] = JSON.parse(omp("config", "get", record, "--json")).value;
+	if (request.show) return console.log(describe(current));
+	const next = plan(current, request, request.aliases.size ? readCatalog() : new Map());
+	const stale = Object.keys(RECORDS).filter((field) => JSON.stringify(current[field]) !== JSON.stringify(next[field]));
+	if (!stale.length) return console.log(`no change: config already matches\n${describe(next)}`);
+	const diff = stale.flatMap((field) => changes(current[field], next[field], RECORDS[field]));
+	console.log([request.dryRun ? "planned changes:" : "changes:", ...diff, describe(next)].join("\n"));
+	if (request.dryRun) return console.log("dry run: nothing written");
+	// modelRoles goes first, so an override never lands pointing at an alias that is not written yet.
+	for (const [i, field] of stale.entries()) {
+		try {
+			omp("config", "set", RECORDS[field], JSON.stringify(next[field]));
+		} catch (error) {
+			const landed = stale.slice(0, i).map((done) => RECORDS[done]);
+			const recovery = landed.length
+				? `${landed.join(", ")} was written and ${RECORDS[field]} was not. Rerun the same command to finish; it writes only what still differs.`
+				: "nothing was written";
+			console.error(`apply-models: ${error.message}\n${recovery}`);
+			process.exit(1);
+		}
+	}
+	console.log(`written: ${stale.map((field) => RECORDS[field]).join(", ")}`);
+}
+
+try {
+	main(process.argv.slice(2));
+} catch (error) {
+	console.error(`apply-models: refused: ${error.message}\nnothing was written`);
+	process.exit(1);
+}