Clean up stale plugin inputs and low-value tests#135
Conversation
There was a problem hiding this comment.
Decision: APPROVED
What I verified
- PR body first line is exactly
Ref: #134✅ - Branch is
issues/orfe-134✅ - Commit
960dc5dis the tip of the branch ✅ npm run build— clean ✅npm run typecheck— clean ✅npm run lint— clean ✅npm test(targeted: changed files) — 37/37 pass ✅test/plugin.test.ts,test/wrapper.test.ts,test/package.test.tssrc/commands/registry/index.test.ts,src/body-contracts.test.ts
Changes reviewed
.gitignore + eslint.config.js
Both correctly gain /.worktrees / .worktrees/**. Addresses AC: lint no longer scans unintended worktree build artifacts. ✅
src/plugin.ts
config and auth_config added to the plugin args schema. This closes the gap: previously the wrapper already read and forwarded these fields, and the help metadata already exposed them via COMMON_CLI_OPTIONS, but the plugin schema silently discarded them at the OpenCode boundary. Now all three surfaces are consistent. ✅
src/wrapper.ts (unchanged in this PR — pre-existing)
readWrapperPathInput already handles config / auth_config correctly. No changes needed here. ✅
docs/orfe/spec.md + README.md
Spec now documents config and auth_config as wrapper-level path overrides. README now distinguishes published package vs local checkout plugin config. Both docs updates are accurate and minimal. ✅
Deleted src/commands/*/definition.test.ts files (15 files)
These were pure metadata-only tests checking hardcoded .name, .group, .leaf, validInputExample, and successDataExample fields. Not behavior tests.
Behavioural validation logic that was co-located in some of these files (e.g. issue set-state validate constraints, issue update mutation-option guard, pr submit-review event enum check) is fully covered at the CLI and core layer:
test/cli.test.ts:4408— state_reason validation error pathtest/cli.test.ts:2773,4452— pr submit-review dismiss rejectiontest/cli.test.ts:3904— issue update at-least-one-mutation guardtest/core.test.ts:3459–3469— pr submit-review dismiss rejection at core layer
Removal is safe. ✅
test/package.test.ts
Refocused on the public contracts that actually matter for publishing: main, exports['.'], exports['./plugin'], bin.orfe, files. The removed metadata (license string, description regex, publishConfig registry, scripts.prepack, etc.) is redundant with the file itself being the source of truth. ✅
test/plugin.test.ts
New behavior test (plugin exposes common path override inputs on the orfe tool) directly instantiates the plugin and checks that config and auth_config are present in orfeTool.args. This is exactly the right kind of test to replace the deleted metadata-only plugin entry points export a plugin function test. ✅
Also: the project set-status help test now additionally asserts that supported_input_fields includes config and auth_config, verifying the end-to-end consistency claim. ✅
test/wrapper.test.ts
Renamed test now supplies config and auth_config and asserts that capturedRequest includes configPath and authConfigPath forwarded to core. Good behavioral coverage for the path-override forwarding. ✅
Important (non-blocking, recommend follow-up)
./server export in package.json is no longer tested and its intent is undocumented.
package.json still exports "./server": "./dist/plugin.js" alongside the current "./plugin" entry point. The removed test in package.test.ts previously asserted this. If ./server is an intentional backward-compat alias (for OpenCode v1.x plugin resolution), it should stay and a brief comment in package.json or the spec would make that explicit. If it is stale, it should be removed (along with the old test assertion). In either case, the current state leaves it silently untested. Recommend a follow-up to either document the alias as intentional or remove it.
This does not block QA since the export is not broken and the issue scope explicitly notes "preserve intentional compatibility aliases if any are still required."
Blockers
None.
Nice to have
- None identified.
Docs / invariants
docs/orfe/spec.md updated for the config/auth_config inputs. Appropriate and accurate. No architecture invariants or ADRs affected by this cleanup change.
Ref: #134
Summary
Testing