New checks

[Keksclan]

This pull request introduces substantial improvements to repository hygiene, continuous integration, modularization, and documentation for the project. The main focus is on enforcing a clean, source-only repository, introducing a comprehensive CI pipeline, splitting optional tools into a separate module, and enhancing documentation for contributors and users.

Repository hygiene and CI enforcement:

• Added a GitHub Actions workflow (.github/workflows/ci.yml) that verifies no binaries or executables are committed, runs formatting checks, tests, race detection, linting, and vulnerability scanning for both core and tools modules.
• Updated CONTRIBUTING.md with explicit rules prohibiting committed binaries, emphasizing source-only contributions, and providing clear build instructions for both core and tools modules.
• Added a Makefile with a verify target to locally replicate CI checks: formatting, linting, tests, race detector, vulnerability check, and tools build.
• Documented the CI process and local verification steps in docs/CI.md.

Modularization and tool separation:

• Split generator and TUI tooling (goconfygen, goconfytui) into a separate tools/ Go module, keeping the core runtime lightweight and dependencies minimal. Updated installation, build, and usage instructions throughout the README.md and documentation. [1] [2] [3] [4] [5]

Documentation and release process:

• Enhanced the README.md and CHANGELOG.md to reflect the new module structure, stricter repository hygiene, and updated minimum Go version (now 1.22+). Added sections on versioning and compatibility. [1] [2]
• Added a German-language release checklist in RELEASE.md to standardize the release process.

Configuration and examples:

• Added a comprehensive example configuration file (config.example.yml) demonstrating best practices and environment macro usage.

Changelog and policy updates:

• Updated CHANGELOG.md with all major and minor changes, including repository hardening, modularization, improved error reporting, and new test/integration infrastructure.

These changes collectively enforce a clean, maintainable, and secure codebase, provide clear contributor guidance, and make the project easier to use and extend.

