feat(setup): update OMO and setup config handling

[marcusrbrown]

No description provided.

[fro-bot]

Good direction — CI-safe config isolation via OPENCODE_CONFIG_CONTENT is the right approach, and the autoupdate: false baseline + user-config merge logic is clean. The tests cover the core cases well.

One blocking issue before this can merge:

❌ opencode-config input not declared in action.yaml

setup.ts reads core.getInput('opencode-config'), but action.yaml has no corresponding opencode-config input entry. Without that declaration, users have no standard way to pass this value — @actions/core reads it from INPUT_OPENCODE-CONFIG env, which requires manual environment wiring rather than the normal with: syntax.

# action.yaml — needs this block added alongside omo-providers
opencode-config:
  description: >-
    JSON object to merge into the OpenCode config (OPENCODE_CONFIG_CONTENT).
    Values are merged on top of the autoupdate:false baseline; user values win on conflicts.
  required: false

Without this, the feature is silently dead for users.

---

Minor observations (non-blocking)

No validation that opencode-config is a JSON object

// If user passes '42' or 'null', JSON.parse succeeds but spreading gives unexpected results:
// {...null}   → {}   (silent, probably fine)
// {...42}     → {}   (silent, probably fine)
// {...["a"]}  → {"0": "a"}  (weird but unlikely)

The outer catch will handle a SyntaxError from invalid JSON, but you might want a guard:

const parsed = JSON.parse(inputs.opencodeConfig) as unknown
if (parsed == null || typeof parsed !== 'object' || Array.isArray(parsed)) {
  core.setFailed(`opencode-config must be a JSON object`)
  return null
}

Or at minimum a test for invalid JSON input (e.g., 'not-json' → should call setFailed).

Test key ordering in the merge assertion

The test at line 269 relies on JSON.stringify key ordering being stable. In practice this is deterministic in V8 (insertion order), and the source code produces {autoupdate: true, model: 'claude-opus-4-5'} which matches the test expectation — just worth being aware of if more keys are added later.

---

Fixing action.yaml unblocks this. The implementation itself is solid.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	22514311495
Cache	hit
Session	ses_35d45d9eeffeeKR2T7er9nh6HE

[fro-bot]

All three items from the previous review have been addressed. Approving.

✅ Blocking issue resolved

action.yaml now declares the opencode-config input with the correct description. The feature is fully wired for standard with: usage.

✅ Object-type validation added

setup.ts now validates that the parsed value is a plain object before merging:

if (parsed == null || typeof parsed !== 'object' || Array.isArray(parsed)) {
  core.setFailed('opencode-config must be a JSON object')
  return null
}

This handles all the edge cases called out in the previous review: null, arrays, numbers, and bare strings all correctly fail with a clear message.

✅ Tests

The test coverage is solid:

• setup.test.ts: baseline export, user-config merge (with key-override semantics), and four rejection cases (null, array, number, string)
• inputs.test.ts: invalid JSON syntax rejection, valid JSON object pass-through, empty → null

One nit (not blocking)

parseSetupInputs() uses || null for the opencode-config field whereas the rest of the codebase uses .trim().length > 0 ? ... : null. In practice @actions/core trims values before returning them, so whitespace-only input would parse as empty string and || null would give the right result — but it's slightly inconsistent with the style elsewhere. Fine to leave as-is.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	22539063008
Cache	hit
Session	ses_357948813ffetiTb6gOsr027e9

[fro-bot]

Previously approved — nothing new to flag on this pass.

All CI checks are green (Build, Lint, Test, CodeQL). Two jobs (Test GitHub Action, Release) were still in-progress at review time but are expected to complete normally.

The implementation is complete:

• opencode-config input declared in action.yaml
• JSON syntax validated in parseActionInputs(); plain-object constraint enforced in runSetup()
• OPENCODE_CONFIG_CONTENT correctly exports {autoupdate: false} merged with any user-supplied overrides
• --skip-auth flag present in oMo installer args
• Test coverage covers all edge cases (baseline, merge, null/array/number/string rejection, invalid JSON syntax)

Ready to merge.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	22554847669
Cache	hit
Session	ses_3545f1716ffeNe19RDfC7gQ87g

[fro-bot]

The only change since the last review is commit 1f9f2c4 — "feat(setup): add kimi-for-coding to oMo provider inputs (#268)". Clean addition, nothing to flag.

✅ kimi-for-coding provider

Follows the existing pattern precisely:

