Skip to content

feat: Wire userConfig into plugin infrastructure#336

Open
ErbolTakhirov wants to merge 6 commits intotractorjuice:mainfrom
ErbolTakhirov:feat/userconfig-wiring
Open

feat: Wire userConfig into plugin infrastructure#336
ErbolTakhirov wants to merge 6 commits intotractorjuice:mainfrom
ErbolTakhirov:feat/userconfig-wiring

Conversation

@ErbolTakhirov
Copy link
Copy Markdown

@ErbolTakhirov ErbolTakhirov commented Apr 20, 2026

PR Title
fix: keep templates static placeholders for classification and owner

PR Description
This PR cleans up the userConfig follow-up branch to match plugin substitution rules and remove merge-risk regressions.

What was fixed
Reverted template-level dynamic placeholders across all template packs:
${user_config.default_classification} / ${default_classification} → [CLASSIFICATION]
${user_config.organisation_name} / ${organisation_name} → [OWNER_NAME_AND_ROLE]
Removed template fallback suffixes introduced by the branch so templates remain static.
Kept runtime substitution architecture intact:
scripts/converter.py logic preserved
arckit-claude/hooks/governance-scan.mjs preserved
command/agent user_config guidance preserved
Why
Template files are static scaffolds and do not get runtime user_config substitution. Dynamic placeholders there leaked into generated artifacts and conflicted with plugin behavior.

Validation
No ${user_config.*} remains in templates.
Default classification guidance in commands remains consistent (OFFICIAL for UK Gov, PUBLIC for Generic when unset).
Plugin manifest version remains 4.9.0.

@tractorjuice tractorjuice self-assigned this Apr 21, 2026
Copy link
Copy Markdown
Owner

@tractorjuice tractorjuice left a comment

Choose a reason for hiding this comment

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

Review

Thank you for the thoughtful wiring work — the converter changes and the governance-scan hook additions are solid. However, this PR has one critical merge blocker (a stale branch that would revert recent work) and one architectural concern about the template-level placeholder approach that I think needs a design discussion before the 60+ template changes land.

Recommending changes requested.


Blocker 1 — Branch is stale and would revert recent work

The branch forked before v4.9.0 and has not been rebased. The diff against main includes unintentional reverts:

File Regression
pyproject.toml Version goes 4.9.04.8.0
docs/README.md Version footer goes 4.9.04.8.0
docs/getting-started.html Removes the --sparse install tip added in #343
tests/mermaid-wardley/convert.mjs Deleted entirely (-442 lines) — this is from PRs #339, #340, #341, #344
tests/mermaid-wardley/test-fidelity.mjs Deleted entirely (-457 lines)
tests/mermaid-wardley/test-real-maps.mjs Large rewrite that reverts recent fixes
tests/mermaid-wardley/package*.json Lock file reverted

The PR description says "540 files" but the actual diff is 583 files; the extra 43 are these unintentional reverts.

Required: rebase onto current main and verify the git diff contains only the intended changes.


Blocker 2 — Template-level ${user_config.*} substitution likely won't work

Per the Claude Code plugin docs, ${user_config.KEY} is substituted in:

  • MCP/LSP server configs
  • Hook commands, monitor commands
  • Skill and agent content (for non-sensitive values)

Templates (.arckit/templates/*.md) are none of these — they're arbitrary filesystem files read by commands via the Read tool at runtime. Read returns raw content; no substitution happens.

That means every one of the 60+ templates in this PR now contains a literal string like:

| Classification | ${user_config.default_classification} (fallback: [PUBLIC / OFFICIAL / OFFICIAL-SENSITIVE / SECRET]) |

When Claude reads the template and copies it into the generated artifact, it will paste that literal string verbatim rather than the configured value. Even if Claude's inference catches the intent sometimes, the compound ${placeholder} (fallback: [...]) pattern is ambiguous enough that some outputs will be wrong.

The correct architecture (already used by the command changes in this PR, e.g. requirements.md line 195):

  • Templates keep the existing [CLASSIFICATION], [OWNER_NAME_AND_ROLE] bracket placeholders as-is.
  • Command bodies reference ${user_config.*} when instructing Claude how to fill them, because command bodies are substituted. The PR already does this correctly for requirements.md, sobc.md, and others.

I think the 60+ template changes should be reverted, and the work of surfacing user_config defaults should live entirely in the command instructions (which the PR already does).

If I've misunderstood the plugin substitution scope, please point me at the docs — happy to re-review if templates do get substituted somewhere I missed.


What's solid (keep this work)

  • scripts/converter.py — the rewrite_user_config_placeholders helper, the in-tree rewrite for copied extensions, and the per-target application look correct. Env-var fallback (${KEY}) for non-Claude targets is the right strategy.
  • arckit-claude/hooks/governance-scan.mjs — verified correct:
    • CLAUDE_PLUGIN_OPTION_* uppercased naming matches the documented plugin subprocess env var convention
    • All three fields (organisation_name, default_classification, governance_framework) are non-sensitive in plugin.json, so they will be exported
    • "Plugin Default Alignment Gaps" section design is genuinely useful — flagging classification/owner mismatches and Generic+UK-Gov artifact collisions
  • Command body updates for requirements.md, sobc.md, tcop.md, atrs.md, service-assessment.md — these correctly reference ${user_config.*} in the instruction text (which IS substituted). This is the right pattern; keep these and remove the template changes.
  • Agent updates for arckit-research.md and arckit-datascout.md — same pattern, correct.

Minor nits (for when you rebase)

  • PR description file count: says "540 files generated correctly" — actual diff shows 583 changed files once the unintended reverts are removed, the real count will drop substantially.
  • Command updates have inconsistent defaults: requirements.md says "OFFICIAL for UK Gov, PUBLIC for Generic" while sobc.md historically said "OFFICIAL for UK Gov, INTERNAL for private sector" and this PR changes it to "OFFICIAL for UK Gov and PUBLIC for Generic" — worth one pass to make the per-mode defaults consistent across all updated commands.
  • Worth adding a short section to docs/guides/ (or CLAUDE.md) explaining the contract: "user_config values are substituted in MCP configs, hook commands, and command/agent bodies — but NOT in template files read at runtime." This would save the next contributor from the same trap.

Proposed path forward

  1. Rebase onto main to drop the version/test/doc reverts (blocker 1).
  2. Revert the 60+ template edits; keep the command-body and agent-body edits which already do the right thing (blocker 2), OR provide evidence that template-level substitution does work in the Claude runtime.
  3. Keep the converter and governance-scan.mjs work as-is — both are good.
  4. Add a short note to CLAUDE.md documenting the user_config substitution contract.

Happy to re-review once the rebase is done.

@tractorjuice tractorjuice added enhancement New feature or request good first issue Good for newcomers labels Apr 21, 2026
@ErbolTakhirov ErbolTakhirov force-pushed the feat/userconfig-wiring branch 2 times, most recently from 04ae93f to 1923ea6 Compare April 21, 2026 17:25
Add organisation_name, default_classification, governance_framework userConfig fields

Update 60+ templates with ${user_config.*} placeholders in Document Control

Add governance framework mode to commands and agents

Fix converter and regenerate extension files
@ErbolTakhirov ErbolTakhirov force-pushed the feat/userconfig-wiring branch from 1923ea6 to 0d9677d Compare April 23, 2026 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants