Skip to content

refactor(schema): extract tool options definition#9649

Merged
jdx merged 2 commits into
jdx:mainfrom
risu729:codex/schema-tool-options
May 10, 2026
Merged

refactor(schema): extract tool options definition#9649
jdx merged 2 commits into
jdx:mainfrom
risu729:codex/schema-tool-options

Conversation

@risu729

@risu729 risu729 commented May 6, 2026

Copy link
Copy Markdown
Contributor

Why

#9582 restored the schema pattern we wanted for task objects: shared property definitions composed with allOf, then closed with unevaluatedProperties. With Tombi pinned to a version that handles evaluated properties correctly, we can use the same pattern for tool option objects.

The top-level [tools] schema and task tools schema both describe the same { version, os } object shape, but with different extra-property rules. Duplicating the common part makes it easy for those two locations to drift. This PR extracts the shared version/os piece and lets each call site keep its own closure behavior.

What changed

  • Adds a shared $defs.tool_options schema for the common version and os fields.
  • Reuses it from both top-level tool options and task tool options.
  • Preserves the important difference between call sites: root tool objects still allow backend-specific values, while task tool objects still only allow string extras.
  • Extends the Tombi regression test so warnings fail and the shared-schema behavior is covered.

Validation

  • git diff --check
  • Direct Tombi validation with tombi@0.9.22 and --error-on-warnings for valid root/task tool options and invalid task tool extra options.
  • Full e2e should run in CI; my earlier local targeted e2e run was stopped after the build step stalled behind another active cargo build on this machine.

This PR was generated by an AI coding assistant.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the project's JSON schemas and the schema generation script to use "unevaluatedProperties": false instead of "additionalProperties": false. This refactoring allows for better property sharing via $refs in task definitions, leveraging improvements in Tombi v0.9.19+. The E2E tests have been updated to use a pinned Tombi version and include new test cases for validating nested properties like backend_options and profile. I have no feedback to provide as there were no review comments.

@greptile-apps

greptile-apps Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts the shared version/os tool options into a reusable $defs/tool_options definition, replacing duplicated inline properties in both mise.json and mise-task.json with allOf + $ref composition and migrating from additionalProperties to unevaluatedProperties for correct JSON Schema 2019-09+ $ref evaluation.

  • Schema refactoring: tool_options is now a single shared $defs entry; root tool objects use allOf with tool_options plus their install_env/depends/postinstall-specific properties, while task tool objects use a single-entry allOf. Both use unevaluatedProperties rather than additionalProperties.
  • Intentional schema expansion for root tools: The unevaluatedProperties for top-level [tools] entries now permits array and object extra-property types in addition to the previous string|number|boolean — enabling backend-specific nested config like backend_options = { nested = true }. This is validated by the updated mise-os.toml fixture, but the PR description's claim of "existing extra-property behavior is preserved" is slightly misleading given this widening.
  • Improved e2e coverage: --error-on-warnings is added to all tombi lint calls, and a new mise-bad-task-tool-extra.toml fixture asserts that object-valued extras are correctly rejected for task tools.

Confidence Score: 5/5

Safe to merge — a structural schema refactoring with deliberately expanded root-tool permissiveness that is covered by explicit tests.

The refactoring is logically consistent across both schema files: unevaluatedProperties correctly inherits evaluated properties from $ref targets under allOf, required: ["version"] inside tool_options propagates through the composition, and the task-tool path still rejects non-string extras as before. The only substantive behavior change — allowing array/object backend extras on root tools — is intentional and validated by the updated e2e fixtures. No schema constraints are accidentally dropped or inverted.

No files require special attention. The schema files are consistent with each other and the test script fully covers the new permissiveness boundaries.

Reviews (3): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile

@risu729

risu729 commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

Greptile note on the root tool extra-property types looks stale against the actual stack/current main. upstream/main:schema/mise.json already allows string | number | boolean | array | object for root tool extra properties; this PR preserves that set by moving the same schema under unevaluatedProperties around the new $defs.tool_options ref. No code change needed.

This comment was generated by an AI coding assistant.

@risu729