• kimiForCoding?: 'no' | 'yes' added to OmoInstallOptions
• VALID_OMO_PROVIDERS array and action.yaml description both updated
• case 'kimi-for-coding' and return-value field in parseOmoProviders complete
• --kimi-for-coding=${kimiForCoding} appended to bunx oh-my-opencode install args
• All three existing provider tests updated with --kimi-for-coding=no in expected arg lists; new test exercises kimiForCoding: 'yes'; positional test confirms --skip-auth placement

CI: Build, Lint, Test, CodeQL green. Release + Test GitHub Action still pending but expected to complete normally.

Previously approved — this commit doesn't change that assessment.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	22555066019
Cache	hit
Session	ses_354548bd5ffeaeC0LbC67qY8FD

[fro-bot]

The new d6dbbb1 commit (omo-config action input) is well-implemented. However, reviewing the full diff surfaces a bug introduced in 1f9f2c4 that I missed on the previous pass.

❌ kimi-for-coding unusable via omo-providers input

kimi-for-coding is listed as a valid value in action.yaml and handled in setup.ts's parseOmoProviders — but it's absent from inputs.ts:

// src/lib/inputs.ts — kimi-for-coding is missing
const VALID_OMO_PROVIDERS = [
  'claude', 'claude-max20', 'copilot', 'gemini', 'openai', 'opencode-zen', 'zai-coding-plan'
] as const

parseActionInputs() runs before runSetup() in main.ts (step 1 vs step 2 in the orchestration flow) and throws on unknown providers:

throw new Error(`Invalid omo-providers value: "${provider}". Valid values: ...`)

A user who sets omo-providers: kimi-for-coding in their workflow gets:

Invalid inputs: Invalid omo-providers value: "kimi-for-coding". Valid values: claude, claude-max20, copilot, gemini, openai, opencode-zen, zai-coding-plan

…and the action exits before setup even runs. The provider is advertised in action.yaml but fails at input validation.

Three files need updating:

1. src/lib/inputs.ts — add 'kimi-for-coding' to VALID_OMO_PROVIDERS and add a kimiForCoding variable + case 'kimi-for-coding' in parseOmoProviders
2. src/lib/types.ts — add readonly kimiForCoding: 'no' | 'yes' to OmoProviders
3. src/lib/inputs.test.ts — add kimi-for-coding to the providers parse tests

---

✅ omo-config feature (d6dbbb1)

Clean, well-structured addition:

• deepMerge() is pure, non-mutating, correct semantics (source wins on conflict, arrays replaced)
• writeOmoConfig() handles missing/corrupt existing config gracefully (falls back to empty base)
• Written before installer runs so oMo can observe custom settings — ordering is correct
• Failure degrades to a logger.warning, doesn't abort setup — right call for an optional feature
• 6 unit tests for deepMerge, 8 integration tests for writeOmoConfig using real tmpdir I/O

---

Minor (non-blocking)

writeOmoConfig doesn't validate the parsed JSON is a plain object.
The same guard applied to opencode-config in setup.ts would be appropriate here:

const userConfig = JSON.parse(configJson) as unknown
if (userConfig == null || typeof userConfig !== 'object' || Array.isArray(userConfig)) {
  throw new Error('omo-config must be a JSON object')
}

Without it, omo-config: '[1,2,3]' silently produces array-index keys ("0", "1", "2") in the merged file. Since any error here already triggers a warning-and-continue, blast radius is low — but symmetry with the opencode-config handling would be cleaner.

deepMerge test name mismatch: "handles null source gracefully" (line 46) passes {}, not null. Test is correct, name is misleading.

---

Fixing the inputs.ts/types.ts gap is all that's needed to unblock kimi-for-coding. The omo-config feature ships clean.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	22555291280
Cache	hit
Session	ses_354496fb4ffeX2kT8c7wHKJtLO

[fro-bot]

The blocking issue from the previous review is resolved. Approving.

✅ kimi-for-coding input validation gap — fixed

a0fda79 correctly fills in both missing pieces:

• 'kimi-for-coding' added to VALID_OMO_PROVIDERS in inputs.ts — the provider is now accepted at input-validation time
• readonly kimiForCoding: 'no' | 'yes' added to OmoProviders in types.ts — the type is complete

Users who set omo-providers: kimi-for-coding in their workflow will no longer hit the "Invalid inputs" failure at step 1.

Nits (non-blocking, carry-overs)

inputs.test.ts still has no omo-providers coverage for kimi-for-coding

