Skip to content

refactor(config)!: remove pkl CLI fallback - #1270

Open
jdx wants to merge 1 commit into
refactor/remove-v1-interfacesfrom
refactor/remove-pkl-cli-fallback
Open

refactor(config)!: remove pkl CLI fallback#1270
jdx wants to merge 1 commit into
refactor/remove-v1-interfacesfrom
refactor/remove-pkl-cli-fallback

Conversation

@jdx

@jdx jdx commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • always evaluate project, subproject, and global configuration with the built-in pklr evaluator
  • remove the HK_PKL_BACKEND switch, pkl subprocess execution, mise fallback, backend cache key, and shell-words dependency
  • return targeted v2 migration guidance when HK_PKL_BACKEND is still set
  • update the configuration and migration documentation for the pklr-only runtime

Stack

Testing

  • cargo test --all --all-features --quiet
  • mise run test:bats test/pklr_backend.bats
  • mise run test:bats test/v2_migration_errors.bats
  • mise run lint
  • mise run render

AI-assisted — Tool: Codex; model: unavailable/unavailable; version: unavailable.


Note

Medium Risk
Config evaluation is a core startup path; removing the CLI fallback is a breaking v2 behavior change, though pklr was already the default and migration errors are explicit.

Overview
hk v2 always evaluates hk.pkl (and global config) with the built-in pklr evaluator. The optional pkl CLI path, mise x -- pkl fallback, HK_PKL_BACKEND, and the shell-words dependency are removed; setting HK_PKL_BACKEND now fails fast with v2 migration text.

Config loading and import analysis always go through pklr. Resolved-config caching no longer keys on backend choice (with_cache_key removed). Docs and migration guide describe pklr-only runtime and reframe HK_PKL_* vars as pklr settings rather than pkl CLI flags.

Bats tests drop pkl-backend selection cases and add coverage for rejecting HK_PKL_BACKEND.

Reviewed by Cursor Bugbot for commit 16c163f. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 04089b44-4edc-47de-8829-791fe7b733f1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes the built-in pklr evaluator the sole configuration runtime and provides an explicit migration error for the removed backend selector.

  • Removes pkl CLI and mise fallback execution, backend selection state, and the shell-words dependency.
  • Simplifies project, subproject, global-config, and import evaluation to use pklr.
  • Removes backend identity from resolved-configuration cache keys.
  • Updates migration and configuration documentation and adds integration coverage for the removed environment variable.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete blocking or independently actionable non-blocking issues identified.

The evaluator, environment, cache, dependency, test, and documentation changes consistently remove backend selection and retain a single pklr configuration path with explicit migration handling.

Important Files Changed

Filename Overview
src/config.rs Routes all Pkl evaluation and import analysis through pklr, rejects the removed backend variable, and removes the subprocess evaluator without an accepted defect.
src/env.rs Removes the obsolete backend setting and evaluator-selection helper while retaining pklr runtime controls.
src/cache.rs Removes the generic extra cache-key builder now that evaluator identity no longer varies.
test/v2_migration_errors.bats Adds command-level coverage confirming that the removed backend variable fails with targeted migration guidance.
test/pklr_backend.bats Removes tests for the deleted CLI backend while preserving core pklr evaluation coverage.
docs/environment_variables.md Removes the backend selector and documents the remaining Pkl environment variables as pklr settings.

Reviews (1): Last reviewed commit: "refactor(config)!: remove pkl CLI fallba..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown
Contributor

Instruction counts

Nothing was compared, and so nothing was gated. No series appears on both sides: either the base has no measurements recorded, or the two were measured on different runner classes, which are deliberately not comparable — counts shift between machine types by more than a real regression does.

New, nothing to compare against: builtins on bamboo-v2-ubuntu24.04-x64-30vcpu-24gb-rust1.97.1, check on bamboo-v2-ubuntu24.04-x64-30vcpu-24gb-rust1.97.1, usage on bamboo-v2-ubuntu24.04-x64-30vcpu-24gb-rust1.97.1, validate on bamboo-v2-ubuntu24.04-x64-30vcpu-24gb-rust1.97.1

Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run.

Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.

16c163f8719b vs 0070b96dfe31 · measured on the runner, not pushed to the history.

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.

1 participant