fix(sdd): parse model spec at first separator and consolidate duplicated parsers

[Basparin]

🔗 Linked Issue

Closes #260

---

🔄 Continuation Notice

This PR continues and supersedes #242 by @ACDPDEV, who explicitly handed off the work in #242 (comment) (university commitments prevented him from landing the review suggestions himself).

Full credit for the original fix goes to @ACDPDEV — his commit 28c3378 is preserved verbatim with original author metadata intact. This PR applies @Alan-TheGentleman's two review suggestions from #242 on top:

1. Shared SplitProviderModel helper — consolidates three identical copies of the first-separator logic. This matches @ACDPDEV's final ask in the handoff: "abstraer lo que se repite en una función".
2. Regression test for OpenRouter /free format + dedicated coverage for extractModelFromAgent, which previously had no direct tests.

Once this PR lands, #242 can be closed — the full contribution (with review feedback applied) lives here.

---

🏷️ PR Type

• type:bug — Bug fix (non-breaking change that fixes an issue)
• type:feature
• type:docs
• type:refactor
• type:chore
• type:breaking-change

---

📝 Summary

• Fixes OpenRouter free-model parsing where openrouter/qwen/qwen3.6-plus:free was split at : before the leading /, producing a broken ProviderID=openrouter/qwen/qwen3.6-plus, ModelID=free.
• Consolidates three identical copies of the provider/model split logic into a single exported SplitProviderModel helper in the model package.
• Adds regression coverage for the OpenRouter format and fills the coverage gap around extractModelFromAgent.

---

📂 Changes

File / Area	What Changed
internal/model/split.go	New: exported SplitProviderModel(spec) (provider, model, ok) helper
internal/model/split_test.go	New: 11 cases including OpenRouter regression and malformed inputs
internal/cli/sync.go	parseModelSpec now delegates to the shared helper
internal/components/sdd/profiles.go	extractModelFromAgent now delegates to the shared helper
internal/components/sdd/read_assignments.go	Inline parser replaced with a call to the shared helper
internal/components/sdd/profiles_test.go	Adds TestExtractModelFromAgent — no previous coverage
internal/components/sdd/read_assignments_test.go	Adds TestReadCurrentModelAssignmentsOpenRouterFreeSuffix regression test

Net: +255 / −41, 7 files. No behavior change in the happy path — existing TestReadCurrentModelAssignments* suite still passes unchanged.

---

🧪 Test Plan

Unit Tests

go test ./internal/model/... ./internal/cli/... ./internal/components/sdd/...

E2E Tests (Docker required)

Not run

Pre-existing test failures in the Windows sandbox around unique-names-generator / bun install are unrelated to this PR — they're tracked in a separate PR (#372).

• Unit tests pass on affected packages
• E2E tests pass (cd e2e && ./docker-test.sh)
• Parsing logic verified via new unit tests across edge cases

---

🤖 Automated Checks

Check	Status	Description
Check Issue Reference	⏳	Closes #260
Check Issue Has status:approved	⏳	Verified: #260 carries status:approved
Check PR Has type:* Label	⏳	Requesting type:bug
Unit Tests	⏳	CI will validate
E2E Tests	⏳	CI matrix will validate

---

✅ Contributor Checklist

