Skip to content

feat(vars): add pattern type to definitions.vars#334

Merged
fohte merged 4 commits into
mainfrom
fohte/feat-vars-pattern-type
May 4, 2026
Merged

feat(vars): add pattern type to definitions.vars#334
fohte merged 4 commits into
mainfrom
fohte/feat-vars-pattern-type

Conversation

@fohte
Copy link
Copy Markdown
Owner

@fohte fohte commented May 4, 2026

Purpose

  • Name a "command + leading global flags" prefix once (e.g. kubectl) and reuse it across multiple rules
    • The same prefix has to be repeated across 7+ allow rules today, so adding a new global flag means editing every rule
  • Existing options cannot express this reuse
    • <flag:name> requires "at least one occurrence", so commands without any flag are not covered
    • <opts> does not absorb space-separated long-flag values (e.g. --context foo); foo is left as a positional
    • Existing definitions.vars (literal / path) only accepts a single token sequence or a canonicalised path, not pattern syntax such as [--flag *]

Approach

  • Add a new pattern value to definitions.vars[<name>].type. Each value is written using the regular rule-pattern syntax

  • A <var:name> reference inlines the matching pattern value at the position where it appears

  • Reject the following at config validation time

    • Other placeholders nested inside a pattern value (<cmd>, <opts>, <vars>, <var:...>, <path:...>, <flag:...>)
    • [<var:pattern-typed>] (a pattern-typed var inside an optional group)
  • Example:

    definitions:
      vars:
        kubectl:
          type: pattern
          values:
            - "kubectl [-n|--namespace *] [--context *] [--cluster *] [--user *] [--kubeconfig *]"
    rules:
      - allow: "<var:kubectl> get|describe|logs *"
      - allow: "<var:kubectl> top node|pod|nodes|pods *"
      - allow: "<var:kubectl> auth can-i|whoami *"

    With this definition, the first rule matches every one of kubectl get pods, kubectl --context foo get pods, and kubectl --kubeconfig ~/.kube/work --context prod get pods -A

Design decisions

How to express reusable global-flag prefixes

Decision Design Pros Cons
Chosen Add a pattern type to definitions.vars and expand the value as a rule pattern fragment in place Orthogonal to the existing types; introduces no behaviour change for literal / path. Rule authors keep using <var:name> without caring whether it is pattern Pattern values share the rule-pattern grammar, so users will be tempted to nest other placeholders inside (mitigated by an explicit validation reject)
Rejected Extend <flag:name> with a "zero-or-more" mode Reuses the existing mechanism Breaks the contract that bool/value-flag aliases are captured through flag_groups
Rejected Extend <opts> to absorb space-separated long-flag values No new placeholder needed Cannot express "the allowed flags for kubectl only" — there is no way to constrain the absorbed flag set

Whether pattern values can nest other placeholders

Decision Design Pros Cons
Chosen Reject nesting of <cmd>, <opts>, <vars>, <var:...>, <path:...>, and <flag:...> inside pattern values at validation time Keeps semantics single-pass; sidesteps cycle detection and pre-parse cache invalidation Reusing <flag:> and similar placeholders inside a pattern var requires a separate future design
Rejected Allow nesting and expand recursively More expressive Cycle detection is required and the pre-parse cache (parsed_pattern_vars) design no longer holds

fohte added 2 commits May 4, 2026 13:50
…-prefix patterns

Adds VarType::Pattern alongside Literal and Path. Each `pattern`-typed
value is parsed as a rule-pattern fragment and inlined wherever
`<var:name>` appears, so a base CLI plus its global flags can be named
once and reused across rules:

    definitions:
      vars:
        kubectl:
          type: pattern
          values:
            - "kubectl [-n|--namespace *] [--context *] [--cluster *]"
    rules:
      - allow: "<var:kubectl> get|describe|logs *"
      - allow: "<var:kubectl> top node|pod|nodes|pods *"

intent(vars): let users name a "command + global flags" prefix once
  instead of repeating `[-n *] [--context *] ...` on every rule, which
  was previously unworkable because flag_groups requires at-least-one
  occurrence and `<opts>` does not absorb space-separated long-flag
  values
decision(vars): inline-expand pre-parsed sub-patterns at `<var:name>`
  positions and splice their tokens into the outer pattern's remaining
  tokens before recursing through match_engine, reusing the existing
  Optional/Wildcard/Flag matching paths instead of inventing a parallel
  engine
