✨ feat(examples): EX-S05 HCL honesty — .tf opaque path - #63
Merged
Conversation
Pre-existing drift from already-merged docs/ADR commits (df77035 and predecessors); unrelated to this lane's EX-S05 work but required for `task check`'s changelog-verify to pass before committing.
…paque decision infra-vars' class now matches envs/**/*.tf as well as *.tfvars, and a new vars/tf-opaque case (a generic resource/module block, invented names only) proves what happens: REVIEW with zero findings, matching adoptertest's undecidable-changeset guard. Measured, not assumed (REQ-EX-S05-02): the mechanism is not "parseHCL rejects the block" as the spec anticipated. internal/change/diff.go's baseProducerFor only routes .tfvars to the HCL parser; .tf falls through to the YAML producer default, which cannot parse HCL syntax at all — block or bare literal alike. A scratch-copy discriminating test (tfvars-shaped literal content under a .tf extension) confirms this: still opaque. So every .tf file is opaque -> REVIEW today regardless of content; only .tfvars gets structured diffing. Routing .tf to the HCL producer is unbuilt engine work, out of this story's internal/change scope — documented as a known limitation in config.yaml, the fixture files, expect.yaml, and examples/README.md, not glossed over. The existing S04 tfvars cases (nested-map-change et al.) still PASS with APPROVE, which is unreachable through the opaque path — positive evidence the adapter still diffs tfvars literals structurally (REQ-EX-S05-03). examples/README.md's Input formats line now claims tf; hack/docs/ example_format_inventory_test.sh's hardcoded real-tree assertion and its GOOD_README-derived polarity fixtures (cue/toml/hcl mutations, which anchored on the literal "tfvars." substring) are updated to match — REQ-EX-S05-04. Non-vacuity (REQ-EX-S05-05), verified in scratch copies: - deleting the tf-opaque case's .tf fixture content (base/head files) makes `assent test` hard-error (exit 2, "found 0" files under base/) - deleting the class's *.tf glob (leaving the explanatory comment, reworded to avoid a literal *.tf token that would have vacuously kept the comment-grep inventory check green) reddens hack/docs/example_format_inventory_test.sh
….tf governance Independent review (F1, P1) found that `assent test`/`--coverage` cannot distinguish "the vars/tf-opaque .tf fixture is governed by the infra-vars class" from "it isn't governed at all": deleting envs/**/*.tf from config.yaml's match.paths, while leaving the fixture in place, produces byte-identical PASS vars/tf-opaque (REVIEW), exit 0. Root cause traced past adoptertest's undecidable-guard collapse (opaque and empty ChangeSets both fall through to a bare REVIEW/no-findings result) to a deeper, deliberate architectural fact: internal/catalogue's Input type omits Config entirely (D-017 B10 — "a rule's classes come from the binding graph ... so Config is deliberately absent"), and no other engine code path (selectBindingForTest, selectBinding, classify.go) reads Config.Classes either. `assent test` was never wired to consult class match.paths at all, for any pack — this is not specific to infra-vars or to .tf. Given that, making `assent test` itself discriminate would mean adding class-routing to the test/catalogue dispatch, a cross-pack engine change well outside this story's examples-only scope. Instead: a companion test (TestInfraVarsTFFixtureIsGovernedByItsClass) asserts, against the real config.yaml and the SAME internal/glob matcher the routing classifier and coverage loop use, that the tf-opaque fixture's path is actually covered by the infra-vars class's match.paths — and a mutation-control test proves that assertion is capable of failing. Verified empirically: deleting the .tf glob from config.yaml reddens this new test (and the existing inventory script); restoring it goes green again; `assent test`/`--coverage` remain unchanged either way, exactly as diagnosed.
Picks up the feat(examples) and test(examples) subjects from this lane's two prior commits.
…ion)
Round-2 independent review (F1, P1): the round-1 fix closed only the
class-path-extension-deleted mutation. The OTHER disjunct REQ-EX-S05-05 names
— deleting the vars/tf-opaque case's base/+head/ .tf fixture files outright,
leaving config.yaml's *.tf glob intact — stayed invisible: `assent test
--coverage` doesn't notice a case that silently vanished from discovery
(--coverage is rule-polarity-level, not case-count-level), the inventory
script derives FORMATS from config.yaml's declared glob extensions rather
than checking any file exists, and the round-1 glob-match test only checked a
hardcoded path STRING against the class pattern — it never touched the
filesystem, so it can't tell the fixture is gone.
TestInfraVarsTFFixtureFilesExist os.Stats the real base/ and head/ .tf files
directly (plus a not-a-directory and not-empty check, the latter guarding
against F2's "emptied to zero bytes" variant from round 1), with a mutation
control proving the assertion can fail. Verified empirically, both disjuncts:
- fixture directory deleted (reproducing the reviewer's exact repro) ->
`assent test --coverage` on a scratch copy: exit 0, fully green, case
silently absent (confirms the finding); the same mutation on the real
tree -> TestInfraVarsTFFixtureFilesExist FAILs; restored -> green again.
- class-path *.tf glob deleted (round-1's mutation, re-checked for
regression) -> TestInfraVarsTFFixtureIsGovernedByItsClass and the
inventory script both still FAIL as before; restored -> both green.
F2 (P2): amends REQ-EX-S05-05's Test:/Verify: fields in
openspec/specs/p5-ex-complex-examples/spec.md to name the actual three-part
non-vacuity proof (inventory + this Go test, not --coverage alone) and record
why --coverage can't do it (D-017 B10 — catalogue.Input omits Config, so no
assent test/--coverage code path ever consults Config.Classes/match.paths).
…ment The previous amendment attributed BOTH the glob.Match class-path check and the os.Stat fixture-existence check to TestInfraVarsTFFixtureIsGovernedByItsClass (the round-1 test name). That test only does the glob.Match half; the os.Stat half is TestInfraVarsTFFixtureFilesExist (round 2). Names each test against its actual disjunct instead of collapsing both into one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.tffixture (envs/prod/backend.tf, a genericresource/moduleblock, D-002-clean invented names) to theinfra-varsexample pack alongside EX-S04's existing structured.tfvarscases, and extends the class'smatch.pathstoenvs/**/*.tf..tffiles: they never reach the HCL parser (only.tfvarsdoes, verified directly againstinternal/change/diff.go's producer routing), so they fall through to a YAML-producer default that fails on any HCL syntax, making the decision always opaque regardless of content. Pins the measured decision (REVIEW,findings: []) rather than an assumed one.cmd/assent/examples_infravars_tf_governance_test.gowith Go tests proving non-vacuity at both disjuncts of REQ-EX-S05-05 (class-path-glob deleted, and fixture files deleted/emptied) — added becauseassent test/--coverage/the format-inventory script are each architecturally blind to at least one of these mutations. Root cause:internal/catalogue.Inputdeliberately omitsConfig, so no runtime routing path ever readsConfig.Classes— documented as D-017 B10.examples/README.mdand the spec's ownTest:/Verify:fields to honestly reflect what's proven and how.Passed independent review across three rounds (each round closed a real, mutation-verified non-vacuity gap) — cleared to land as APPROVE.
Test plan
task checkgreen (includingchangelog-verify)task coveragegreen (91.1%, required 91%)./bin/assent test --coverage examples/packs/infra-vars— 11 cases includingvars/tf-opaque (REVIEW), 5 rules both polarities coveredgo test ./cmd/assent/... -run TestInfraVarsTF -v— all 4 governance tests (2 assertions + 2 mutation-control proofs) pass