Implementation Plan: Add Configurable Label Whitelist to Dynaconf Config#505
Conversation
Adds allowed_labels list to GitHubConfig with check_label_allowed() validation method. Validates labels in claim_issue, release_issue, prepare_issue, and _file_or_update_github_issue before any GitHub API calls. Empty list (default) preserves existing unrestricted behavior. defaults.yaml seeds the standard project label set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add check_labels_allowed() to GitHubConfig so prepare_issue handler can delegate multi-label validation with a single call, satisfying REQ-CNST-008 (no for-loops in @mcp.tool handlers).
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested
| labels = config.report_bug.github_labels | ||
| for lbl in labels: | ||
| if err := config.github.check_label_allowed(lbl): | ||
| return {"skipped": True, "reason": err} |
There was a problem hiding this comment.
[warning] cohesion: _file_or_update_github_issue returns {skipped: true, reason: err} for label validation failure while sibling handlers return {success: false, error: err}. Inconsistent error schema makes uniform error handling impossible for callers.
There was a problem hiding this comment.
Investigated — this is intentional. _file_or_update_github_issue is an internal best-effort helper (not an @mcp.tool handler) that uses {skipped, reason} consistently throughout all its early-exit paths (lines 461, 467, 471). The 'sibling handlers' are @mcp.tool-level functions with a different public contract. Comparing the two schemas is a category error — the function's internal consistency is correct.
| default_repo=val(gh, "default_repo", _gh["default_repo"]) or None, | ||
| in_progress_label=str(val(gh, "in_progress_label", _gh["in_progress_label"])), | ||
| staged_label=str(val(gh, "staged_label", _gh["staged_label"])), | ||
| allowed_labels=list(val(gh, "allowed_labels", _gh["allowed_labels"])), |
There was a problem hiding this comment.
[warning] defense: list(val(gh, allowed_labels, _gh[allowed_labels])) will raise TypeError if the resolved config value is a non-iterable scalar or None. Other fields use str() guards; add an isinstance/list guard here.
There was a problem hiding this comment.
Investigated — this is intentional. The list(val(...)) pattern without isinstance guards is used consistently for all list-typed fields in the settings builder: line 365 (migration.suppressed) and line 390 (report_bug.github_labels) follow the same pattern with no guards. This is a project-wide convention, not a defect specific to line 383.
| def test_automation_config_allowed_labels_default_is_empty(self): | ||
| """AutomationConfig().github.allowed_labels defaults to empty list.""" | ||
| cfg = AutomationConfig() | ||
| assert cfg.github.allowed_labels == [] |
There was a problem hiding this comment.
[warning] tests: cfg.allowed_labels should be cfg.github.allowed_labels. AutomationConfig wraps github config under .github; accessing directly on top-level will raise AttributeError instead of a clean assertion failure.
There was a problem hiding this comment.
Investigated — this is intentional. Line 16 already reads assert cfg.github.allowed_labels == [] with the correct .github accessor. The reviewer's claim was based on a misread of the current code.
|
|
||
| if should_stage: | ||
| if err := tool_ctx.config.github.check_label_allowed(effective_staged_label): | ||
| return json.dumps( |
There was a problem hiding this comment.
[warning] cohesion: release_issue label-validation error response includes extra fields issue_number and label that prepare_issue and claim_issue omit. Decide: add context fields to all sibling handlers, or remove extra fields here.
There was a problem hiding this comment.
Valid observation — flagged for design decision. release_issue staged-label validation error includes issue_number and label context fields; prepare_issue and claim_issue return only {success: false, error: err}. Both options are defensible. Requires human decision: add context fields to all sibling handlers, or remove extra fields from release_issue.
| assert "unlisted-staged" in result["error"] | ||
| mock_client.ensure_label.assert_not_called() | ||
|
|
||
| @pytest.mark.anyio |
There was a problem hiding this comment.
[warning] tests: test_release_issue_no_staging_skips_validation asserts result[success] is True but does not verify whether remove_label was called, making the assertion potentially vacuous. Clarify: is this test verifying that label-validation is skipped, or that remove_label is still called?
There was a problem hiding this comment.
Investigated — this is intentional. The assertion result['success'] is True with allowed_labels=['in-progress'] is not vacuous: if staged-label validation incorrectly ran, the default 'staged' label would be blocked, causing success=False. The test's intent (docstring: 'doesn't validate staged_label') is correctly verified. remove_label being called unconditionally is a separate concern covered by other test cases.
… not backward compat
Summary
Add a
github.allowed_labelslist to the dynaconf configuration layer. When configured, all label creation and application operations in the server tool handlers validate each label name against this list before executing. An empty or absent list preserves current behavior (no restriction). The validation lives as a method onGitHubConfig— the dataclass that owns the field — and is called at every label operation site in the server layer.Requirements
CFG
github.allowed_labelskey that accepts a list of strings.github.allowed_labelskey must be optional — when absent or empty, no label restriction is enforced.defaults.yamlmust include a defaultgithub.allowed_labelslist containing the project's standard labels.VAL
TEST
Architecture Impact
Operational Diagram
%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%% flowchart TB %% CLASS DEFINITIONS %% classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff; classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff; classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff; classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; subgraph Config ["● CONFIGURATION HIERARCHY (read-only)"] direction TB EnvVars["Environment Variables<br/>━━━━━━━━━━<br/>AUTOSKILLIT_GITHUB__ALLOWED_LABELS<br/>Highest priority override"] ProjectYAML["Project .autoskillit/config.yaml<br/>━━━━━━━━━━<br/>github.allowed_labels:<br/> [custom list]"] DefaultsYAML["● defaults.yaml<br/>━━━━━━━━━━<br/>github.allowed_labels:<br/> [bug, enhancement, in-progress,<br/> staged, autoreported,<br/> recipe:implementation,<br/> recipe:remediation]"] end subgraph GitHubCfg ["● GITHUBCONFIG (read/write state)"] direction TB AllowedLabels["● GitHubConfig<br/>━━━━━━━━━━<br/>allowed_labels: list[str]<br/>in_progress_label: str<br/>staged_label: str"] CheckMethod["● check_label_allowed(label)<br/>━━━━━━━━━━<br/>Returns None → permitted<br/>Returns str → error message"] end subgraph CLI ["CLI ENTRY POINTS"] direction TB ConfigShow["autoskillit config show<br/>━━━━━━━━━━<br/>Inspect resolved allowed_labels<br/>(JSON output)"] ServeCmd["autoskillit serve<br/>━━━━━━━━━━<br/>Start MCP server<br/>Loads config at startup"] end subgraph ValidationGate ["● LABEL VALIDATION GATE (server layer)"] direction TB PrepareIssue["● prepare_issue<br/>━━━━━━━━━━<br/>check_labels_allowed(labels[])<br/>before skill session launch"] ClaimIssue["● claim_issue<br/>━━━━━━━━━━<br/>check_label_allowed(effective_label)<br/>before ensure_label + add_labels"] ReleaseIssue["● release_issue<br/>━━━━━━━━━━<br/>check_label_allowed(staged_label)<br/>before ensure_label + add_labels"] ReportBug["● _file_or_update_github_issue<br/>━━━━━━━━━━<br/>check_label_allowed per label<br/>before create_issue"] end subgraph Outcome ["OUTCOMES"] direction TB Proceed["Label Operation Proceeds<br/>━━━━━━━━━━<br/>GitHub API called normally"] Reject["Error Returned<br/>━━━━━━━━━━<br/>success=false / skipped=true<br/>Lists disallowed label + alternatives"] end %% CONFIG FLOW %% EnvVars -->|"highest priority"| AllowedLabels ProjectYAML -->|"project override"| AllowedLabels DefaultsYAML -->|"base defaults"| AllowedLabels %% VALIDATION FLOW %% AllowedLabels --> CheckMethod CheckMethod --> PrepareIssue CheckMethod --> ClaimIssue CheckMethod --> ReleaseIssue CheckMethod --> ReportBug %% CLI %% ConfigShow -->|"reads"| AllowedLabels ServeCmd -->|"loads"| AllowedLabels %% OUTCOMES %% PrepareIssue -->|"None = allowed"| Proceed ClaimIssue -->|"None = allowed"| Proceed ReleaseIssue -->|"None = allowed"| Proceed ReportBug -->|"None = allowed"| Proceed PrepareIssue -->|"str = rejected"| Reject ClaimIssue -->|"str = rejected"| Reject ReleaseIssue -->|"str = rejected"| Reject ReportBug -->|"str = rejected"| Reject %% CLASS ASSIGNMENTS %% class EnvVars,ProjectYAML,DefaultsYAML phase; class AllowedLabels,CheckMethod stateNode; class ConfigShow,ServeCmd cli; class PrepareIssue,ClaimIssue,ReleaseIssue,ReportBug handler; class Proceed output; class Reject detector;Security Diagram
%%{init: {'flowchart': {'nodeSpacing': 40, 'rankSpacing': 50, 'curve': 'basis'}}}%% flowchart TB %% CLASS DEFINITIONS %% classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff; classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff; classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000; classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff; subgraph Callers ["UNTRUSTED CALLERS (headless sessions / orchestrator)"] direction LR PrepareCall["prepare_issue call<br/>━━━━━━━━━━<br/>labels=[list supplied<br/>by AI session]"] ClaimCall["claim_issue call<br/>━━━━━━━━━━<br/>label= param<br/>or config default"] ReleaseCall["release_issue call<br/>━━━━━━━━━━<br/>staged_label param<br/>or config default"] ReportCall["report_bug call<br/>━━━━━━━━━━<br/>github_labels from<br/>report_bug config"] end subgraph WhitelistSource ["● WHITELIST SOURCE (read-only config)"] direction TB DefaultsYAML["● defaults.yaml<br/>━━━━━━━━━━<br/>bug, enhancement, in-progress,<br/>staged, autoreported,<br/>recipe:implementation,<br/>recipe:remediation"] ProjectOverride["Project config.yaml<br/>━━━━━━━━━━<br/>github.allowed_labels<br/>(operator-controlled)"] EnvOverride["Environment override<br/>━━━━━━━━━━<br/>AUTOSKILLIT_GITHUB__<br/>ALLOWED_LABELS"] end subgraph ValidationGate ["● VALIDATION GATE (server layer — trust boundary)"] direction TB GitHubCfg["● GitHubConfig.allowed_labels<br/>━━━━━━━━━━<br/>Resolved whitelist<br/>[] = permissive mode"] CheckSingle["● check_label_allowed(label)<br/>━━━━━━━━━━<br/>Single-label check<br/>used by claim/release/report"] CheckBatch["● check_labels_allowed(labels)<br/>━━━━━━━━━━<br/>Batch check<br/>used by prepare_issue"] Decision{"Label in<br/>allowed_labels<br/>OR list empty?"} end subgraph Blocked ["BLOCKED PATH"] ErrorResp["Error Response<br/>━━━━━━━━━━<br/>success=false / skipped=true<br/>Names disallowed label<br/>Lists allowed alternatives<br/>NO GitHub API call made"] end subgraph GitHubAPI ["TRUSTED EXTERNAL: GitHub API"] CreateLabel["gh label create<br/>━━━━━━━━━━<br/>Creates label on repo"] AddLabel["gh issue edit --add-label<br/>━━━━━━━━━━<br/>Applies label to issue"] end %% CONFIG FLOW %% DefaultsYAML -->|"base"| GitHubCfg ProjectOverride -->|"override"| GitHubCfg EnvOverride -->|"highest priority"| GitHubCfg %% VALIDATION ROUTING %% GitHubCfg --> CheckSingle GitHubCfg --> CheckBatch CheckSingle --> Decision CheckBatch --> Decision %% CALLER → GATE %% PrepareCall -->|"labels list"| CheckBatch ClaimCall -->|"effective_label"| CheckSingle ReleaseCall -->|"staged_label"| CheckSingle ReportCall -->|"each github_label"| CheckSingle %% OUTCOMES %% Decision -->|"DENY: str error"| ErrorResp Decision -->|"ALLOW: None"| CreateLabel Decision -->|"ALLOW: None"| AddLabel %% CLASS ASSIGNMENTS %% class PrepareCall,ClaimCall,ReleaseCall,ReportCall cli; class DefaultsYAML,ProjectOverride,EnvOverride phase; class GitHubCfg,CheckSingle,CheckBatch stateNode; class Decision detector; class ErrorResp gap; class CreateLabel,AddLabel output;Closes #503
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260325-091250-375300/.autoskillit/temp/make-plan/add_configurable_label_whitelist_plan_2026-03-25_000001.md🤖 Generated with Claude Code via AutoSkillit