fix(slopguard): accept source environment template defaults - #127
Conversation
Retire placeholder value classification for named environment templates while preserving other target and context guards. Document disclosure responsibility and cover branch preimages and the CLI boundary. Fixes #126
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![slopzapper[bot]](https://avatars.githubusercontent.com/in/4527847?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔵 Needs a closer look
It relaxes a sensitive-path/content boundary that affects disclosure behavior, and should receive final human review despite the added tests and documentation.
Pull request overview
This PR updates Slopguard’s target collection to treat explicitly named environment template files as reviewable source material even when they contain ordinary comments, literal defaults, and placeholder-like strings—removing the prior “placeholder-only” value classification that blocked review (including for deleted/preimage content in branch diffs).
Changes:
- Remove environment template value/diff validation so
.env.*.{example,sample,template}content is bundled without placeholder-only enforcement. - Expand/adjust regression coverage to assert template defaults and deleted preimages are included, while explicit context inclusion of
.env.*remains rejected. - Update TARGETS documentation and add a binary-level E2E test to confirm
.env.exampleis reviewable but real.envis still blocked before provider invocation.
File summaries
| File | Description |
|---|---|
| cli/slopguard/internal/target/target_test.go | Updates unit tests to cover defaults, comments, and deleted preimages for environment templates; keeps explicit-context rejection coverage. |
| cli/slopguard/internal/target/material.go | Removes environment-template content safety checks for untracked/deleted materials; template handling now relies on filename-based exception only. |
| cli/slopguard/internal/target/freeze.go | Drops environment-template diff validation during collection to allow non-placeholder template content in diffs. |
| cli/slopguard/internal/target/diff.go | Removes validateEnvironmentTemplateDiff, eliminating placeholder-only enforcement at diff parsing time. |
| cli/slopguard/docs/TARGETS.md | Documents the updated disclosure boundary and clarifies that template names do not imply secret-free content. |
| cli/slopguard/cmd/slopguard/template_e2e_test.go | Adds binary E2E coverage for the .env.example vs .env boundary and provider-call behavior. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Problem
Ordinary comments, literal defaults and embedded placeholders in named environment templates prevent review before the provider runs, including when they occur only in deleted source. Fixes #126.
Solution
Remove the obsolete value classifier for changed
.env.*templates ending in.example,.sampleor.template. Keep the existing filename exception and all other collection guards, including rejection of real.envfiles and sensitive explicit context files. Document that operators authorize disclosure: a template filename does not establish that its contents are secret-free.Proof
All five template collection cases failed before the fix and passed afterward. Built-binary tests confirm that a synthetic template branch reaches the fake provider exactly once, while the same content in
.envfails before any provider call. Local release verification and snapshot packaging passed for all four targets. Independent whole-branch slopguard review with Codex Astra medium returned clean on9b37f45against5ee30e4.