refactor(#812): unify mintcore handler construction via getEnv injection - #6255
refactor(#812): unify mintcore handler construction via getEnv injection#6255fullsend-ai-coder[bot] wants to merge 6 commits into
Conversation
Replace the dual-constructor pattern (NewHandler + NewHandlerFromConfig)
with a single NewHandler that accepts a getEnv func(string) string
parameter. Native entrypoints (GCF, standalone) pass os.Getenv; the CF
Worker WASM host passes a callback that looks up Worker bindings by
name from the config JSON.
This eliminates the duplicated role-parsing and validation logic that
existed between NewHandler (env-based) and NewHandlerFromConfig
(explicit-config). Adding a new config variable now requires a single
getEnv("FOO") call in mintcore rather than a new struct field threaded
through every entrypoint path.
Removed types/functions:
- WorkerConfig struct
- ParseWorkerConfig
- NewHandlerFromConfig
The WASM entrypoint (cmd/mint-wasm) now handles version stamping,
custom role permission registration, and required-field validation
directly before calling NewHandler with its getEnv callback. The
TypeScript buildWasmConfig function now emits env-var-named keys
instead of PascalCase struct field names.
Closes #812
|
🤖 Finished Review · ✅ Success · Started 11:53 AM UTC · Completed 12:07 PM UTC Commit: |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
ReviewFindingsHigh
Medium
Next steps:
Previous runReviewFindingsMedium
Previous run (2)ReviewFindingsMedium
Previous run (3)ReviewFindingsMedium
Previous run (4)ReviewFindingsLow
Previous run (5)ReviewFindingsLow
Labels: PR refactors Go code in the mint subsystem (internal/mintcore, cmd/mint, cmd/mint-wasm) |
|
/fs-fix Good start on
Done when: new mint setting = one |
|
🤖 Finished Fix · ❌ Failure · Started 1:19 PM UTC · Completed 1:42 PM UTC Commit: |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/31949476398 Details: Please check the workflow logs for full details and retry with |
|
/fs-fix redo last fix but make sure to pass basic precommit checks this time including go vet |
|
🤖 Finished Fix · ✅ Success · Started 1:53 PM UTC · Completed 2:17 PM UTC Commit: |
…ion, version via ldflags - Replace configJSON/buildWasmConfig with a sync getEnvCallback in the WASM bridge (same pattern as PEM/fetch callbacks). Mintcore decides which keys to read — the JS side no longer serializes a config map. - Move CUSTOM_ROLE_PERMISSIONS registration into NewHandler so all three entrypoints (GCF, standalone, CF Worker) share one validation path. Remove duplicate registerCustomPermissions from cmd/mint and cmd/mint-wasm. - Add nil guard for getEnv parameter in NewHandler. - Stamp version/commit into the WASM binary via -ldflags in BuildWASMFn, matching how writeVersionGoToZip works for GCF. Remove writeVersionTS, version.ts imports, and runtime version injection through JS/config. - Update all BuildWASMFn test stubs (including internal/cli/mint_test.go) to match the new (outPath, version, commit) signature. Addresses review feedback on #6255
🔧 Fix agent — iteration 1 (human-triggered)Addressed 5 of 8 review findings. Implemented getEnv callback pattern (replacing configJSON), unified CUSTOM_ROLE_PERMISSIONS validation in NewHandler, stamped WASM version via -ldflags (replacing writeVersionTS), added nil guard for getEnv, and fixed the go vet failure from the previous attempt. Disagreed with 3 findings: verifier getEnv passthrough (deferred to follow-up), test naming (cosmetic), and WASM-specific test coverage (requires WASM CI). Fixed (5):
Disagreed (3):
Tests: passedNext steps:
|
|
🤖 Finished Review · ✅ Success · Started 2:18 PM UTC · Completed 2:31 PM UTC Commit: |
|
/fs-fix Address remaining review findings and CI gaps from the latest pass ( 1.
|
|
🤖 Finished Fix · ❌ Failure · Started 2:44 PM UTC · Completed 3:07 PM UTC Commit: |
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/31953542446 Details: Please check the workflow logs for full details and retry with |
|
/fs-fix retry previous fix but do not neglect to update embed files |
|
🤖 Finished Fix · ✅ Success · Started 3:10 PM UTC · Completed 3:29 PM UTC Commit: |
… sync embeds - Add GetEnv func(string) string to JWKSVerifierConfig and STSVerifierConfig. Each constructor now reads OIDC_AUDIENCE via getEnv and returns an error if empty — misconfiguration is caught at construction time, not on first Verify(). Remove duplicate per-entrypoint OIDC_AUDIENCE checks from cmd/mint (checkRequired), internal/mint (requiredEnvVars), and cmd/mint-wasm (manual getEnv extraction). - Delete dead internal/dispatch/cf/workersrc/src/version.ts (no longer imported or embedded after writeVersionTS removal). - Sync all embed files: jwks_verifier.go.embed, sts_verifier.go.embed, main.go.embed — fixes the DESYNC pre-commit failure from the previous iteration. Addresses review feedback on #6255
🔧 Fix agent — iteration 2 (human-triggered)Addressed 5 of 6 review findings. Injected getEnv into verifier configs for OIDC_AUDIENCE validation at construction time, deleted dead version.ts, synced all embed files (fixing the previous iteration's DESYNC failure), updated all test stubs, and removed stale per-entrypoint OIDC_AUDIENCE checks. Disagreed with patch coverage finding (requires WASM build env). Fixed (5):
Disagreed (1):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 3:31 PM UTC · Completed 3:45 PM UTC Commit: |
|
/fs-fix Two items:
Pass pre-commit ( |
|
🤖 Finished Fix · ✅ Success · Started 4:36 PM UTC · Completed 4:46 PM UTC Commit: |
Add -s -w ldflags to both the Makefile wasm-build target and the provisioner's buildWASM function to strip debug info from the WASM binary, reducing gzip size. Extract wasmLDFlags() helper for testability and add tests covering the ldflags construction and version/commit forwarding through ensureWASMArtifacts, bringing patch coverage above the 80% threshold. Addresses review feedback on #6255
🔧 Fix agent — iteration 3 (human-triggered)Addressed both items from the human instruction: (1) added -s -w strip flags to Makefile wasm-build and provisioner buildWASM via an extracted wasmLDFlags() helper, (2) added TestWasmLDFlags and TestEnsureWASMArtifacts_ForwardsVersionCommit tests to bring patch coverage above the 80% threshold. All tests pass. Fixed (3):
Tests: passed Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 4:47 PM UTC · Completed 4:59 PM UTC Commit: |
|
/fs-fix Rework verifier initialization to fix the WASM size regression (~12.4 MB raw / ~3.2 MB gzip vs ~7.5 MB / ~2.0 MB on main). Passing Target designCentralize
WASM gateAfter changes, Tests / embed sync
Closes the WASM blocker noted on this PR / #6262. |
|
🤖 Finished Fix · ✅ Success · Started 7:23 PM UTC · Completed 7:39 PM UTC Commit: |
Rework verifier initialization to fix the WASM size regression (~12.4 MB raw / ~3.2 MB gzip vs ~7.5 MB / ~2.0 MB on main). Passing getEnv into NewJWKSVerifier (commit 39c01b8) caused the bloat by pulling closure dependencies into the WASM binary. Changes: - Add VerifierFactory type: func(audience string) (OIDCVerifier, error) - NewHandler now reads OIDC_AUDIENCE from getEnv once, validates it, then calls the factory with the resolved audience string - Replace GetEnv field with Audience string in JWKSVerifierConfig and STSVerifierConfig — verifiers no longer depend on getEnv - Entrypoints (cmd/mint-wasm, cmd/mint, internal/mint, GCF embed) supply verifier-specific factories that capture only platform deps - Use errors.New for constant error strings (no fmt.Errorf bloat) - Sync all .go.embed files - Update all test call sites for new signatures Addresses review feedback on #6255
🔧 Fix agent — iteration 4 (human-triggered)Reworked verifier initialization to fix the WASM size regression. Added VerifierFactory type, centralized OIDC_AUDIENCE resolution in NewHandler, replaced GetEnv with Audience string in verifier configs, updated all entrypoints and tests. All 16 modified files trace to the human instruction. Fixed (8):
Tests: passed
Decision points
Next steps:
|
|
🤖 Finished Review · ✅ Success · Started 7:41 PM UTC · Completed 7:54 PM UTC Commit: |
|
/fs-fix
ScopeOnly cover new/changed production lines in this PR — do not refactor unrelated code. Likely gaps (verify with
Verification
BT/e2e failures on the last run looked like infra flakes (GCP TLS timeout, harness workflow failures, triage |
|
🤖 Finished Fix · ✅ Success · Started 9:27 PM UTC · Completed 9:39 PM UTC Commit: |
… coverage Add tests for three previously uncovered error paths in NewHandler (empty OIDC_AUDIENCE, failing VerifierFactory, RegisterCustomRolePermissions collision) and one in NewSTSVerifier (empty Audience). These raise NewHandler from 93.6% to 100% and NewSTSVerifier from 91.7% to 100%, addressing the codecov/patch threshold failure. Addresses review feedback on #6255
🔧 Fix agent — iteration 5 (human-triggered)Addressed codecov/patch coverage failure by adding 4 tests covering all uncovered new error paths in NewHandler (3 paths) and NewSTSVerifier (1 path). NewHandler coverage rose from 93.6% to 100%, NewSTSVerifier from 91.7% to 100%. Overall mintcore package coverage increased from 84.4% to 84.8%. All tests pass with -race. Fixed (4):
Tests: passedNext steps:
|
|
🤖 Review · ❌ Terminated · Started 9:40 PM UTC · Ended 9:53 PM UTC Commit: |
|
/fs-fix
Root causeUncovered patch lines are in
Required fixAdd provisioner tests that exercise the Patterns already in Verification
|
|
🤖 Finished Review · ✅ Success · Started 9:40 PM UTC · Completed 9:53 PM UTC Commit: |
|
🤖 Fix · Commit: |
Summary
getEnv func(string) stringintoNewHandlerinstead of maintaining dual constructors (NewHandler+NewHandlerFromConfig). Native entrypoints passos.Getenv; the CF Worker WASM host passes a callback that looks up Worker bindings by name.WorkerConfigstruct,ParseWorkerConfig, andNewHandlerFromConfig— all entrypoints now share one constructor and one code path for role parsing, allowlist validation, and handler assembly.buildWasmConfigto emit env-var-named keys (e.g.ROLE_APP_IDS) instead of PascalCase struct field names (e.g.RoleAppIDs), matching the newmap[string]stringcontract on the Go side.Context
Handler construction was split across two nearly parallel paths:
NewHandlerreados.Getenvdirectly, whileNewHandlerFromConfig(used by the Worker WASM entrypoint) accepted explicit string parameters. The role-parsing and validation logic was duplicated between them, creating a maintenance risk as mint deployments diversify (GCF, standalone, CF Worker). See PR #6174 for a live example of the duplication cost.The maintainer's preferred direction (per issue comment) was to inject the env-var lookup rather than introducing a config struct, keeping the coupling minimal — adding a new config var becomes a single
getEnv("FOO")call in mintcore.Test plan
go test ./...passes ininternal/mintcore/,internal/mint/, andcmd/mint/with-racego vet ./...passes in all three modulesTestEmbeddedMintSource_MatchesOriginalpasses (embed sync verified)NewHandlerat 100%,SplitCSVat 100%TestNewHandler_CustomGetEnvtests verify the getEnv injection pattern (map-backed callback simulating Worker bindings)cmd/mint-wasmcannot be compiled in this sandbox (GOOS=js GOARCH=wasmrequired) — manual verification or CI build neededCloses #812
Post-script verification
agent/812-unify-mintcore-handler)53456d472dcba81bcea79f8d715e9d6159cefa5c..HEAD)