Skip to content

feat: support variables in context instructions#75

Merged
kowyo merged 1 commit into
mainfrom
feat/context-variable-substitution
Jun 20, 2026
Merged

feat: support variables in context instructions#75
kowyo merged 1 commit into
mainfrom
feat/context-variable-substitution

Conversation

@kowyo

@kowyo kowyo commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • share variable substitution between skills and project instructions
  • expand variables in AGENTS.md using the same built-in variables as skills
  • document $VARIABLE and ${VARIABLE} support for skills and context files

Verification

  • make check
  • make type-check
  • local smoke test for SKILL.md and AGENTS.md expansion, confirming only MODEL_NAME expands while shell, dotenv-only, and missing variables remain literal

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

Copy link
Copy Markdown

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 refactors variable substitution logic into a dedicated module and extends it to support environment variables in both skill descriptions and project instructions, updating the documentation accordingly. The review feedback highlights a critical Python 3 syntax error in system_prompt.py where multiple exceptions are caught using commas instead of parentheses. Additionally, it points out that capturing os.environ at import time in config.py before load_dotenv runs prevents .env variables from being substituted and makes testing difficult, suggesting dynamic evaluation instead.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

for i, path in enumerate(files, 1):
try:
content = path.read_text().strip()
content = substitute_variables(path.read_text().strip(), variables)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The except PermissionError, OSError: block on the next line (line 65) contains a syntax error in Python 3.

In Python 3, the comma syntax for catching multiple exceptions was removed. It must be parenthesized as except (PermissionError, OSError):.

However, since PermissionError is a subclass of OSError in Python 3, you can simplify this to just:

except OSError:

This will catch both PermissionError and other OSError exceptions cleanly and is fully compatible with Python 3.

Note: The same syntax error also exists at line 15 of this file (except PermissionError, OSError:), which should also be updated to except OSError:.

Comment thread src/mini_agent/config.py Outdated
PROJECT_SKILLS_DIR = WORKDIR / ".agents" / "skills"
SKILLS_DIRS = [HOME_SKILLS_DIR, PROJECT_SKILLS_DIR]

SHELL_ENVIRONMENT = dict(os.environ)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Capturing os.environ at import time before load_dotenv is called has two significant drawbacks:

  1. Excludes .env variables: Any environment variables loaded from the .env file via load_dotenv will not be available for substitution in AGENTS.md or SKILL.md. While the PR description mentions this is intentional ("dotenv-only variables remaining literal"), this behavior is highly counter-intuitive to users who expect .env to act as an extension of their shell environment.
  2. Fragile and hard to test: Capturing environment variables at import time makes it difficult to dynamically update or mock environment variables during testing or runtime.

Recommendation:
Consider removing SHELL_ENVIRONMENT entirely and dynamically reading os.environ inside get_variables() in src/mini_agent/agent/variables.py:

def get_variables() -> dict[str, str]:
    variables = dict(os.environ)
    variables["MODEL_NAME"] = config.get_model()
    return variables

If you must exclude sensitive variables (like API keys), you can explicitly filter them out by key name rather than excluding all .env variables.

@greptile-apps

greptile-apps Bot commented Jun 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR extracts the variable substitution helpers (get_variables / substitute_variables) from skills.py into a new shared variables.py module, then applies the same substitution to AGENTS.md context files when building the system prompt. Only MODEL_NAME is currently defined, and unknown $PLACEHOLDER patterns fall through unchanged.

  • variables.py centralises the regex and dict-lookup logic previously duplicated inside SkillLoader; both skills.py and system_prompt.py now call the shared helpers.
  • system_prompt.py calls substitute_variables() on each context file's content inside _build_context_section(), which runs once at module-import time — consistent with how skill descriptions are resolved.
  • Docs are updated for both AGENTS.md and SKILL.md to document the $VARIABLE (unbraced) form alongside ${VARIABLE}.

Confidence Score: 5/5

Safe to merge — the refactoring is behaviour-preserving and the new AGENTS.md expansion only adds MODEL_NAME, keeping the variable surface small and consistent with skills.

