fix(tests): fix version assertion and ask-user TS type error

[ldangelo]

Summary

• Update version assertion in packages/development/tests/auto-team-trd-parser.test.js from 2.12.0 to 2.13.0 to match the current implement-trd-beads.yaml metadata version
• Fix TS2322 type error in packages/pi/extensions/ask-user.ts line 30 by replacing the JSON Schema wrapper object ({ type: 'object', properties: {...}, required: [...] }) with the flat Record<string, { type: string; description: string }> shape required by the ExtensionAPI.parameters interface

Root Cause

Failure 1: implement-trd-beads.yaml was bumped to 2.13.0 without updating the version assertion in the corresponding test.

Failure 2: ask-user.ts used a JSON Schema-style parameters object with a type: 'object' wrapper. The ExtensionAPI interface and the test both expect a flat map of parameter names to { type, description } descriptors.

Test plan

• Verify auto-team-trd-parser.test.js passes with the updated 2.13.0 assertion
• Verify ask-user.test.ts passes with capturedConfig.parameters.question.type === 'string' resolving correctly
• Confirm no other tests regress

🤖 Generated with Claude Code

[claude]

Code Review

Overview

The PR title says "fix version assertion and ask-user TS type error" and the description lists only two small bug fixes, but the actual diff is 99 files, +4 308 / -766 lines — it implements an entirely new Tiered Model Aliases feature (PRD/TRD-2026-021). The two stated fixes are included, but they are minor relative to the bulk of this PR. Reviewers should be aware of the true scope before approving.

---

🔴 Critical

1. Runtime state files committed to git (.beads/)

.beads/dolt-server.pid and .beads/dolt-server.port are machine-specific runtime state that change every time the Beads server restarts. These will create noisy, meaningless diffs on every developer's machine. .gitignore currently lists .beads.bd/ but not .beads/ itself. Add to .gitignore:

.beads/dolt-server.pid
.beads/dolt-server.port

.beads/issues.jsonl and .beads/.local_version changes may be intentional, but the PID/port files definitely should not be tracked.

2. BYPASS_COMMANDS namespace mismatch — map-model self-blocks

In packages/core/lib/config-loader.js:

const BYPASS_COMMANDS = ['map-model', 'migrate-model-config'];

In packages/core/lib/model-resolver.js:

function preflightValidate(config, commandName) {
  if (BYPASS_COMMANDS.includes(commandName)) { return; }
  ...
}

But the command YAML declares name: ensemble:map-model. If selectModel is called with the full namespaced name (as it should be from metadata), the bypass never fires. A user with an invalid config (e.g., a model ID not in KNOWN_MODEL_IDS) who runs /ensemble:map-model to fix it will get a PreflightError blocking the very command they need to repair their config. The fix is either to include the namespaced forms in BYPASS_COMMANDS or to strip the prefix before checking:

const BYPASS_COMMANDS = ['map-model', 'migrate-model-config', 'ensemble:map-model', 'ensemble:migrate-model-config'];

---

🟡 High

3. selectModel() is a breaking API change with no mention in PR description

model-resolver.js completely rewrites selectModel(command, config, options) to selectModel(commandName, explicitTier, startDir). The functions extractModelPreference, getModelAlias, and resolveModelAlias are also removed from exports. Any callers (internal or external plugins) depending on the old signature will silently break at runtime. This deserves explicit mention in the PR description and a search for callers.

4. Test duplicates the function it's testing

scripts/tests/lint-model-ids.test.js re-implements extractModelValue directly because lint-model-ids.js calls main() on load and can't be require()'d cleanly:

// --- Replicated helpers (must stay in sync with lint-model-ids.js) ---
function extractModelValue(yamlContent) { ... }

The comment "must stay in sync" is a maintenance smell. The fix is to export helpers from the script using a guard:

// in lint-model-ids.js
if (require.main === module) { main(); }
module.exports = { extractModelValue, findYamlFiles, collectYamlFiles };

This lets tests import the real functions instead of duplicating them.

---

🟠 Medium

5. ensemble-model-config-schema.json inconsistent strictness

"tiers": { "additionalProperties": false },  // strict
...
"additionalProperties": true  // root: permissive

A typo like "extarKnownModelIds" at the root level would be silently accepted. Consider "additionalProperties": false at the root, or at minimum list extraKnownModelIds as an "unevaluatedProperties": false guard.

6. legacy-config-migrator.js imports writeConfigAtomic from map-model-wizard.js

The migrator conceptually has no dependency on the interactive wizard; it imports writeConfigAtomic which is a generic file-write utility. This creates an unintuitive coupling. writeConfigAtomic should be moved to a shared utility module (e.g., lib/fs-utils.js) so both the wizard and migrator can import it independently.

7. map-model.yaml has model: medium but is a bypass command

The map-model command sets model: medium in its own YAML. If selectModel is called with the command's metadata tier before the bypass check resolves (depending on call order), this is a chicken-and-egg risk. It's low severity given the bypass should run first, but the self-referential model: medium on a config-repair command is confusing. Consider documenting why it's safe, or setting no model: field on bypass commands.

---

🟢 Low / Suggestions

8. emitFirstRunHint accumulates stale path keys

seen-hints.json uses the absolute project root path as a key. These paths are machine-specific, accumulate indefinitely, and are never pruned. A project moved to a different directory gets a duplicate hint. A hash of the project root, or a relative path from $HOME, would be more portable.

9. Mass YAML regeneration obscures the meaningful diff

~60 agent/command YAML files were bulk-updated solely to rename opus/sonnet/haiku → high/medium/low. These generated changes should ideally be a separate commit from the core logic changes to make the diff easier to review.

---

✅ What's Good

• The new config architecture (project-scoped .claude/ensemble-model-config.json vs old global XDG path) is a cleaner design.
• writeConfigAtomic using tmp + rename is the correct pattern for safe config writes.
• preflightValidate against KNOWN_MODEL_IDS is good defensive programming.
• The DEFAULT_CONFIG with Object.freeze and proper fallback chain in loadConfig is solid.
• The one-time warning/hint sentinel pattern in checkLegacyXdgFile / emitFirstRunHint is a thoughtful UX detail.
• The two stated bug fixes (ask-user.ts TS2322, version assertion 2.12.0→2.13.0) are correct.
• schemas/ensemble-model-config-schema.json is a welcome addition.

---

Summary

The implementation quality of the tiered model aliases feature is generally solid, but items #1 (.beads/ runtime files) and #2 (BYPASS_COMMANDS namespace mismatch) should be fixed before merge — the second one will cause a broken user experience when the feature is most needed. Items #3 and #4 are high priority for maintainability. The PR description needs to accurately reflect that this is a feature PR, not just a bug-fix.

[ldangelo]

Closing — branch was accidentally created from feature/trd-2026-021-tiered-model-aliases instead of main. Recreating with only the two targeted fixes.