• PR is linked to an issue with status:approved (Configure Opencode Models writes invalid model ID prefix (opencode/ instead of openrouter/) #260)
• I have added the appropriate type:* label to this PR (pending maintainer — external contributors cannot apply labels)
• Unit tests pass
• My commits follow Conventional Commits format
• My commits do not include Co-Authored-By trailers

---

💬 Notes for Reviewers

• Original author preserved: first commit (28c3378) is @ACDPDEV's verbatim, author metadata intact. GitHub should surface him as a co-contributor via commit history.
• Two new commits on top apply @Alan-TheGentleman's review from fix(sdd): parse model string on first separator to fix OpenRouter /free bug #242: refactor(model): extract SplitProviderModel helper... and test(sdd): add OpenRouter free suffix regression coverage.
• Helper placement: chose internal/model because ModelAssignment already lives there and both cli and sdd packages import it — zero import-cycle risk.
• Closing fix(sdd): parse model string on first separator to fix OpenRouter /free bug #242: once this lands, fix(sdd): parse model string on first separator to fix OpenRouter /free bug #242 is safe to close. Thanks to @ACDPDEV for the clean original fix and for trusting the continuation.

[ACDPDEV]

Gracias por la mención, estoy ansioso por revisar esta PR

[Alan-TheGentleman]

This is a good candidate to merge after a current CI pass.

The parser extraction is scoped and the regression coverage hits the OpenRouter /free case, but the branch has old validation noise in the history. Please refresh against current main so we merge a clean branch.

[Alan-TheGentleman]

Looks good. The shared parser fixes the OpenRouter /free regression without widening behavior, and the new tests cover the mixed slash/colon cases that matter.

[Alan-TheGentleman]

CI is blocked on TestReadCurrentModelAssignmentsOpenRouterFreeSuffix: sdd-orchestrator is missing from the parsed assignments after the latest main update. Please update the parser or test fixture so OpenRouter /free specs still keep the phase assignment. Once that passes, this one is back in merge shape.

[Basparin]

@Alan-TheGentleman pushed the fix you flagged → commit f972839.

Fix

The regression test was asserting got["sdd-orchestrator"] but, after the alias-normalization that landed in main, ReadCurrentModelAssignments now maps the legacy sdd-orchestrator config key to gentle-orchestrator in the returned map. Updated the assertion accordingly while keeping the legacy key in the JSON fixture — that way the test now exercises both invariants in a single real-world scenario (un-synced config + first-separator split). Production code untouched.

Local validation

go test ./internal/components/sdd/... ./internal/model/...

All relevant tests pass:

• TestReadCurrentModelAssignmentsOpenRouterFreeSuffix ✅ (the one you flagged)
• All other TestReadCurrentModelAssignments* (8 tests) ✅
• TestSplitProviderModel (11 cases) ✅
• TestExtractModelFromAgent (9 cases) ✅

Remaining CI red is not from this PR

CI Unit Tests is still failing on the latest run (job 75265388060), but the failing test is TestRunInstallEngramForPiAndOpenCodeProvisionsBothMCPTargets in internal/cli/run_integration_test.go — error: Pi requires the pi executable in PATH.

Evidence this is upstream, not this PR:

1. The test doesn't exist on this branch — git grep -l TestRunInstallEngramForPiAndOpenCodeProvisionsBothMCPTargets returns nothing on fix/260-openrouter-free-continuation. It comes in via CI's auto-merge with upstream/main.
2. This branch doesn't touch internal/cli/ or internal/agentbuilder/ — git log upstream/main..HEAD -- internal/agentbuilder/ is empty.
3. Main itself is red on the same test — the last two pi commits on main (f315c05 fix(pi): write packages as array, d1027c4 fix(pi): wire engram mcp adapter) both failed CI with the same error. Runs: 25641715595, 25641492770.

Looks like the Pi test needs either a skip when pi isn't on PATH or the binary installed in the runner. Once main is green again, this PR should pass without further changes from my side. Happy to help with the Pi test fix too if useful — let me know.

[Alan-TheGentleman]

@Basparin — heads-up: this PR's Unit Tests failure is TestRunInstallEngramForPiAndOpenCodeProvisionsBothMCPTargets. That test was rewritten in main by PR #660 (fix(agents/pi): use positional pnpm dlx argument, landed in v1.31.0) to expect the new pnpm dlx gentle-engram@X pi-engram init form instead of the old pnpm dlx --package ... form.

Your branch still expects the old form, so the CI run uses the pre-#660 assertion and fails.

Fix: rebase against current main — the conflict should resolve cleanly (your changes are in different files than what #660 touched, but the integration test now has the new expectation). After the rebase, this specific failure goes away.

Pseudocode:

git fetch upstream main
git rebase upstream/main
# resolve any conflicts (likely none for this PR's scope)
git push --force-with-lease

The other PR you have open (and #371 + #374 + #372) all show the same failure for the same reason — same rebase fixes all three. Thanks for sticking with these.

[Alan-TheGentleman]

Thanks for picking this up and preserving @ACDPDEV's original commit — the refactor is clean and the new tests are solid. The multi-slash openrouter/anthropic/claude-sonnet-4 case and the :free regression are exactly right, and everything passes locally with no conflicts against main.

One blocker before merge: this fixes the wrong layer of #260.

The issue's symptom is the TUI writing opencode/qwen3.6-plus-free into opencode.json. That string is built at inject.go:1884 from assignment.FullID() (ProviderID + "/" + ModelID), and ProviderID comes from state.SelectedProvider in model_picker.go:272 — i.e. the model is being offered under the wrong provider in the catalog (the issue's own "Additional finding": it shows under OpenCode Zen but only works under OpenRouter).

Your change only fixes parsing model strings read back from config — a real and worthwhile bug — but it doesn't stop the bad prefix from being written. So Closes #260 would auto-close an issue whose main symptom is still present.

Could you either:

• (a) narrow the scope: change Closes #260 → Refs #260 (and link the colon-first parse bug it actually fixes), or
• (b) extend the PR to address the catalog/provider-mapping write path so #260's repro actually produces openrouter/...:free?

Small nit: gofmt on split_test.go to align the struct columns.