References:
[1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

Summary by CodeRabbit

• New Features

  • Explain reporting to track configuration sources/overrides; FILE macro to load values from files; redact-by-convention (password/secret/token) option; JSON Schema and example config added.

• Bug Fixes

  • Macro redaction fixed; dotenv loading no longer mutates OS env; stricter exact-match macro expansion.

• Documentation

  • CI, tooling, generator, install, config policy, and release guides added/updated.

• Refactor

  • Optional tools moved to a separate tools module; improved error reporting (path/line/column).

• Tests

  • Extensive unit, integration, golden, and fuzz tests added; CI workflow with lint/vulncheck.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds CI workflow and Makefile; introduces an optional tools module; defines canonical config schema and JSON Schema; implements explain/reporting for provenance; adds FILE and ENV macro expansion; enhances redaction (by-convention); expands tests (benchmarks, fuzz, integration, golden) and improves error reporting with FieldError metadata.

Changes

Cohort / File(s)	Summary
CI & Repo Hygiene \.github/workflows/ci.yml, Makefile, \.gitignore, docs/CI.md	Add GitHub Actions workflow (verify/lint/vulncheck), Makefile targets (verify, fmt-check, test, vulncheck, build-tools), and new ignore rules; document CI/local targets.
Docs & Release CHANGELOG.md, CONTRIBUTING.md, README.md, RELEASE.md, docs/*, config.example.yml	Major doc updates: lower Go version, tools module separation, install/build guidance for optional tools, config policy, CI/release guides, example config added.
Tools Module & Reorg tools/go.mod, tools/*, tools/internal/*, tools/cmd/*, tools/examples/*, tools/tests/*, tools/docs/RELEASE_CHECKLIST.md	Introduce tools module, move generator/TUI code and examples, update many import paths, add golden tests and release checklist.
Explain / Reporting explain/report.go, options.go, loader.go, tests/explain_test.go, explain/report_test.go	Add explain.Report, Entry, Source, Reporter; wire reporter into loader to record provenance, overrides, and redaction notes; expose WithExplainReporter option and tests.
Macro Expansion internal/macros/expand.go, internal/envmacro/expand.go, internal/macros/fuzz_test.go, internal/macros/expand_test.go, tests/envmacro_test.go	New macros package with FILE and ENV macro support, path-aware ExpandNode, FieldError reporting, OnExpand callbacks, and fuzz/unit tests.
Redaction internal/redact/engine.go, internal/redact/reflect.go, redact.go, tests/hardening_test.go	Add ByConvention option, IsSecret/IsConventionSecret, StripIndices, thread redact.Options through recursion, and tests validating convention-based redaction and explain coverage.
Config Schema & Types internal/config/config.go, internal/config/config_test.go, schema/config.schema.json, config.example.yml, examples/dotenv/.env	Add canonical Config types with tags, JSON Schema file, example config, and tests ensuring YAML/schema/struct alignment.
Loader & Core Flow loader.go, internal/decode/strict.go, internal/envmacro/...	Refactor loader to integrate macros.ExpandNode, explain reporting, profile override collection, FieldError wrapping for richer error metadata, and strict-decode error wrapping.
Error & FieldError errors.go, tests/loader_test.go, tests/strict_test.go, tests/*	Extend FieldError with Line, Column, Layer, Path; update Error() formatting; adjust tests to assert new metadata.
Testing & Benchmarks internal/bench/yaml_bench_test.go, internal/dotenv/fuzz_test.go, internal/profiles/fuzz_test.go, tests/integration/*, tests/*.go, tools/tests/*	Add YAML benchmarks, fuzz tests, extensive integration tests (with testdata), golden-file tests, and expanded unit tests.
Generator Improvements tools/generator/registry/registry.go, tools/generator/yamltemplate/*, tools/internal/tools/generator/commands/*	Sort registry.List output, capture options tag into template metadata, add allowed-values comment generation, and update generator command imports.
go.mod Changes go.mod, tools/go.mod	Lower root module Go directive to 1.22 and simplify requirements to YAML v3; add tools module with its own go.mod and dependencies.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Loader as goconfy.Load
    participant Parser as YAML Parser
    participant Macros as macros.ExpandNode
    participant Reporter as explain.Report
    participant Redact as redact.Apply
    participant Decode as Decoder

    App->>Loader: Load(file, options)
    Loader->>Parser: parse YAML -> *yaml.Node*
    Parser-->>Loader: node
    Loader->>Macros: ExpandNode(node, ExpandOptions{LookupEnv, AllowedKeys, OnExpand})
    Macros-->>Loader: expanded node (+ OnExpand callbacks)
    Loader->>Reporter: AddEntry(path, source, value, isSecret)
    Loader->>Decode: decode expanded node -> config struct
    Decode-->>Loader: struct or FieldError
    Loader->>Redact: Apply(struct, redact.Options{ByConvention...})
    Redact-->>Loader: redacted representation
    Loader->>Reporter: WriteJSON/WriteText (final report)
    Loader-->>App: config (validated) + optional report

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I nibbled through YAML, secrets hid from sight,

Files and ENV expanded, each source set right.
Reports that tell the story, redaction keeps it neat,
Tools tucked in their module, CI keeps the beat.
Hooray — a hop, a test, a tidy little feat!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title "New checks" is vague and generic, using non-descriptive language that does not convey meaningful information about the substantial changeset.	Provide a more descriptive title that highlights the primary changes, such as: "Add CI workflow, modularize tools, and enforce repository hygiene" or "Introduce GitHub Actions CI and separate tools module".

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch newChecks

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR restructures the repo into a minimal core Go module plus an optional tools/ Go module, while adding CI/Makefile enforcement and expanding functionality around macros, redaction, and explain/reporting.

Changes:

• Introduces a separate tools/ Go module for generator + TUI, updates import paths, adds examples and golden tests.
• Adds/updates core features: {FILE:...} macros, redaction-by-convention, and an explain/reporting pipeline; improves strict decode error detail.
• Adds repository hygiene enforcement: GitHub Actions CI workflow, make verify, updated docs/config policy + schema + example config, and additional tests (integration/fuzz/bench).

Reviewed changes

Copilot reviewed 79 out of 97 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
.github/workflows/ci.yml	Adds CI pipeline (binary guard, fmt/tidy/tests/race/lint/vulncheck) for root + tools modules.
.gitignore	Expands ignore rules for binaries/artifacts and temp outputs.
CHANGELOG.md	Documents new hygiene/CI/tools split and feature changes under Unreleased.
CONTRIBUTING.md	Updates build instructions and adds repository hygiene rules.
Makefile	Adds local verification targets mirroring CI and tools build.
README.md	Updates feature list and documents tools module split + install paths.
RELEASE.md	Adds release guide (German).
config.example.yml	Adds canonical example configuration aligned with policy and schema.
docs/CI.md	Documents CI checks and local make equivalents (German).
docs/CLI.md	Updates generator install/build paths to tools/ module.
docs/CONFIG_POLICY.md	Adds config-policy document (German).
docs/GENERATOR.md	New docs for generator/TUI usage under separate tools module.
docs/GETTING_STARTED.md	Updates generator/TUI build/install paths and references.
docs/INSTALL_TOOLS.md	New doc explaining core-only vs tools install/build/run.
docs/SECURITY_MODEL.md	Updates security model to include {FILE:...} macros and redaction-by-convention.
errors.go	Extends FieldError with Layer/Path/Line/Column and improves formatting.
examples/dotenv/.env	Tweaks example env value.
explain/report.go	Introduces explain report model + text/JSON output.
go.mod	Lowers root module Go version and trims deps to core-only (yaml.v3).
go.sum	Removes tool-related deps from root module sum file.
internal/bench/yaml_bench_test.go	Adds YAML parsing/decoding benchmarks.
internal/config/config.go	Adds canonical Config struct with tags/options/examples.
internal/config/config_test.go	Validates config.example.yml ↔ struct ↔ JSON schema sync.
internal/decode/strict.go	Wraps strict decode unknown-field errors with line/field details.
internal/dotenv/fuzz_test.go	Adds fuzz test for dotenv parsing.
internal/envmacro/expand.go	Reimplements envmacro expansion via unified macros package.
internal/macros/expand.go	Adds new macro engine supporting {ENV:...} + {FILE:...} expansion.
internal/macros/fuzz_test.go	Adds fuzz test to ensure macro expansion doesn’t panic.
internal/profiles/fuzz_test.go	Adds fuzz test coverage for profile deep merge/apply.
internal/redact/engine.go	Adds ByConvention option and IsSecret helper for secret detection.
internal/redact/reflect.go	Threads redact options through reflection redaction and adds convention redaction.
loader.go	Integrates macros engine, explain reporting, redaction-by-convention, and improved error layering.
options.go	Adds options for explain reporter and redaction-by-convention (load + Redacted call).
redact.go	Passes convention option through to internal redaction engine.
schema/config.schema.json	Adds JSON schema for the canonical config.
tests/envmacro_test.go	Adds assertions for path/line metadata and deep-path macro errors.
tests/explain_test.go	Adds tests for explain reporting and secret redaction behavior.
tests/hardening_test.go	Adds tests for convention-based redaction, file macros, and explain + file/convention.
tests/integration/integration_test.go	Adds full pipeline integration tests incl. profiles, macros, redaction, explain.
tests/integration/testdata/.env	New integration test dotenv file.
tests/integration/testdata/base.yml	New integration base YAML with env/file macros + profiles.
tests/integration/testdata/db_url.txt	New integration file-macro fixture.
tests/loader_test.go	Adjusts MultiError assertion to be Go-version agnostic.
tests/strict_test.go	Strengthens strict-mode error assertions and adds layer/path checks.
tools/cmd/goconfygen/main.go	Switches CLI entrypoint to tools module command implementation.
tools/cmd/goconfytui/main.go	Updates install path comment and imports to tools TUI app.
tools/docs/RELEASE_CHECKLIST.md	Adds tools-aware release checklist.
tools/examples/generator/.env	Adds generator example dotenv.
tools/examples/generator/README.md	Updates run instructions for tools module.
tools/examples/generator/config.yml	Adds generator example config output.
tools/examples/generator/register.go	Updates registry/yamltemplate import paths to tools module.
tools/examples/tui/.env	Adds TUI example dotenv.
tools/examples/tui/README.md	Updates paths/run instructions for tools module layout.
tools/examples/tui/config.yml	Adds TUI example config.
tools/generator/registry/doc.go	Updates doc import path to tools registry.
tools/generator/registry/registry.go	Sorts provider IDs for deterministic listing.
tools/generator/yamltemplate/comments.go	Adds options tag emission to comments.
tools/generator/yamltemplate/defaults.go	New helper for defaults and macro emission.
tools/generator/yamltemplate/doc.go	New package documentation for yamltemplate.
tools/generator/yamltemplate/emit.go	New yaml template emitter (YAML + profiles skeleton + dotenv support).
tools/generator/yamltemplate/reflect.go	Adds options tag extraction.
tools/go.mod	New tools module definition and dependencies.
tools/go.sum	New tools module sum file.
tools/internal/tools/generator/commands/doc.go	Updates docs to point at tools registry path.
tools/internal/tools/generator/commands/dump.go	Updates registry import path to tools module.
tools/internal/tools/generator/commands/fmt.go	Adds fmt subcommand implementation.
tools/internal/tools/generator/commands/init.go	Updates registry/yamltemplate import paths to tools module.
tools/internal/tools/generator/commands/paths.go	Adds shared path resolution helpers for commands.
tools/internal/tools/generator/commands/validate.go	Updates registry import path to tools module.
tools/internal/tools/tui/app/model.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/app/routes.go	Adds screen routing enum/constants.
tools/internal/tools/tui/components/confirm.go	Updates style import path for tools module.
tools/internal/tools/tui/components/footer_help.go	Updates style import path for tools module.
tools/internal/tools/tui/components/header.go	Updates style import path for tools module.
tools/internal/tools/tui/components/inputs.go	Adds shared text input helper.
tools/internal/tools/tui/components/list.go	Updates style import path for tools module.
tools/internal/tools/tui/components/viewport.go	Adds shared viewport helper.
tools/internal/tools/tui/logic/operations.go	Updates imports to tools registry/yamltemplate and tools state pkg.
tools/internal/tools/tui/logic/preview.go	Adds YAML preview redaction + secret detection heuristics.
tools/internal/tools/tui/screens/dump.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/screens/filepicker.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/screens/fmt.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/screens/home.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/screens/init.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/screens/inspect.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/screens/settings.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/screens/validate.go	Updates imports to tools TUI packages.
tools/internal/tools/tui/state/config.go	Adds shared TUI config state with defaults.
tools/internal/tools/tui/style/theme.go	Adds centralized lipgloss theme styles.
tools/tests/golden_test.go	Adds golden tests for generator commands and outputs.
tools/tests/registry_test.go	Updates registry import to tools module path.
tools/tests/testdata/golden/dump-out.json	Adds golden output for dump.
tools/tests/testdata/golden/fmt-out.yml	Adds golden output for fmt.
tools/tests/testdata/golden/init-config.yml	Adds golden output for init config template.
tools/tests/testdata/golden/init.env	Adds golden output for init dotenv.
tools/tests/tui_logic_test.go	Updates TUI logic/state imports to tools module paths.
tools/tests/yamltemplate_test.go	Updates yamltemplate import to tools module path.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 19

🧹 Nitpick comments (13)

internal/profiles/fuzz_test.go (1)

49-53: Consider adding profile-structured seeds for ApplyProfile coverage.

The current seeds lack a profiles: key, so ApplyProfile likely short-circuits or returns an error for all inputs without exercising its merge logic. Adding a seed with profile structure would improve coverage.

💡 Example seed with profile structure

 	seeds := []struct {
 		target   string
 		override string
 	}{
 		{"k1: v1", "k1: v2"},
 		{"k1: v1", "k2: v2"},
 		{"k1: {s1: v1}", "k1: {s1: v2}"},
 		{"k1: {s1: v1}", "k1: {s2: v2}"},
 		{"k1: v1", "k1: {s1: v1}"},
 		{"k1: {s1: v1}", "k1: v1"},
 		{"k1: [v1]", "k1: [v2]"},
 		{"", "k1: v1"},
 		{"k1: v1", ""},
+		{"k1: v1\nprofiles:\n  dev:\n    k1: v2", ""},
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/profiles/fuzz_test.go` around lines 49 - 53, The fuzz test currently
only feeds arbitrary YAML without a profiles: section so ApplyProfile(&rootNode,
profileName) never exercises its merge path; add at least one structured seed
string for targetYaml that includes a top-level "profiles:" mapping with an
entry matching profileName (e.g., profiles: {<profileName>: {<fields>}}) and
some fields that will be merged/overridden so ApplyProfile's merge logic is
triggered; update the test seeds in internal/profiles/fuzz_test.go to include
this profile-structured YAML (using the existing rootNode, targetYaml and
profileName variables) and assert or otherwise observe the mutated rootNode to
ensure coverage of ApplyProfile's behavior.

examples/dotenv/.env (1)

1-4: Optional: Consider alphabetical key ordering.

The static analysis tool suggests alphabetical ordering of keys (APP_VERSION_DISPLAY before GRPC_PORT). While the current grouping (APP_* and GRPC_* keys) has its own logic, alphabetical ordering can improve scannability in example files that others might reference.

📝 Alphabetically sorted alternative

 APP_NAME="todo-service-dev"
+APP_VERSION_DISPLAY=true
 GRPC_PORT=60051
 GRPC_TIMEOUT="5s"
-APP_VERSION_DISPLAY=true

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@examples/dotenv/.env` around lines 1 - 4, Reorder the environment keys in the
example .env so they are alphabetically sorted for readability: place APP_NAME,
APP_VERSION_DISPLAY, GRPC_PORT, GRPC_TIMEOUT (i.e., move APP_VERSION_DISPLAY
before GRPC_PORT and ensure GRPC_TIMEOUT follows GRPC_PORT alphabetically) while
preserving the same values; update the file content where the keys APP_NAME,
APP_VERSION_DISPLAY, GRPC_PORT, and GRPC_TIMEOUT appear.

RELEASE.md (1)

15-18: Consider using make verify as the primary pre-release gate.

Using the consolidated target keeps local release prep aligned with CI as checks evolve.

Suggested wording tweak

-    - Stelle sicher, dass alle Tests lokal bestehen: `make test test-race`.
-    - Führe den Linter aus: `make lint`.
-    - Überprüfe auf Sicherheitslücken: `make vulncheck`.
+    - Führe die vollständigen CI-nahen Checks lokal aus: `make verify`.
+    - Optional einzeln: `make test test-race`, `make lint`, `make vulncheck`.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@RELEASE.md` around lines 15 - 18, Replace the explicit pre-release checklist
items (`make test test-race`, `make lint`, `make vulncheck`, update
`CHANGELOG.md`) with a single recommendation to run the consolidated target
`make verify` as the primary pre-release gate, and optionally note that `make
verify` currently runs those checks so maintainers can still run the individual
commands if needed; update the sentence in RELEASE.md to instruct developers to
run `make verify` to keep local prep aligned with CI.

Makefile (2)

13-13: Consider adding a clean target.

A clean target is a common Makefile convention for removing build artifacts. This would complement build-tools by allowing developers to reset the build state.

🧹 Proposed addition after line 13

## clean: Remove build artifacts
clean:
	rm -rf tools/bin

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Makefile` at line 13, Add a new Makefile target named clean to remove build
artifacts and complement the existing build-tools target: define a target called
clean that removes the tools/bin output directory (e.g., rm -rf tools/bin) so
developers can reset the build state; reference the existing build-tools
workflow and the cd tools && go build -o bin/goconfytui ./cmd/goconfytui command
to ensure the clean target removes the produced bin artifacts.

---

1-1: Add missing targets to .PHONY declaration.

The .PHONY declaration is missing all, build-tools, and update-golden. This can cause issues if files with these names ever exist in the directory.

📦 Proposed fix

-.PHONY: test test-race lint fmt-check vulncheck verify help
+.PHONY: all verify build-tools test test-race lint fmt-check vulncheck update-golden help clean

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Makefile` at line 1, The .PHONY line is missing targets that should be
treated as phony; update the .PHONY declaration (which currently lists test
test-race lint fmt-check vulncheck verify help) to also include the targets all,
build-tools, and update-golden so those names are always treated as phony (e.g.,
add all build-tools update-golden to the .PHONY list).

tools/go.mod (1)

9-9: Pseudo-version dependency on goConfy core.

The tools module depends on github.com/keksclan/goConfy v0.0.0-20260219103719-a3b470266ef3. This is a pseudo-version pointing to a specific commit. Consider updating to a tagged version once the core module is released to ensure version stability and reproducible builds.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/go.mod` at line 9, tools/go.mod currently pins
github.com/keksclan/goConfy to a pseudo-version
(v0.0.0-20260219103719-a3b470266ef3); update that dependency to a released
semver tag when available (e.g., replace the pseudo-version with the official
tag like v1.2.0) to ensure reproducible builds, or temporarily add a go.mod
replace directive pointing to a stable module path if you must control the
commit (use go get github.com/keksclan/goConfy@<tag> or add a replace for
github.com/keksclan/goConfy => github.com/keksclan/goConfy <tag/commit>), and
then run go mod tidy to update tools/go.mod and tools/go.sum accordingly.

internal/bench/yaml_bench_test.go (1)

42-77: Consider validating decode errors during benchmark setup.

While ignoring the decode error inside the benchmark loop (line 76) is acceptable to avoid measurement overhead, consider adding a single validation decode before b.ResetTimer() to confirm the Config struct matches config.example.yml. This ensures the benchmark measures strict decoding of valid data rather than repeated decode failures.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/bench/yaml_bench_test.go` around lines 42 - 77, In
BenchmarkStrictDecodingV3 add a one-time validation decode before
b.ResetTimer(): create a yaml.NewDecoder(bytes.NewReader(data)), call
dec.KnownFields(true) and dec.Decode(&cfg) into a Config instance and if err !=
nil call b.Fatalf("strict decode validation failed: %v", err) (or b.Fatal) to
fail the benchmark setup; leave the existing loop unchanged so the timed section
still only benchmarks repeated dec.Decode calls.

internal/envmacro/expand.go (1)

16-24: Documentation may need updating for FILE macro support.

The docstring states this function "expands environment macros on scalar values that exactly match the {ENV:KEY:default} pattern." However, the delegation to macros.ExpandNode now also expands {FILE:...} macros. This could be intentional (unified expansion) or a behavioral change that should be documented.

Consider updating the docstring

-// ExpandNode recursively walks a yaml.Node tree and expands environment macros
-// on scalar values that exactly match the {ENV:KEY:default} pattern.
+// ExpandNode recursively walks a yaml.Node tree and expands macros
+// on scalar values, including {ENV:KEY:default} and {FILE:/path:default} patterns.
 func ExpandNode(node *yaml.Node, opts ExpandOptions) error {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/envmacro/expand.go` around lines 16 - 24, The docstring for
ExpandNode is outdated: it only mentions expanding "{ENV:KEY:default}" but the
implementation delegates to macros.ExpandNode which also supports "{FILE:...}"
macros; update the comment on the ExpandNode function to reflect that it expands
both ENV and FILE macros (or more generally "environment and file macros" or the
unified macro syntax supported by macros.ExpandNode), and mention that it only
operates on scalar values matching the macro pattern and that behavior is driven
by ExpandOptions (Lookup, Prefix, AllowedKeys, OnExpand).

explain/report.go (1)

59-81: Consider using a map for O(1) entry lookup.

The current implementation scans the Entries slice linearly to find existing paths. For configurations with many entries, this could become slow. Consider maintaining an internal map[string]int index for O(1) lookups while preserving ordered Entries slice for output.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@explain/report.go` around lines 59 - 81, The linear scan over r.Entries to
find an entry by path is O(n); add a lookup map on the report struct (e.g.,
index map[string]int) and refactor the add/update logic to use the map for O(1)
existence checks and index retrieval instead of the for loop iterating
r.Entries; ensure you update the index whenever you append a new entry
(preserving the ordered r.Entries slice for output) and keep the existing merge
logic that updates OverriddenBy, ValueRedacted, Source (including the SourceBase
-> source promotion) and Notes so behavior remains identical.

internal/macros/expand.go (1)

80-87: Simplify default detection using regex capture group.

The colon-counting logic duplicates what the regex already captures. matches[2] is non-empty only when a default is provided (second capture group). This could be simplified:

Proposed simplification

 	if matches := FileMacroRegex.FindStringSubmatch(node.Value); matches != nil {
 		filePath := matches[1]
 		defaultVal := matches[2]
-		// Count colons: {FILE:path} has 1, {FILE:path:} or {FILE:path:default} has 2+.
-		colons := 0
-		for _, ch := range matches[0] {
-			if ch == ':' {
-				colons++
-			}
-		}
-		hasDefault := colons >= 2
+		// matches[2] is only non-empty when `:default` part is present
+		hasDefault := len(matches) > 2 && matches[0][len(matches[0])-1] != '}'  || matches[2] != "" || strings.Count(matches[0], ":") >= 2

Actually, the regex (?::([^}]*))? makes matches[2] empty string even for {FILE:path:}. The colon counting correctly distinguishes {FILE:path} (no default) from {FILE:path:} (empty default). Consider adding a comment explaining this nuance.

On reflection, the colon counting is necessary to distinguish {FILE:path} from {FILE:path:}. A clarifying comment would help maintainability.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/macros/expand.go` around lines 80 - 87, The colon-counting branch
(variables colons, hasDefault) is necessary because the regexp's optional
capture `(?::([^}]*))?` yields an empty matches[2] for both `{FILE:path}` and
`{FILE:path:}`, so you must keep the current logic but add a clear comment above
the loop explaining that nuance: state that matches[2] can be empty even when a
trailing colon exists, therefore we count ':' characters in matches[0] to
distinguish `{FILE:path}` (no default) from `{FILE:path:}` (explicit empty
default); reference the variables matches, colons, and hasDefault in the comment
so future maintainers understand why the regex alone is insufficient.

internal/decode/strict.go (2)

43-58: Consider unifying with the public FieldError type.

The fieldError type in this package duplicates the structure of goconfy.FieldError (from errors.go). This creates two similar error types with identical semantics, which could lead to confusion. Consider returning *goconfy.FieldError directly or aliasing to avoid duplication.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/decode/strict.go` around lines 43 - 58, The local type fieldError
duplicates goconfy.FieldError; replace the local definition by using the public
type to avoid duplicate semantics: remove the fieldError struct and its methods
and either (a) change references to return *goconfy.FieldError directly, or (b)
declare type fieldError = goconfy.FieldError as an alias, and update Error(),
GetField() and GetLine() usages to use the methods on goconfy.FieldError (or
rely on the alias). Ensure constructors/return sites that created
&fieldError{...} now create &goconfy.FieldError{...} (or &fieldError{...} if
aliasing) so callers and error interface implementations remain correct.

---

12-12: Document the undocumented yaml.v3 error message format dependency.

The regex parsing at line 12 relies on gopkg.in/yaml.v3's error message format (line N: field X not found in type Y). The library does not expose structured error types with line and field information—it only returns plain error strings with embedded location data. Since this format is undocumented and could change in future versions, add a comment explaining this dependency and consider adding a unit test that validates the regex against real yaml.v3 error outputs to catch any format changes early.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/decode/strict.go` at line 12, The regex yamlErrRegex in strict.go
depends on the undocumented error string format emitted by gopkg.in/yaml.v3
("line N: field X not found in type Y"); add a clear comment above the
yamlErrRegex declaration documenting this dependency and the risk of format
changes, and add a unit test (e.g., TestYamlV3ErrorFormat) that generates a real
yaml.v3 parse error and asserts the regex still matches the produced error
string so any future format changes are detected early.

internal/redact/engine.go (1)

33-36: Avoid rebuilding pathSet on every IsSecret call.

Lines 33-36 allocate and populate a map each invocation. In recursive redaction paths this can add unnecessary overhead; consider precomputing once and reusing it through the traversal.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/redact/engine.go` around lines 33 - 36, Move the one-off creation of
the path set out of IsSecret and build it once before traversal (e.g., in the
constructor/NewEngine or at the start of Redact) using opts.Paths, storing the
resulting map on the Engine/Redactor struct (e.g., engine.pathSet
map[string]bool). Update IsSecret to consult that cached engine.pathSet instead
of recreating the map each call, and remove the per-call allocation of pathSet;
ensure the stored map is populated from opts.Paths and accessible to recursive
traversal functions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 51-57: The "Check Formatting (gofmt)" step currently runs "gofmt
-l ." which misses files under tools/; update the step that runs gofmt
(referenced by the step name "Check Formatting (gofmt)" and the command "gofmt
-l .") to use a recursive pattern such as "gofmt -l ./..." (or explicitly
include tools/) so the formatting check covers all modules including tools/, and
ensure the existing conditional and error output logic remains the same.
- Around line 120-129: The CI workflow currently pins golangci-lint to the
unsupported version "v1.64" in the job steps named "golangci-lint (Root)" and
"golangci-lint (Tools)"; update both steps to a recent supported release (e.g.,
"v2.10.1" or the latest stable tag) by changing the with.version value for the
golangci/golangci-lint-action@v6 uses so the workflow runs a supported linter
binary and avoids the cancelled/failed release.

In `@docs/CLI.md`:
- Line 12: Update the CLI build examples so the Go binary is placed under the
tools-module layout instead of the repo root: replace the build invocation that
currently uses "-o ../goconfygen" (the example "(cd tools && go build -o
../goconfygen ./cmd/goconfygen)") with an output path into tools/bin (e.g. "-o
tools/bin/goconfygen"), and make the same change for the second occurrence of
that example later in the file.

In `@docs/GENERATOR.md`:
- Around line 19-20: Update the two build commands that currently write binaries
to the repo root so they output into tools/bin/ instead: change the (cd tools &&
go build -o ../goconfygen ./cmd/goconfygen) and (cd tools && go build -o
../goconfytui ./cmd/goconfytui) invocations to write to ../tools/bin/goconfygen
and ../tools/bin/goconfytui respectively, and ensure the tools/bin directory is
created or documented before these commands run.

In `@docs/SECURITY_MODEL.md`:
- Around line 15-21: Update the unlabeled fenced code block containing the ENV
and FILE regexes so it includes a language hint (e.g., "regex"); locate the
block that contains the lines starting with "# ENV" and the patterns
"^\{ENV:([A-Z0-9_]+)(?::([^}]*))?\}$" and "^\{FILE:([^:]+)(?::([^}]*))?\}$" and
change the opening triple backticks to include the language hint (```regex) to
enable proper Markdown linting and editor highlighting.

In `@errors.go`:
- Around line 32-34: The error formatting writes "col 0" when Column is unset;
update the logic in the error formatting function (the block using e.Line,
e.Column, sb.WriteString and fmt.Sprintf) to only append the column portion when
e.Column > 0 (i.e., include "col %d" conditionally), leaving the existing "line
%d" behavior unchanged so no "col 0" is printed for unknown column values.

In `@explain/report.go`:
- Around line 92-108: The WriteText method on Report ignores errors from its
fmt.Fprintf calls; update Report.WriteText to capture and handle each
fmt.Fprintf (and any other write) error by checking the returned (int, error)
and returning the first non-nil error encountered. Locate the fmt.Fprintf calls
in WriteText (header, separator, and per-entry line) and replace the ignored
calls with error checks (e.g., if _, err := fmt.Fprintf(...); err != nil {
return err }) so the function returns write errors instead of always returning
nil.

In `@internal/bench/yaml_bench_test.go`:
- Around line 14-20: The benchmarks (BenchmarkParseSmallV3,
BenchmarkParseMediumV3, and BenchmarkStrictDecodingV3) are ignoring os.ReadFile
errors; update each to check the error returned from os.ReadFile and call
b.Fatal(err) (or b.Fatalf with context) if non-nil before ResetTimer so the
benchmark never runs on missing/invalid data. Locate the file-read lines in
those functions and add the error handling right after the read, ensuring the
benchmark uses valid data.

In `@internal/config/config_test.go`:
- Around line 123-127: TestNoHiddenConfig is currently a no-op and should either
perform its intended verification or be explicitly skipped; update the test so
it either (A) implements a read-only scanner that walks the repository, finds
os.Getenv calls and asserts each env key appears in our canonical Config
representation (use the Config type or the function that returns the canonical
config) and fails the test on mismatches, or (B) if you don't want the scanner
now, replace the empty body with t.Skip("skipping TestNoHiddenConfig: scanner
not implemented") so test output accurately reflects that it's intentionally
skipped; reference the TestNoHiddenConfig function, os.Getenv occurrences, and
the canonical Config/type when making the change.

In `@loader.go`:
- Around line 36-37: The current check unconditionally skips any key named
"profiles" (variable key), which hides nested keys like "app.profiles"; update
the condition so "profiles" is only excluded when it's a top-level key. Change
the if that tests key == "profiles" to also verify the current context/path is
top-level (e.g., parent path is empty or path segment count == 1) before
continuing, using the existing context variable that holds the current key path
(or split the current path string into segments and check length). Ensure nested
keys named "profiles" are not skipped so they will appear in the explain output.

In `@Makefile`:
- Around line 31-36: The fmt-check target only runs gofmt on the root module;
update the fmt-check target (target name: fmt-check) to run gofmt -l on both the
root and the tools module (for example by invoking gofmt -l . tools or otherwise
listing both modules) and use that combined command in the conditional ($$(gofmt
-l ...)) so the check fails if any file in either module is unformatted.

In `@README.md`:
- Line 10: The README’s feature list mentions the {FILE:/path:default} macro but
the Macros section only documents {ENV:KEY:default}; update the Macros section
to document {FILE:/path:default} alongside {ENV:KEY:default}, describing
exact-match semantics, security guarantees (no shell injection), default value
handling, and provide one or two usage examples showing path lookup and fallback
to default; ensure the macro parsing rules and examples match the brief feature
list wording so both sections stay consistent.
- Around line 321-323: Update the build commands so the Go binaries are placed
into the repo's tools/bin directory instead of the tools module root: change the
two commands that currently run "(cd tools && go build -o goconfygen
./cmd/goconfygen)" and "(cd tools && go build -o goconfytui ./cmd/goconfytui)"
to output to "tools/bin/goconfygen" and "tools/bin/goconfytui" respectively, and
ensure the build step creates the tools/bin directory if it doesn’t exist (e.g.,
mkdir -p tools/bin before running go build).
- Around line 26-41: The README contains conflicting Go minimum versions: the
header string "Requires **Go 1.22+**" and the Compatibility section text that
mentions "Go 1.26+"; pick a single minimum (recommend normalizing to the higher
floor, e.g., Go 1.26+) and update every occurrence in the document so the header
"Requires **Go X.Y+**" and the Compatibility/Versioning section consistently
state that same version; search for the literal strings "Requires **Go", "Go
1.22", and "Go 1.26" and replace them to ensure a single, consistent version
floor across the README.

In `@tests/hardening_test.go`:
- Around line 33-34: Replace the unchecked type assertions on the Redacted
results with checked assertions: after calling goconfy.Redacted(cfg) and
goconfy.Redacted(cfg2) (the variables currently cast via
redacted.(map[string]any) and similar at the other sites), use the comma-ok form
to verify the value is a map[string]any and call t.Fatalf with a clear message
if the assertion fails; do this for the occurrences around the current casts
(the two-value assertion should assign the map and ok, and fatal when !ok) so
the test fails with a clear error instead of panicking.

In `@tests/integration/integration_test.go`:
- Around line 258-268: The test currently iterates capturedReport.Entries and
only validates secrecy/redaction if an entry is present, allowing missing
entries to pass; update the ExplainRedaction subtest to first assert that
entries with Path "db.password" and "secret_key" exist in capturedReport.Entries
(e.g., find/count them) and fail the test if missing, then continue to assert
entry.IsSecret and entry.ValueRedacted == "[REDACTED]"; reference the
capturedReport.Entries slice and the loop that checks entry.Path ==
"db.password" || entry.Path == "secret_key" to locate where to add presence
assertions.

In `@tools/examples/tui/README.md`:
- Around line 17-26: Update the walkthrough file paths so they are relative to
the working directory used to start the app (the tools/ directory);
specifically, change the Config YAML and .env examples used in the "Enter paths
in the file picker" step so they point to examples/tui/config.yml and
examples/tui/.env (instead of tools/examples/tui/...), ensuring references in
README.md's walkthrough match starting the app with (cd tools && go run
./cmd/goconfytui).

In `@tools/tests/golden_test.go`:
- Around line 76-86: The test ignores the os.Pipe() error and only restores
os.Stdout after calling commands.RunDump, making it brittle on failures; change
the setup in TestGolden (or the test function containing the snippet) to check
and handle the error returned by os.Pipe(), and ensure os.Stdout is always
restored by capturing the original stdout and deferring its restoration
immediately after successful pipe creation (and closing the writer/reader via
defer as appropriate) so commands.RunDump(args) runs with the temporary pipe but
stdout is restored even if RunDump or other steps fail; reference the os.Pipe()
call and the os.Stdout assignment, and the use of r, w and commands.RunDump to
locate the code to modify.

---

Nitpick comments:
In `@examples/dotenv/.env`:
- Around line 1-4: Reorder the environment keys in the example .env so they are
alphabetically sorted for readability: place APP_NAME, APP_VERSION_DISPLAY,
GRPC_PORT, GRPC_TIMEOUT (i.e., move APP_VERSION_DISPLAY before GRPC_PORT and
ensure GRPC_TIMEOUT follows GRPC_PORT alphabetically) while preserving the same
values; update the file content where the keys APP_NAME, APP_VERSION_DISPLAY,
GRPC_PORT, and GRPC_TIMEOUT appear.

In `@explain/report.go`:
- Around line 59-81: The linear scan over r.Entries to find an entry by path is
O(n); add a lookup map on the report struct (e.g., index map[string]int) and
refactor the add/update logic to use the map for O(1) existence checks and index
retrieval instead of the for loop iterating r.Entries; ensure you update the
index whenever you append a new entry (preserving the ordered r.Entries slice
for output) and keep the existing merge logic that updates OverriddenBy,
ValueRedacted, Source (including the SourceBase -> source promotion) and Notes
so behavior remains identical.

In `@internal/bench/yaml_bench_test.go`:
- Around line 42-77: In BenchmarkStrictDecodingV3 add a one-time validation
decode before b.ResetTimer(): create a yaml.NewDecoder(bytes.NewReader(data)),
call dec.KnownFields(true) and dec.Decode(&cfg) into a Config instance and if
err != nil call b.Fatalf("strict decode validation failed: %v", err) (or
b.Fatal) to fail the benchmark setup; leave the existing loop unchanged so the
timed section still only benchmarks repeated dec.Decode calls.

In `@internal/decode/strict.go`:
- Around line 43-58: The local type fieldError duplicates goconfy.FieldError;
replace the local definition by using the public type to avoid duplicate
semantics: remove the fieldError struct and its methods and either (a) change
references to return *goconfy.FieldError directly, or (b) declare type
fieldError = goconfy.FieldError as an alias, and update Error(), GetField() and
GetLine() usages to use the methods on goconfy.FieldError (or rely on the
alias). Ensure constructors/return sites that created &fieldError{...} now
create &goconfy.FieldError{...} (or &fieldError{...} if aliasing) so callers and
error interface implementations remain correct.
- Line 12: The regex yamlErrRegex in strict.go depends on the undocumented error
string format emitted by gopkg.in/yaml.v3 ("line N: field X not found in type
Y"); add a clear comment above the yamlErrRegex declaration documenting this
dependency and the risk of format changes, and add a unit test (e.g.,
TestYamlV3ErrorFormat) that generates a real yaml.v3 parse error and asserts the
regex still matches the produced error string so any future format changes are
detected early.

In `@internal/envmacro/expand.go`:
- Around line 16-24: The docstring for ExpandNode is outdated: it only mentions
expanding "{ENV:KEY:default}" but the implementation delegates to
macros.ExpandNode which also supports "{FILE:...}" macros; update the comment on
the ExpandNode function to reflect that it expands both ENV and FILE macros (or
more generally "environment and file macros" or the unified macro syntax
supported by macros.ExpandNode), and mention that it only operates on scalar
values matching the macro pattern and that behavior is driven by ExpandOptions
(Lookup, Prefix, AllowedKeys, OnExpand).

In `@internal/macros/expand.go`:
- Around line 80-87: The colon-counting branch (variables colons, hasDefault) is
necessary because the regexp's optional capture `(?::([^}]*))?` yields an empty
matches[2] for both `{FILE:path}` and `{FILE:path:}`, so you must keep the
current logic but add a clear comment above the loop explaining that nuance:
state that matches[2] can be empty even when a trailing colon exists, therefore
we count ':' characters in matches[0] to distinguish `{FILE:path}` (no default)
from `{FILE:path:}` (explicit empty default); reference the variables matches,
colons, and hasDefault in the comment so future maintainers understand why the
regex alone is insufficient.

In `@internal/profiles/fuzz_test.go`:
- Around line 49-53: The fuzz test currently only feeds arbitrary YAML without a
profiles: section so ApplyProfile(&rootNode, profileName) never exercises its
merge path; add at least one structured seed string for targetYaml that includes
a top-level "profiles:" mapping with an entry matching profileName (e.g.,
profiles: {<profileName>: {<fields>}}) and some fields that will be
merged/overridden so ApplyProfile's merge logic is triggered; update the test
seeds in internal/profiles/fuzz_test.go to include this profile-structured YAML
(using the existing rootNode, targetYaml and profileName variables) and assert
or otherwise observe the mutated rootNode to ensure coverage of ApplyProfile's
behavior.

In `@internal/redact/engine.go`:
- Around line 33-36: Move the one-off creation of the path set out of IsSecret
and build it once before traversal (e.g., in the constructor/NewEngine or at the
start of Redact) using opts.Paths, storing the resulting map on the
Engine/Redactor struct (e.g., engine.pathSet map[string]bool). Update IsSecret
to consult that cached engine.pathSet instead of recreating the map each call,
and remove the per-call allocation of pathSet; ensure the stored map is
populated from opts.Paths and accessible to recursive traversal functions.

In `@Makefile`:
- Line 13: Add a new Makefile target named clean to remove build artifacts and
complement the existing build-tools target: define a target called clean that
removes the tools/bin output directory (e.g., rm -rf tools/bin) so developers
can reset the build state; reference the existing build-tools workflow and the
cd tools && go build -o bin/goconfytui ./cmd/goconfytui command to ensure the
clean target removes the produced bin artifacts.
- Line 1: The .PHONY line is missing targets that should be treated as phony;
update the .PHONY declaration (which currently lists test test-race lint
fmt-check vulncheck verify help) to also include the targets all, build-tools,
and update-golden so those names are always treated as phony (e.g., add all
build-tools update-golden to the .PHONY list).

In `@RELEASE.md`:
- Around line 15-18: Replace the explicit pre-release checklist items (`make
test test-race`, `make lint`, `make vulncheck`, update `CHANGELOG.md`) with a
single recommendation to run the consolidated target `make verify` as the
primary pre-release gate, and optionally note that `make verify` currently runs
those checks so maintainers can still run the individual commands if needed;
update the sentence in RELEASE.md to instruct developers to run `make verify` to
keep local prep aligned with CI.

In `@tools/go.mod`:
- Line 9: tools/go.mod currently pins github.com/keksclan/goConfy to a
pseudo-version (v0.0.0-20260219103719-a3b470266ef3); update that dependency to a
released semver tag when available (e.g., replace the pseudo-version with the
official tag like v1.2.0) to ensure reproducible builds, or temporarily add a
go.mod replace directive pointing to a stable module path if you must control
the commit (use go get github.com/keksclan/goConfy@<tag> or add a replace for
github.com/keksclan/goConfy => github.com/keksclan/goConfy <tag/commit>), and
then run go mod tidy to update tools/go.mod and tools/go.sum accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e01a8bbb-9310-4f99-a898-a96be59fade0

📥 Commits

Reviewing files that changed from the base of the PR and between c5ff408 and 70f3782.

⛔ Files ignored due to path filters (2)

• go.sum is excluded by !**/*.sum
• tools/go.sum is excluded by !**/*.sum

📒 Files selected for processing (95)

• .github/workflows/ci.yml
• .gitignore
• CHANGELOG.md
• CONTRIBUTING.md
• Makefile
• README.md
• RELEASE.md
• config.example.yml
• docs/CI.md
• docs/CLI.md
• docs/CONFIG_POLICY.md
• docs/GENERATOR.md
• docs/GETTING_STARTED.md
• docs/INSTALL_TOOLS.md
• docs/SECURITY_MODEL.md
• errors.go
• examples/dotenv/.env
• explain/report.go
• go.mod
• internal/bench/yaml_bench_test.go
• internal/config/config.go
• internal/config/config_test.go
• internal/decode/strict.go
• internal/dotenv/fuzz_test.go
• internal/envmacro/expand.go
• internal/macros/expand.go
• internal/macros/fuzz_test.go
• internal/profiles/fuzz_test.go
• internal/redact/engine.go
• internal/redact/reflect.go
• loader.go
• options.go
• redact.go
• schema/config.schema.json
• tests/envmacro_test.go
• tests/explain_test.go
• tests/hardening_test.go
• tests/integration/integration_test.go
• tests/integration/testdata/.env
• tests/integration/testdata/base.yml
• tests/integration/testdata/db_url.txt
• tests/loader_test.go
• tests/strict_test.go
• tools/cmd/goconfygen/main.go
• tools/cmd/goconfytui/main.go
• tools/docs/RELEASE_CHECKLIST.md
• tools/examples/generator/.env
• tools/examples/generator/README.md
• tools/examples/generator/config.yml
• tools/examples/generator/register.go
• tools/examples/tui/.env
• tools/examples/tui/README.md
• tools/examples/tui/config.yml
• tools/generator/registry/doc.go
• tools/generator/registry/registry.go
• tools/generator/yamltemplate/comments.go
• tools/generator/yamltemplate/defaults.go
• tools/generator/yamltemplate/doc.go
• tools/generator/yamltemplate/emit.go
• tools/generator/yamltemplate/reflect.go
• tools/go.mod
• tools/internal/tools/generator/commands/doc.go
• tools/internal/tools/generator/commands/dump.go
• tools/internal/tools/generator/commands/fmt.go
• tools/internal/tools/generator/commands/init.go
• tools/internal/tools/generator/commands/paths.go
• tools/internal/tools/generator/commands/validate.go
• tools/internal/tools/tui/app/model.go
• tools/internal/tools/tui/app/routes.go
• tools/internal/tools/tui/components/confirm.go
• tools/internal/tools/tui/components/footer_help.go
• tools/internal/tools/tui/components/header.go
• tools/internal/tools/tui/components/inputs.go
• tools/internal/tools/tui/components/list.go
• tools/internal/tools/tui/components/viewport.go
• tools/internal/tools/tui/logic/operations.go
• tools/internal/tools/tui/logic/preview.go
• tools/internal/tools/tui/screens/dump.go
• tools/internal/tools/tui/screens/filepicker.go
• tools/internal/tools/tui/screens/fmt.go
• tools/internal/tools/tui/screens/home.go
• tools/internal/tools/tui/screens/init.go
• tools/internal/tools/tui/screens/inspect.go
• tools/internal/tools/tui/screens/settings.go
• tools/internal/tools/tui/screens/validate.go
• tools/internal/tools/tui/state/config.go
• tools/internal/tools/tui/style/theme.go
• tools/tests/golden_test.go
• tools/tests/registry_test.go
• tools/tests/testdata/golden/dump-out.json
• tools/tests/testdata/golden/fmt-out.yml
• tools/tests/testdata/golden/init-config.yml
• tools/tests/testdata/golden/init.env
• tools/tests/tui_logic_test.go
• tools/tests/yamltemplate_test.go

[Copilot]

Pull request overview

Copilot reviewed 82 out of 100 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 9

🧹 Nitpick comments (8)

tests/integration/integration_test.go (3)

95-99: Remove redundant type conversion.

DumpRedactedJSON returns (string, error), so redacted is already a string. The conversion on line 99 is unnecessary.

Suggested fix

 	redacted, err := goconfy.DumpRedactedJSON(cfg)
 	if err != nil {
 		t.Fatalf("failed to dump redacted: %v", err)
 	}
-	redactedStr := string(redacted)
-	if strings.Contains(redactedStr, "secret-pass") {
+	if strings.Contains(redacted, "secret-pass") {
 		t.Error("DB password leaked in redacted dump")
 	}
-	if strings.Contains(redactedStr, "env-secret") {
+	if strings.Contains(redacted, "env-secret") {
 		t.Error("Secret key leaked in redacted dump")
 	}
-	if !strings.Contains(redactedStr, "[REDACTED]") {
+	if !strings.Contains(redacted, "[REDACTED]") {
 		t.Error("Redacted dump does not contain [REDACTED] placeholder")
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/integration/integration_test.go` around lines 95 - 99, The variable
redacted is already a string returned by DumpRedactedJSON(cfg), so remove the
redundant conversion on redactedStr (i.e., delete or replace the line that does
redactedStr := string(redacted)); instead use redacted directly (or assign
redactedStr := redacted) where needed; update any usages expecting redactedStr
to reference the existing redacted variable or the new direct assignment.

---

168-200: Same test isolation issue with os.Unsetenv.

Similar to the MissingRequiredEnvMacro test, this subtest uses os.Unsetenv without preserving the original state.

Suggested fix

 	t.Run("DotenvNoMutation", func(t *testing.T) {
 		// Key only in .env, not in OS env
 		key := "ONLY_IN_DOTENV"
-		os.Unsetenv(key)
+		if orig, ok := os.LookupEnv(key); ok {
+			t.Cleanup(func() { os.Setenv(key, orig) })
+		}
+		os.Unsetenv(key)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/integration/integration_test.go` around lines 168 - 200, The
DotenvNoMutation subtest mutates global OS env state by calling os.Unsetenv
without restoring it; capture the original value with os.LookupEnv at the start
of the subtest (inside the t.Run "DotenvNoMutation"), then restore the original
state in a defer or t.Cleanup: if the original was present call os.Setenv(key,
origVal) else call os.Unsetenv(key). Apply the same preserve-and-restore pattern
used in the MissingRequiredEnvMacro test to avoid leaking environment changes
across tests.

---

117-134: Improve test isolation for environment variable unsetting.

Using os.Unsetenv directly doesn't restore the original environment state after the test. If DB_PASSWORD was set in the parent environment, it won't be restored.

The idiomatic pattern for test-safe unsetting:

Suggested fix

 	t.Run("MissingRequiredEnvMacro", func(t *testing.T) {
-		// Clear env
-		os.Unsetenv("DB_PASSWORD")
+		// Clear env safely - register for cleanup first, then unset
+		if orig, ok := os.LookupEnv("DB_PASSWORD"); ok {
+			t.Cleanup(func() { os.Setenv("DB_PASSWORD", orig) })
+		}
+		os.Unsetenv("DB_PASSWORD")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/integration/integration_test.go` around lines 117 - 134, The test
"MissingRequiredEnvMacro" currently calls os.Unsetenv("DB_PASSWORD") without
restoring the original environment; update the test to capture the original
value via os.LookupEnv("DB_PASSWORD"), then unset for the test and defer a
restore that uses os.Setenv to put the original value back if present or
os.Unsetenv if it was absent, ensuring test isolation around the
goconfy.Load[FullConfig] call so other tests are unaffected.

Makefile (1)

35-37: Pin govulncheck versions instead of using @latest in both Makefile and CI workflow.

Using @latest makes CI non-deterministic and can break without a repo change. This affects:

• Makefile (lines 36–37): go run golang.org/x/vuln/cmd/govulncheck@latest
• .github/workflows/ci.yml: go install golang.org/x/vuln/cmd/govulncheck@latest

Consider defining a version variable and applying it consistently across both files.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 35 - 37, The Makefile target vulncheck currently uses
go run golang.org/x/vuln/cmd/govulncheck@latest (and the CI uses go install
...@latest), which makes builds non-deterministic; introduce a pinned
GOVULNCHECK_VERSION variable (e.g., GOVULNCHECK_VERSION := vX.Y.Z) and update
the vulncheck target command to use
golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION), then update the CI
workflow to use the same pinned version (replace `@latest` in go install with
@$(GOVULNCHECK_VERSION) or hardcode the same version) so both Makefile target
(vulncheck) and CI consistently reference the exact govulncheck version.

loader.go (1)

19-56: Dead code at lines 24-26.

The check if path == "profiles" { return } is unreachable. The top-level "profiles" key is already skipped at line 36-37 (key == "profiles" && path == ""), so path can never equal "profiles" when entering this function.

Proposed cleanup

 func collectBaseEntries[T any](node *yaml.Node, report *explain.Report, path string, opts redact.Options) {
 	if node == nil || report == nil {
 		return
 	}
-
-	if path == "profiles" {
-		return
-	}
 
 	switch node.Kind {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@loader.go` around lines 19 - 56, The guard in collectBaseEntries[T] that
returns when path == "profiles" is dead code and should be removed; update the
function collectBaseEntries (the top-level nil checks remain) by deleting the
unreachable if path == "profiles" { return } branch so the behavior relies
solely on the existing mapping-node check (key == "profiles" && path == "") that
skips the top-level profiles key.

explain/report_test.go (1)

35-38: Remove stale debugging comment.

The comment "This is where the bug is expected" appears to be a leftover from TDD or debugging. If the test passes, this comment is confusing and should be removed.

Proposed cleanup

-	// This is where the bug is expected
 	if entry.Source != SourceProfile {
 		t.Errorf("expected Source to be '%s', got '%s'", SourceProfile, entry.Source)
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@explain/report_test.go` around lines 35 - 38, Remove the stale debugging
comment "// This is where the bug is expected" in the test around the assertion
checking entry.Source against SourceProfile; simply delete that comment so the
test only contains the meaningful assertion (the comparison of entry.Source to
SourceProfile and the t.Errorf call) to avoid confusion when the test passes.

tests/explain_test.go (2)

104-114: Test name is misleading.

TestExplainDisabled_NoAllocations suggests allocation verification, but the test only confirms that loading works without a reporter. Consider renaming to TestExplainDisabled_LoadsSuccessfully or adding actual allocation checks via testing.AllocsPerRun.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/explain_test.go` around lines 104 - 114, The test name
TestExplainDisabled_NoAllocations is misleading because it only checks that
goconfy.Load[ExplainConfig] succeeds without a reporter; rename the test to a
clearer name like TestExplainDisabled_LoadsSuccessfully (or alternatively add
real allocation checks using testing.AllocsPerRun if you intended to assert
allocations). Update the test function name TestExplainDisabled_NoAllocations
and any references to it, keeping the body that calls
goconfy.Load[ExplainConfig] with goconfy.WithBytes([]byte(yamlData)) and the
existing error check, or replace the body with a testing.AllocsPerRun invocation
that measures allocations around the goconfy.Load call if you choose to assert
allocation behavior.

---

37-38: Prefer t.Setenv for test environment variables.

Using os.Setenv with defer os.Unsetenv can cause test pollution if tests run in parallel. t.Setenv (available since Go 1.17) is automatically cleaned up and marks the test as incompatible with t.Parallel().

Proposed fix

-	os.Setenv("APP_TOKEN", "super-secret")
-	defer os.Unsetenv("APP_TOKEN")
+	t.Setenv("APP_TOKEN", "super-secret")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/explain_test.go` around lines 37 - 38, Replace the manual environment
setup using os.Setenv("APP_TOKEN", "super-secret") and defer
os.Unsetenv("APP_TOKEN") with t.Setenv("APP_TOKEN", "super-secret") in the test
(tests/explain_test.go) so the test framework automatically restores the
variable and prevents parallel-test pollution; ensure the test function has a
testing.T parameter named t (e.g., func TestXxx(t *testing.T)) before using
t.Setenv and remove the corresponding defer os.Unsetenv call.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 12-126: The CI is missing a lint stage — add a new job (e.g., job
name "lint") or insert steps into the existing "verify" job to run a Go linter;
install and run a linter such as golangci-lint (install via go install
github.com/golangci/golangci-lint/cmd/golangci-lint@latest) and run
golangci-lint run ./... (or use staticcheck similarly) after "Set up Go" and
before running tests, and ensure the job fails the workflow on any linter
errors; reference the existing "verify" and "vulncheck" jobs and the "Check
Formatting (gofmt)" step to determine placement.

In `@CHANGELOG.md`:
- Around line 14-27: The changelog contains duplicate subsection headings ("###
Added" and "### Changed") in the Unreleased section which triggers MD024;
consolidate these duplicates by merging their bullet lists under a single unique
"### Added" and a single unique "### Changed" heading (or rename one set to a
distinct label) so each heading appears only once, preserving all entries and
order (references: the "### Added" and "### Changed" headings and their bullet
items shown in the diff).

In `@docs/SECURITY_MODEL.md`:
- Around line 15-21: The docs' FILE regex example is stricter than the actual
parser: update the SECURITY_MODEL.md FILE regex to match the implementation in
internal/macros/expand.go by allowing colons in path segments (so Windows drive
letters like C:\... are accepted), i.e., change the documented FILE macro
pattern to the same pattern used by the code that parses the FILE macro in
internal/macros/expand.go (the FILE macro parsing/regex) and ensure the examples
mention Windows paths.

In `@internal/bench/yaml_bench_test.go`:
- Around line 20-22: The benchmark currently discards errors from
yamlparse.ParseBytes inside the b.N loop; change each loop to capture the
returned error (e.g., _, err := yamlparse.ParseBytes(data)) and call b.Fatalf or
b.Fatalf("yaml parse error: %v", err) when err != nil so the benchmark fails
fast on parse regressions; apply this change to every benchmark loop that
currently ignores the error (the loops using b.N and calling
yamlparse.ParseBytes).
- Around line 83-85: The benchmark currently ignores decoder errors by
discarding the result of dec.Decode(&cfg); update the loop to check the error
returned from dec.Decode(&cfg) after creating the decoder (yaml.NewDecoder and
dec.KnownFields(true)) and handle/report it (e.g., call b.Fatalf or b.Error) so
invalid decodes fail the benchmark; reference the decoder variable dec, the call
dec.Decode(&cfg), and the cfg target when adding the error check.

In `@internal/macros/expand.go`:
- Around line 121-128: The code currently returns a FieldError when a key is not
in opts.AllowedKeys (using slices.Contains) even if a default exists; change the
logic in the block that references opts.AllowedKeys and slices.Contains so it
only returns the FieldError when the key is disallowed AND there is no
default/fallback available on the node (i.e., preserve fallback-to-default/empty
behavior). Update the condition to check for the node's default/empty value
presence before returning the FieldError (leave FieldError construction with
Path/Line/Column/Message intact when actually failing).

In `@Makefile`:
- Line 1: Add all non-file Makefile targets to the .PHONY declaration to avoid
false up-to-date behavior: update the existing .PHONY line (which currently
lists test test-race fmt-check vulncheck verify help) to also include all,
build-tools, and update-golden so that targets named "all", "build-tools", and
"update-golden" are always run rather than treated as files.
- Around line 16-24: Update the Go version directive in the tools module so CI
on Go 1.22 can build: open the tools module's go directive that currently reads
"go 1.26" and change it to "go 1.22" (matching the root module), then run `go
mod tidy` in the tools module to ensure the module file is consistent.

In `@README.md`:
- Around line 127-132: The unlabeled fenced code block showing macro syntax (the
block containing "{ENV:KEY}", "{ENV:KEY:default}", "{FILE:/path/to/file}",
"{FILE:/path:default}") should include a language hint to satisfy markdown lint
(MD040); update the opening fence to use a language token (e.g., add "text"
after the triple backticks so it reads ```text) to explicitly label the block
and prevent the lint warning.

---

Nitpick comments:
In `@explain/report_test.go`:
- Around line 35-38: Remove the stale debugging comment "// This is where the
bug is expected" in the test around the assertion checking entry.Source against
SourceProfile; simply delete that comment so the test only contains the
meaningful assertion (the comparison of entry.Source to SourceProfile and the
t.Errorf call) to avoid confusion when the test passes.

In `@loader.go`:
- Around line 19-56: The guard in collectBaseEntries[T] that returns when path
== "profiles" is dead code and should be removed; update the function
collectBaseEntries (the top-level nil checks remain) by deleting the unreachable
if path == "profiles" { return } branch so the behavior relies solely on the
existing mapping-node check (key == "profiles" && path == "") that skips the
top-level profiles key.

In `@Makefile`:
- Around line 35-37: The Makefile target vulncheck currently uses go run
golang.org/x/vuln/cmd/govulncheck@latest (and the CI uses go install
...@latest), which makes builds non-deterministic; introduce a pinned
GOVULNCHECK_VERSION variable (e.g., GOVULNCHECK_VERSION := vX.Y.Z) and update
the vulncheck target command to use
golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION), then update the CI
workflow to use the same pinned version (replace `@latest` in go install with
@$(GOVULNCHECK_VERSION) or hardcode the same version) so both Makefile target
(vulncheck) and CI consistently reference the exact govulncheck version.

In `@tests/explain_test.go`:
- Around line 104-114: The test name TestExplainDisabled_NoAllocations is
misleading because it only checks that goconfy.Load[ExplainConfig] succeeds
without a reporter; rename the test to a clearer name like
TestExplainDisabled_LoadsSuccessfully (or alternatively add real allocation
checks using testing.AllocsPerRun if you intended to assert allocations). Update
the test function name TestExplainDisabled_NoAllocations and any references to
it, keeping the body that calls goconfy.Load[ExplainConfig] with
goconfy.WithBytes([]byte(yamlData)) and the existing error check, or replace the
body with a testing.AllocsPerRun invocation that measures allocations around the
goconfy.Load call if you choose to assert allocation behavior.
- Around line 37-38: Replace the manual environment setup using
os.Setenv("APP_TOKEN", "super-secret") and defer os.Unsetenv("APP_TOKEN") with
t.Setenv("APP_TOKEN", "super-secret") in the test (tests/explain_test.go) so the
test framework automatically restores the variable and prevents parallel-test
pollution; ensure the test function has a testing.T parameter named t (e.g.,
func TestXxx(t *testing.T)) before using t.Setenv and remove the corresponding
defer os.Unsetenv call.

In `@tests/integration/integration_test.go`:
- Around line 95-99: The variable redacted is already a string returned by
DumpRedactedJSON(cfg), so remove the redundant conversion on redactedStr (i.e.,
delete or replace the line that does redactedStr := string(redacted)); instead
use redacted directly (or assign redactedStr := redacted) where needed; update
any usages expecting redactedStr to reference the existing redacted variable or
the new direct assignment.
- Around line 168-200: The DotenvNoMutation subtest mutates global OS env state
by calling os.Unsetenv without restoring it; capture the original value with
os.LookupEnv at the start of the subtest (inside the t.Run "DotenvNoMutation"),
then restore the original state in a defer or t.Cleanup: if the original was
present call os.Setenv(key, origVal) else call os.Unsetenv(key). Apply the same
preserve-and-restore pattern used in the MissingRequiredEnvMacro test to avoid
leaking environment changes across tests.
- Around line 117-134: The test "MissingRequiredEnvMacro" currently calls
os.Unsetenv("DB_PASSWORD") without restoring the original environment; update
the test to capture the original value via os.LookupEnv("DB_PASSWORD"), then
unset for the test and defer a restore that uses os.Setenv to put the original
value back if present or os.Unsetenv if it was absent, ensuring test isolation
around the goconfy.Load[FullConfig] call so other tests are unaffected.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2dcd9b7f-b421-4971-816e-dd2cda1b6992

📥 Commits

Reviewing files that changed from the base of the PR and between 70f3782 and 4c53ac8.

📒 Files selected for processing (30)

• .github/workflows/ci.yml
• CHANGELOG.md
• CONTRIBUTING.md
• Makefile
• README.md
• RELEASE.md
• docs/CI.md
• docs/CLI.md
• docs/GENERATOR.md
• docs/GETTING_STARTED.md
• docs/INSTALL_TOOLS.md
• docs/SECURITY_MODEL.md
• errors.go
• explain/report.go
• explain/report_test.go
• internal/bench/yaml_bench_test.go
• internal/config/config_test.go
• internal/decode/strict.go
• internal/macros/expand.go
• internal/macros/expand_test.go
• internal/redact/engine.go
• loader.go
• tests/explain_test.go
• tests/hardening_test.go
• tests/integration/integration_test.go
• tests/loader_test.go
• tools/docs/RELEASE_CHECKLIST.md
• tools/examples/tui/README.md
• tools/test_fmt.go
• tools/tests/golden_test.go

✅ Files skipped from review due to trivial changes (1)

• docs/INSTALL_TOOLS.md

🚧 Files skipped from review as they are similar to previous changes (8)

• tests/hardening_test.go
• errors.go
• tools/tests/golden_test.go
• internal/config/config_test.go
• internal/decode/strict.go
• docs/GETTING_STARTED.md
• docs/CLI.md
• tools/examples/tui/README.md