The previous review asked for this. omo.test.ts does cover the installer-side (kimiForCoding: 'yes' → --kimi-for-coding=yes), but the parseActionInputs path — where the earlier failure actually occurred — is still untested for this provider. A simple addition would close the gap:

it('parses kimi-for-coding provider', () => {
  mockGetInput.mockImplementation((name: string) => {
    const inputs: Record<string, string> = {
      'github-token': 'ghp_test123',
      'auth-json': '{"anthropic":{"type":"api","key":"sk-ant-test"}}',
      'omo-providers': 'kimi-for-coding',
    }
    return inputs[name] ?? ''
  })
  const result = parseActionInputs()
  expect(result.success).toBe(true)
  expect(result.success && result.data.omoProviders.kimiForCoding).toBe('yes')
})

Fine to add in a follow-up.

writeOmoConfig plain-object guard still absent (flagged in the previous review — low blast radius since errors fall back to a warning, but symmetry with opencode-config handling would be cleaner)

deepMerge test name mismatch — "handles null source gracefully" (line 46) passes {}, not null. Cosmetic.

---

All three items are follow-up material, not blockers. The feature is correctly wired end-to-end.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	22555856949
Cache	hit
Session	ses_3542cfb77ffe2PAx6JQ2VMOeoD

[Copilot]

Pull request overview

This PR extends the action’s setup pipeline to support additional oMo provider configuration, adds configurable oMo/OpenCode config inputs, and improves CI reliability for oMo installation.

Changes:

• Add kimi-for-coding support to oMo provider parsing and installer flags, plus --skip-auth for headless CI installs.
• Introduce omo-config (written to oh-my-opencode.json before running the installer) and opencode-config (merged into an autoupdate:false baseline and exported via OPENCODE_CONFIG_CONTENT).
• Add/extend Vitest coverage for the new config behaviors and installer argument changes.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/lib/types.ts	Extends ActionInputs and OmoProviders to include opencodeConfig and kimiForCoding.
src/lib/setup/types.ts	Adds omoConfig to setup input typing.
src/lib/setup/setup.ts	Writes omo-config before oMo install and exports merged OPENCODE_CONFIG_CONTENT.
src/lib/setup/setup.test.ts	Adds tests for OPENCODE_CONFIG_CONTENT export and omo-config behavior.
src/lib/setup/omo.ts	Adds --skip-auth and --kimi-for-coding installer args.
src/lib/setup/omo.test.ts	Updates installer arg expectations and adds coverage for new flags.
src/lib/setup/omo-config.ts	New helper to deep-merge and write oh-my-opencode.json.
src/lib/setup/omo-config.test.ts	Unit tests for deep merge and config writing behavior.
src/lib/inputs.ts	Parses/validates new opencode-config input and adds kimi-for-coding provider option.
src/lib/inputs.test.ts	Adds tests for opencode-config parsing/validation.
src/lib/agent/opencode.test.ts	Updates test configs to include the new kimiForCoding provider flag.
action.yaml	Documents new inputs (opencode-config, omo-config) and updates provider list.
README.md	Documents omo-config input (but not opencode-config).

[Copilot]

deepMerge assigns arbitrary keys from user-controlled JSON onto a plain object. Keys like __proto__, constructor, or prototype can trigger prototype mutation on assignment and produce unexpected results. Consider creating the merge result with a null prototype and/or explicitly rejecting these special keys during merge.

[Copilot]

writeOmoConfig() assumes the parsed omo-config JSON is a plain object via a type assertion. Inputs like null, arrays, or primitives will either throw later (e.g., null passed to deepMerge) or silently produce unexpected merges. Consider validating userConfig is a non-null, non-array object right after JSON.parse and throwing a clear error if it isn’t.

Suggested change

	const userConfig = JSON.parse(configJson) as Record<string, unknown>
	const parsedUserConfig: unknown = JSON.parse(configJson)
	if (parsedUserConfig == null || typeof parsedUserConfig !== 'object' || Array.isArray(parsedUserConfig)) {
	throw new Error('omo-config must be a JSON object (non-null, non-array)')
	}
	const userConfig = parsedUserConfig as Record<string, unknown>

[Copilot]

inputs.opencodeConfig is parsed with JSON.parse without a local try/catch. If the input is malformed JSON (or even whitespace, since the input isn’t trimmed), JSON.parse will throw and the function will fall into the outer catch with a generic failure message rather than a clear validation error. Consider trimming the input and catching JSON parse errors here to call core.setFailed with a specific message (and add a test for invalid JSON).

[Copilot]

