Roadmap: 12 PRs across 12 issues — bug fixes, regression locks, two new features #4293
YOMXXX
started this conversation in
Show and tell
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Hi maintainers and reviewers — over the past few sessions I worked through 12 of the open issues and opened 11 PRs against
code-yeongyu/oh-my-openagent(plus 1 against thelsp-tools-mcpsubmodule). This discussion is a single landing page so you can scan all of them at a glance, see the rationale, and pick a review order that makes sense for you.I tried to follow systematic-debugging (root cause before fixes) and TDD (red-green for every actual bug fix) throughout. Each PR is independent, scoped to one issue, and small enough to review in isolation — but the table below also groups them so you can batch similar concerns together.
Roadmap at a glance
A. True bug fixes (TDD red-green for each)
syncskill resolvers &getSkillByNamealign with async (short-name + case-insensitive + ambiguity protection)skill-template-resolver.ts,loader.ts,executor.ts,skill-matcher.ts(genericized)package.jsonbundled-version.ts,hook.tsbun buildemitsdist/locales/{en,zh,index}.jsso the published tarball self-documents (locale data is also still inlined indist/index.js)package.jsonbuild script[search-mode],<hyperplan-mode>, etc.) is already in the text — fixes/undo+ resend double-bannerkeyword-detector/hook.ts.bat/.cmdbefore the literal name, sojdtls+jdtls.batresolves to the wrapper instead of the shebang scriptpackages/lsp-tools-mcp/src/lsp/process.tsEvery one of these has a regression test that fails on
dev-as-of-issue and passes after the fix. I verified the red-green cycle locally for each.B. Regression locks — source already fixed, tests were missing
These are issues where investigation revealed the symptom had already been fixed in source (commits like
ed6e9955for path-separator normalization, the existing Zod enums forreasoningEffort). The reporters were likely on stale dist artifacts. To prevent the bugs from quietly drifting back, each PR adds focused regression tests that pin the existing contract.hasCliSuffixregression case using the literal Windows-style ast_grep cli path from the bug reportreasoningEffort: "max"onCategoryConfigSchema/AgentOverrideConfigSchema/FallbackModelObjectSchema+ a doctor end-to-end testcli-suffixregression mirroring the lsp-tools-mcp candidate paths in the issue's resolution simulationapplyMcpConfigidempotency tests that survive simulated/newconfig resetsNone of these PRs change production code. They're test-only, ~0 risk to merge.
C. New capabilities
doctorwarns when a custom provider model is missinglimit.context(auto-compaction would never trigger). 7 new unit tests cover the matcher; wired into existingcheckModels.excluded_agentsconfig — custom lightweight agents (e.g.cybersec) keep working but omo skips its 3 most invasive runtime injectors for them (ultrawork tag, keyword-mode banners, AGENTS.md/README/rules tool-output injection). Safety/recovery hooks still run.@modelcontextprotocol/server-memory. Stable default path under$XDG_CACHE_HOME/~/.cache/opencode/so memory survives switching projects;OMO_MEMORY_FILE_PATHenv override; users can opt out viadisabled_mcps: ["memory"]and bring their own.Suggested review order
If you have ~20 minutes, the highest-value pass is the regression locks (B) — pure test additions, very fast to merge, lock down hard-won fixes.
If you have more time, A then C is the natural order: bug fixes first to harden the baseline, then features on top.
Within A:
Within C:
disabled_mcps).A note on two reports where the root cause differed
Two of the original reporter root-cause analyses turned out to be off, but the PRs still ship useful work — I flagged this explicitly in each PR so reviewers don't merge based on a wrong assumption:
provider/modelintoopencode.json. It does not — users own that file. The fix shape changed from "populatelimitautomatically" (would silently override user intent) to "doctor surfaces the missing field with a concrete remediation string." Same problem solved, no mutation.applyMcpConfighad stateful drift on/new. The function is actually already idempotent. Whether OpenCode re-invokes theconfighook on/newis upstream behavior we can't reach from a plugin event handler. The PR locks the omo-side contract so a future refactor can't introduce the bug we don't currently have.Happy to split, rebase, or close any of these if direction doesn't match. Thanks for the project.
All reactions