This comment was marked as outdated.

@risu729

This comment was marked as outdated.

@risu729 risu729 force-pushed the codex/schema-tool-options branch from e780ea1 to 4897354 Compare May 6, 2026 23:45
@risu729 risu729 marked this pull request as ready for review May 7, 2026 00:23
@jdx jdx merged commit f0baa08 into jdx:main May 10, 2026
35 checks passed
@risu729 risu729 deleted the codex/schema-tool-options branch May 10, 2026 13:28
mise-en-dev added a commit that referenced this pull request May 11, 2026
### 🚀 Features

- **(cli)** add minimum release age flag to lock and ls-remote by
@risu729 in [#9269](#9269)
- **(config)** add run field for hooks by @risu729 in
[#9718](#9718)
- **(github)** add native oauth token source by @jdx in
[#9654](#9654)
- **(oci)** scope build to project config by default by @jdx in
[#9766](#9766)
- add support for prefixed latest version queries in outdated checks by
@roele in [#9767](#9767)

### 🐛 Bug Fixes

- **(activate)** guard bash chpwd hook under nounset by @risu729 in
[#9716](#9716)
- **(backend)** date-check latest stable fast path by @risu729 in
[#9650](#9650)
- **(config)** parse core tool options consistently by @risu729 in
[#9742](#9742)
- **(exec)** propagate __MISE_DIFF so nested mise recovers pristine PATH
by @jdx in [#9765](#9765)
- **(forgejo)** include prereleases when opted in by @risu729 in
[#9717](#9717)
- **(github)** avoid caching empty release assets by @risu729 in
[#9616](#9616)
- **(java)** resolve lockfile URLs from metadata by @risu729 in
[#9719](#9719)
- **(lock)** cache unavailable github attestations by @risu729 in
[#9741](#9741)
- **(pipx)** preserve options when reinstalling tools by @risu729 in
[#9663](#9663)
- **(python)** skip redundant lockfile provenance verification by
@risu729 in [#9739](#9739)
- **(vfox)** run pre_uninstall hook by @risu729 in
[#9662](#9662)

### 🚜 Refactor

- **(schema)** extract tool options definition by @risu729 in
[#9649](#9649)

### ⚡ Performance

- **(aqua)** bake rkyv aqua package blobs by @risu729 in
[#9535](#9535)

### 📦️ Dependency Updates

- lock file maintenance by @renovate[bot] in
[#9773](#9773)

### 📦 Registry

- add vector
([github:vectordotdev/vector](https://github.com/vectordotdev/vector))
by @kquinsland in [#9761](#9761)
- add oc and openshift-install (http backend) by @konono in
[#9669](#9669)

### New Contributors

- @konono made their first contribution in
[#9669](#9669)
- @kquinsland made their first contribution in
[#9761](#9761)
jdx pushed a commit that referenced this pull request May 11, 2026
## Why
This is the same follow-up enabled by #9582 and extended by #9649: now
that Tombi correctly tracks evaluated properties through referenced
schemas, repeated env directive property schemas can be shared instead
of copied into every object variant.

The env schema has many repeated pieces: primitive env values,
path/path-array handling, `tools`, `redact`, `required`, and age
encryption fields. Keeping those inline makes the schema large and makes
it easy for one directive form to drift from another. Reusing
definitions keeps env object validation consistent across the main
config schema and the generated task schema.

## What changed
- Adds shared env schema definitions for primitive env values, path/path
arrays, `tools`, `redact`, `required`, and age value/format fields.
- Replaces repeated inline env and env-directive property schemas with
refs to those definitions.
- Expands Tombi coverage for env value objects, env file/source/path
directives, age format validation, and directive `required` validation.
- Inherits the #9649 test harness change that treats Tombi warnings as
failures.

## Validation
- `git diff --check`
- Direct Tombi validation with `tombi@0.9.22` and `--error-on-warnings`
for valid env value/directive cases and invalid age/directive cases.
- Full e2e should run in CI; my earlier local targeted e2e run was
stopped after the build step stalled behind another active cargo build
on this machine.

This PR was generated by an AI coding assistant.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants