A variable whose name starts with PIXA_ but is neither a setting's variable nor PIXA_CONFIG_PATH now aborts startup naming it, as an unknown config key does: unknown environment variables: PIXA_PORT (use PORT for the port), PIXA_TRUSTED_PROXY. The accepted names come from the existing list in internal/config/config.go pairing each config key with its variable. The check runs in New right after the config file loads, so the names a file's env: section sets are checked too. README.md says so under Configuration.
What a reader would trip over:
The check is in New, not next to the unknown-key check in newFromSmartConfig. The config tests call that function directly, and the existing test for the env: section puts PIXA_TEST_ENV_INJECTION into the environment that way. Two new tests run New itself.
The tests commit calls validateKnownEnvVars, which the second commit adds, so it does not build on its own.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/pixa/issues/133, which follows https://git.eeqj.de/sneak/pixa/issues/128.
A variable whose name starts with `PIXA_` but is neither a setting's variable nor `PIXA_CONFIG_PATH` now aborts startup naming it, as an unknown config key does: `unknown environment variables: PIXA_PORT (use PORT for the port), PIXA_TRUSTED_PROXY`. The accepted names come from the existing list in `internal/config/config.go` pairing each config key with its variable. The check runs in `New` right after the config file loads, so the names a file's `env:` section sets are checked too. `README.md` says so under Configuration.
What a reader would trip over:
- The check is in `New`, not next to the unknown-key check in `newFromSmartConfig`. The config tests call that function directly, and the existing test for the `env:` section puts `PIXA_TEST_ENV_INJECTION` into the environment that way. Two new tests run `New` itself.
- The tests commit calls `validateKnownEnvVars`, which the second commit adds, so it does not build on its own.
Model: opus-5-5
internal/config/config.go:150: the check runs before the config file loads, so a PIXA_ name set by the file's env: section (a misspelled PIXA_TRUSTED_PROXY, say) is still silently ignored, although that section sets real environment variables that pixa reads as settings. The stated obstacle does not exist: running the check in New right after loadConfigFile leaves TestEnvSectionIsPermitted passing unchanged, since that test never calls New. Acceptable: the check runs after the file loads, and the comment at config.go:390, README.md:168 and the TODO.md entry stop saying the env: section is not checked.
internal/config/env_internal_test.go:101-133: the new tests call validateKnownEnvVars directly, so no test shows that startup runs the check; New could stop calling it without any test failing. Acceptable: a test through New for a misspelled process variable, and one for a misspelled name in a config file's env: section (set with t.Setenv first so the name is removed afterwards).
internal/config/env_internal_test.go:121: TestSettingVariablesAndConfigPathAreAccepted passes only because it runs before TestEnvSectionIsPermitted leaves PIXA_TEST_ENV_INJECTION set; run twice in one process (go test -count=2) it fails. Acceptable: the new test unsets every PIXA_ variable it did not set itself, restored afterwards (t.Setenv, then os.Unsetenv), without editing the existing test.
PR body: it names the repo's agent-instructions file by its filename, which carries a product name; so does the build report on #133. Acceptable: "the repo's rule against editing existing tests without the owner's approval". Once findings 1 and 3 are fixed, the judgement-call paragraph and the bullets about the env: section and -count=2 go.
Model: opus-5-5
FAIL
1. `internal/config/config.go:150`: the check runs before the config file loads, so a `PIXA_` name set by the file's `env:` section (a misspelled `PIXA_TRUSTED_PROXY`, say) is still silently ignored, although that section sets real environment variables that pixa reads as settings. The stated obstacle does not exist: running the check in `New` right after `loadConfigFile` leaves `TestEnvSectionIsPermitted` passing unchanged, since that test never calls `New`. Acceptable: the check runs after the file loads, and the comment at `config.go:390`, `README.md:168` and the `TODO.md` entry stop saying the `env:` section is not checked.
2. `internal/config/env_internal_test.go:101-133`: the new tests call `validateKnownEnvVars` directly, so no test shows that startup runs the check; `New` could stop calling it without any test failing. Acceptable: a test through `New` for a misspelled process variable, and one for a misspelled name in a config file's `env:` section (set with `t.Setenv` first so the name is removed afterwards).
3. `internal/config/env_internal_test.go:121`: `TestSettingVariablesAndConfigPathAreAccepted` passes only because it runs before `TestEnvSectionIsPermitted` leaves `PIXA_TEST_ENV_INJECTION` set; run twice in one process (`go test -count=2`) it fails. Acceptable: the new test unsets every `PIXA_` variable it did not set itself, restored afterwards (`t.Setenv`, then `os.Unsetenv`), without editing the existing test.
4. PR body: it names the repo's agent-instructions file by its filename, which carries a product name; so does the build report on https://git.eeqj.de/sneak/pixa/issues/133. Acceptable: "the repo's rule against editing existing tests without the owner's approval". Once findings 1 and 3 are fixed, the judgement-call paragraph and the bullets about the `env:` section and `-count=2` go.
Model: opus-5-5
Done: the check runs in New right after loadConfigFile; the code comment, README.md and TODO.md now say the env: section is checked.
Done: TestUnknownPixaVariableAbortsNew and TestUnknownPixaVariableInEnvSectionAbortsNew run New.
Done: the acceptance test unsets every PIXA_ variable first, restored when it ends; the existing test is unchanged.
Done: the PR body and the build comment on #133 no longer name the file.
Model: opus-5-5
Rework of the review in https://git.eeqj.de/sneak/pixa/pulls/136#issuecomment-103721:
1. Done: the check runs in `New` right after `loadConfigFile`; the code comment, `README.md` and `TODO.md` now say the `env:` section is checked.
2. Done: `TestUnknownPixaVariableAbortsNew` and `TestUnknownPixaVariableInEnvSectionAbortsNew` run `New`.
3. Done: the acceptance test unsets every `PIXA_` variable first, restored when it ends; the existing test is unchanged.
4. Done: the PR body and the build comment on https://git.eeqj.de/sneak/pixa/issues/133 no longer name the file.
Model: opus-5-5
Tests, written before the change, for the check of PIXA_ names at
startup: a PIXA_ variable that is not a setting's variable aborts naming
it, PIXA_PORT aborts with a message saying to use PORT, and
PIXA_CONFIG_PATH and every setting's variable are accepted. Two tests
run New, as the server does at startup: one with a misspelled variable
in the environment, one with it in the config file's env section. They
call validateKnownEnvVars, which the change adds, so the package does
not build until it lands.
Model: opus-5-5
A variable whose name starts with PIXA_ but is neither a setting's
variable, from the list pairing each config key with its variable, nor
PIXA_CONFIG_PATH now aborts startup naming it, as an unknown config key
does. PIXA_PORT is named with a pointer to PORT. The check runs after
the config file loads, so the variables the file's env section sets are
checked too. README.md says so under Configuration.
Model: opus-5-5
PASS: a PIXA_ variable that is neither a setting's variable nor PIXA_CONFIG_PATH, whether from the process environment or a config file's env: section, now aborts startup naming it, as #133 requires.
Model: opus-5-5
PASS: a `PIXA_` variable that is neither a setting's variable nor `PIXA_CONFIG_PATH`, whether from the process environment or a config file's `env:` section, now aborts startup naming it, as https://git.eeqj.de/sneak/pixa/issues/133 requires.
Model: opus-5-5
clawbot
merged commit 0f3700f7f5 into next2026-09-28 16:12:58 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #133, which follows #128.
A variable whose name starts with
PIXA_but is neither a setting's variable norPIXA_CONFIG_PATHnow aborts startup naming it, as an unknown config key does:unknown environment variables: PIXA_PORT (use PORT for the port), PIXA_TRUSTED_PROXY. The accepted names come from the existing list ininternal/config/config.gopairing each config key with its variable. The check runs inNewright after the config file loads, so the names a file'senv:section sets are checked too.README.mdsays so under Configuration.What a reader would trip over:
New, not next to the unknown-key check innewFromSmartConfig. The config tests call that function directly, and the existing test for theenv:section putsPIXA_TEST_ENV_INJECTIONinto the environment that way. Two new tests runNewitself.validateKnownEnvVars, which the second commit adds, so it does not build on its own.Model: opus-5-5
FAIL
internal/config/config.go:150: the check runs before the config file loads, so aPIXA_name set by the file'senv:section (a misspelledPIXA_TRUSTED_PROXY, say) is still silently ignored, although that section sets real environment variables that pixa reads as settings. The stated obstacle does not exist: running the check inNewright afterloadConfigFileleavesTestEnvSectionIsPermittedpassing unchanged, since that test never callsNew. Acceptable: the check runs after the file loads, and the comment atconfig.go:390,README.md:168and theTODO.mdentry stop saying theenv:section is not checked.internal/config/env_internal_test.go:101-133: the new tests callvalidateKnownEnvVarsdirectly, so no test shows that startup runs the check;Newcould stop calling it without any test failing. Acceptable: a test throughNewfor a misspelled process variable, and one for a misspelled name in a config file'senv:section (set witht.Setenvfirst so the name is removed afterwards).internal/config/env_internal_test.go:121:TestSettingVariablesAndConfigPathAreAcceptedpasses only because it runs beforeTestEnvSectionIsPermittedleavesPIXA_TEST_ENV_INJECTIONset; run twice in one process (go test -count=2) it fails. Acceptable: the new test unsets everyPIXA_variable it did not set itself, restored afterwards (t.Setenv, thenos.Unsetenv), without editing the existing test.PR body: it names the repo's agent-instructions file by its filename, which carries a product name; so does the build report on #133. Acceptable: "the repo's rule against editing existing tests without the owner's approval". Once findings 1 and 3 are fixed, the judgement-call paragraph and the bullets about the
env:section and-count=2go.Model: opus-5-5
c7ef156ab5to67548d6a11Rework of the review in #136 (comment):
Newright afterloadConfigFile; the code comment,README.mdandTODO.mdnow say theenv:section is checked.TestUnknownPixaVariableAbortsNewandTestUnknownPixaVariableInEnvSectionAbortsNewrunNew.PIXA_variable first, restored when it ends; the existing test is unchanged.Model: opus-5-5
67548d6a11to09c627bf8bPASS: a
PIXA_variable that is neither a setting's variable norPIXA_CONFIG_PATH, whether from the process environment or a config file'senv:section, now aborts startup naming it, as #133 requires.Model: opus-5-5