decision(vars): forbid placeholder nesting (<cmd>, <opts>, <vars>,
  <var:>, <path:>, <flag:>) inside pattern values, and reject
  [<var:pattern-typed>] (pattern var inside an optional group), both
  at validate() time, keeping semantics single-pass and mirroring the
  long-standing <flag:name> restriction
rejected(vars): extending <flag:name> with a "zero-or-more" mode -
  bool/value-flag aliases must collapse into flag_groups captures and
  the change would break that contract
rejected(vars): adding implicit recursive expansion of nested <var:>
  references - opens the door to cycles and conflicts with pre-parse
  caching (parsed_pattern_vars)
constraint(matcher): pattern-typed values cannot be consumed via the
  single-token match_var_ref path, so match_value_with_type returns
  false for VarType::Pattern; multi-token consumption goes through
  match_pattern_prefix / inline expansion in match_engine instead
learned(matcher): rule_engine::collect_value_flags and matcher's
  collect_value_flag_aliases both walk pattern tokens to register
  value-flag aliases for the command parser. Both must recurse into
  parsed_pattern_vars sub-patterns - otherwise `--namespace foo` from a
  sub-pattern's `[--namespace *]` is mis-tokenised as a positional
…refix budget

intent(docs): the previous commit's "Pattern Type Inline Expansion" table
  embedded the rule pattern (which contains `|`) inside a header cell, so
  remark-gfm parsed it as extra columns and the rendered Starlight page
  showed an unaligned, mostly-empty grid
intent(matcher): doc comment of match_pattern_prefix described an
  approach (append `Wildcard`, read remaining-suffix length) that does
  not match the implementation, which enumerates `for k in 0..=rest.len()`
  and runs full-match per k. Rewrite the comment to match the code.
intent(matcher): each `k` iteration of match_pattern_prefix used to
  allocate a fresh `Cell::new(0)` step budget, so the global
  MAX_MATCH_STEPS DoS guard was effectively multiplied by `cmd_tokens`
  length when a pattern-typed `<var:>` sat in command position. Share
  one budget across all k trials.
decision(docs): also document that pattern-typed `<var:name>` exposes
  only the command-name token under `vars.<name>` in `when` clauses --
  users could otherwise expect the captured prefix (kubectl + global
  flags) since `<flag:name>` does capture every absorbed value
constraint(matcher): only the `steps` cell is shared across k trials;
  `after_double_dash` / `var_captures` / `flag_group_captures` must
  still be re-initialised per k because each k is an independent trial
  and any state leakage would cross-contaminate sibling alternatives
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

❌ Patch coverage is 75.65543% with 65 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.75%. Comparing base (5dc8483) to head (c0b0c45).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/rules/pattern_matcher/mod.rs 40.25% 46 Missing ⚠️
src/config/model.rs 91.40% 11 Missing ⚠️
src/rules/pattern_matcher/token_matching.rs 90.56% 5 Missing ⚠️
src/rules/rule_engine.rs 66.66% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #334      +/-   ##
==========================================
- Coverage   89.04%   88.75%   -0.29%     
==========================================
  Files          53       53              
  Lines       11871    12131     +260     
==========================================
+ Hits        10570    10767     +197     
- Misses       1301     1364      +63     
Flag Coverage Δ
Linux 88.57% <75.65%> (-0.29%) ⬇️
macOS 89.92% <75.65%> (-0.33%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

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

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 introduces a new pattern type for definitions.vars, enabling reusable command-prefix patterns that expand inline within rule patterns. The implementation includes matching engine updates for recursive expansion, prefix consumption logic, and validation to prevent nested placeholders or invalid usage in optional groups. Documentation and integration tests are also provided. I have no feedback to provide.

fohte added 2 commits May 4, 2026 14:14
intent(releases): release notes follow the convention of suffixing each
  entry's heading with the PR link, e.g.
  "### Feature ([no.NNN](https://github.com/fohte/runok/pull/NNN))".
  The pattern-type entry was missing this link, leaving readers unable
  to trace the change back to its PR.
…n var section

intent(placeholders): placeholders.md documents what a placeholder
  expands into syntactically; "which commands a particular rule pattern
  matches" is rule-evaluation behaviour and belongs elsewhere. Replace
  the multi-rule + match-result table with a single before/after
  expansion sentence so the section stays focused on the placeholder's
  semantics
@fohte fohte merged commit 49f6212 into main May 4, 2026
10 checks passed
@fohte fohte deleted the fohte/feat-vars-pattern-type branch May 4, 2026 05:25
@fohte-bot fohte-bot Bot mentioned this pull request May 4, 2026
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