The extraction of substitution logic into variables.py is a clean, well-scoped refactor with no behavioural change for skills and a straightforward, additive change for AGENTS.md. The only defined variable is MODEL_NAME; unknown placeholders fall through unchanged, so there is no risk of accidental expansion of sensitive values.

docs/usage.md is missing a variable reference table that would make it clear which placeholders are actually recognised.

Important Files Changed

Filename Overview
src/mini_agent/agent/variables.py New shared module correctly extracts variable resolution and substitution logic from skills.py into reusable helpers; regex and fallback logic are sound.
src/mini_agent/agent/skills.py Clean refactor — drops local _variables()/_substitute() implementations and delegates to the new variables module; no behavioral change for skill loading.
src/mini_agent/agent/system_prompt.py Applies substitute_variables() to AGENTS.md content before building the SYSTEM constant; MODEL_NAME is the only defined variable, matching existing skills behavior.
docs/usage.md Documents variable substitution for AGENTS.md but omits a variable reference table like the one in skills.md; users may not know only MODEL_NAME is currently defined.
docs/skills.md Correctly updates the variable syntax description to include both $VARIABLE and ${VARIABLE} forms; the reference table is already accurate.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Config
    participant variables.py
    participant skills.py
    participant system_prompt.py

    Note over system_prompt.py: Module import time
    system_prompt.py->>variables.py: get_variables()
    variables.py->>Config: config.get_model()
    Config-->>variables.py: model_id
    variables.py-->>system_prompt.py: "{"MODEL_NAME": model_id}"
    system_prompt.py->>variables.py: substitute_variables(agents_md_text, vars)
    variables.py-->>system_prompt.py: expanded text → SYSTEM const

    Note over skills.py: On get_descriptions() call
    skills.py->>variables.py: get_variables()
    variables.py->>Config: config.get_model()
    Config-->>variables.py: model_id
    variables.py-->>skills.py: "{"MODEL_NAME": model_id}"
    skills.py->>variables.py: substitute_variables(desc, vars)
    variables.py-->>skills.py: expanded description

    Note over skills.py: On get_skill(name) call
    skills.py->>variables.py: get_variables()
    variables.py->>Config: config.get_model()
    Config-->>variables.py: model_id
    variables.py-->>skills.py: "{"MODEL_NAME": model_id}"
    skills.py->>variables.py: substitute_variables(body, vars)
    variables.py-->>skills.py: expanded skill body
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Config
    participant variables.py
    participant skills.py
    participant system_prompt.py

    Note over system_prompt.py: Module import time
    system_prompt.py->>variables.py: get_variables()
    variables.py->>Config: config.get_model()
    Config-->>variables.py: model_id
    variables.py-->>system_prompt.py: "{"MODEL_NAME": model_id}"
    system_prompt.py->>variables.py: substitute_variables(agents_md_text, vars)
    variables.py-->>system_prompt.py: expanded text → SYSTEM const

    Note over skills.py: On get_descriptions() call
    skills.py->>variables.py: get_variables()
    variables.py->>Config: config.get_model()
    Config-->>variables.py: model_id
    variables.py-->>skills.py: "{"MODEL_NAME": model_id}"
    skills.py->>variables.py: substitute_variables(desc, vars)
    variables.py-->>skills.py: expanded description

    Note over skills.py: On get_skill(name) call
    skills.py->>variables.py: get_variables()
    variables.py->>Config: config.get_model()
    Config-->>variables.py: model_id
    variables.py-->>skills.py: "{"MODEL_NAME": model_id}"
    skills.py->>variables.py: substitute_variables(body, vars)
    variables.py-->>skills.py: expanded skill body
Loading

Reviews (2): Last reviewed commit: "feat: support variables in context instr..." | Re-trigger Greptile

Comment thread src/mini_agent/config.py Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 6 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/mini_agent/agent/variables.py
Co-authored-by: Codex <noreply@openai.com>
@kowyo kowyo force-pushed the feat/context-variable-substitution branch from f409697 to 18291aa Compare June 20, 2026 04:19
@kowyo kowyo merged commit 28c4eb9 into main Jun 20, 2026
16 checks passed
@kowyo kowyo deleted the feat/context-variable-substitution branch June 20, 2026 04:20
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