parseSetupInputs() reads opencode-config / omo-config with core.getInput(...) || null and does not trim. This means whitespace-only values are treated as provided, which can later trigger JSON parsing errors (opencode-config) or a warning path (omo-config) unexpectedly. Prefer core.getInput(...).trim() and convert empty string to null consistently (as done for other inputs in this file).

[Copilot]

parseActionInputs() only validates that opencode-config is valid JSON, but action.yaml documents it as a JSON object to merge into config. As written, values like 42, "str", or null will pass validation here but later cannot be merged as an object. Consider validating that the parsed JSON is a plain object (non-null, non-array) to align runtime behavior with the documented input contract.

Suggested change

	validateJsonString(opencodeConfig, 'opencode-config')
	validateJsonString(opencodeConfig, 'opencode-config')
	const parsedOpencodeConfig = JSON.parse(opencodeConfig)
	const isObject = typeof parsedOpencodeConfig === 'object'
	const isNull = parsedOpencodeConfig === null
	const isArray = Array.isArray(parsedOpencodeConfig)
	if (!isObject || isNull || isArray) {
	throw new Error("Input 'opencode-config' must be a JSON object")
	}

[Copilot]

The README inputs table was updated to document omo-config, but this PR also introduces the opencode-config input in action.yaml and it is not documented anywhere in the README. Please add a corresponding row so users can discover and correctly format the new input.

Suggested change

	| `omo-config` | No | — | Custom oMo configuration JSON (deep-merged) |
	| `omo-config` | No | — | Custom oMo configuration JSON (deep-merged) |
	| `opencode-config` | No | — | Custom OpenCode configuration JSON (deep-merged) |

[Copilot]

This test title says “opencode-config is null”, but the mocked input is the literal JSON string "null" (i.e., a provided value that parses to null). Renaming the test to reflect the actual scenario will make failures easier to interpret.

[Copilot]

The test name “handles null source gracefully” doesn’t match the behavior being exercised (it passes {} as the source, not null). Rename the test or adjust the case to avoid misleading coverage assumptions.

[fro-bot]

The 619abdfa commit addresses all carry-over nits from the previous review. Approving.

✅ All carry-over nits resolved

writeOmoConfig plain-object guard — isMergeableObject guard added; null, arrays, and primitives now throw with a clear message before attempting the merge. Symmetric with the opencode-config handling in setup.ts. ✅

deepMerge test name mismatch — "handles null source gracefully" → "returns target unchanged when source is empty object". Test now describes what it actually tests. ✅

parseSetupInputs() style inconsistency — || null replaced with .trim().length > 0 ? ... : null for all four inputs (app-id, private-key, opencode-config, omo-config), consistent with the rest of the codebase. ✅

opencode-config missing from README table — Row added. ✅

✅ Additional hardening (good additions)

Prototype pollution fix — UNSAFE_MERGE_KEYS = new Set(['__proto__', 'prototype', 'constructor']) filters in both target and source iterations. Object.create(null) as the accumulator means the merged result has no prototype chain of its own. Test at the new "ignores prototype pollution keys during merge" case verifies Object.prototype is unaffected. Correct and well-tested.

Explicit JSON parse error path in setup.ts — The try/catch around JSON.parse in runSetup now produces 'opencode-config must be valid JSON' for syntax errors, distinct from the 'opencode-config must be a JSON object' message for type errors. Clear signal to the user either way. Test added.

Whitespace-only inputs — opencode-config: ' ' and omo-config: ' ' now correctly treated as not provided; tests verify the baseline OPENCODE_CONFIG_CONTENT export and that no oMo config write occurs. ✅

Object-type validation in inputs.ts — parseActionInputs now validates JSON is a plain object (not just valid syntax), closing the gap where a caller could bypass the setup.ts layer. Three inputs.test.ts cases added for null, array, and string literals.

One note

Object.create(null) in deepMerge result — The returned object has no prototype chain methods (.toString(), .hasOwnProperty(), etc.). Safe for all current usage (JSON.stringify enumerates own properties correctly), but worth keeping in mind if deepMerge is ever called where the caller expects a regular object. Not a concern today.

Remaining carry-over nit (follow-up)

inputs.test.ts still has no parseActionInputs path coverage for kimi-for-coding as a provider — only the installer side (omo.test.ts) is exercised. Mentioned in the last two reviews. Fine to add in a follow-up PR.

---

All blocking issues resolved. Ready to merge.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	22560141693
Cache	hit
Session	ses_353688fdaffeXZvKA5OTdLSt2O