feat: add development-only local mint server by ggallen · Pull Request #1784 · fullsend-ai/fullsend

ggallen · 2026-06-02T14:21:07Z

Summary

Add a standalone token mint for local development and evaluation. This eliminates the need for GCP infrastructure (Secret Manager, WIF, Cloud Functions) to get started with fullsend.

Dev mint server (`internal/devmint/`)

Local HTTP server that mints real GitHub App installation tokens
Disk-based PEM storage — write PEM files and the mint picks them up on startup
OIDC verification via JWKS (direct signature validation, no STS exchange)
Optional --insecure-no-auth flag for local dev
cloudflared tunnel support for exposing the local mint to GitHub Actions runners
/v1/status endpoint gated behind OIDC auth (matching production mint behavior)

CLI changes

fullsend mint run command with --data-dir, --port, --tunnel, --insecure-no-auth, --oidc-audience flags
--mint-data-dir flag on fullsend admin install — writes PEMs directly to disk instead of Secret Manager
Enhanced --mint-url / --skip-mint-check validation (allows http://localhost, http://127.0.0.1, http://[::1] for dev mint)
storePEMToDisk() for disk-based PEM persistence during install

Documentation

New guide: docs/guides/infrastructure/dev-mint.md
Updated installation guide and docs README
Added mint run to CLI command tree in cli-internals.md

Review focus

Is storePEMToDisk safe against path traversal via role names?
Is the cloudflared tunnel lifecycle robust (startup wait, shutdown, URL extraction)?
Do --insecure-no-auth and --mint-data-dir compose correctly with existing flags?

Test plan

go test ./internal/devmint/... — tests pass
go test ./internal/cli/... — CLI tests pass (including IPv6 localhost)
go vet ./... — clean
go test ./... — full test suite passes
Manual: fullsend mint run → tunnel → fullsend admin install → agent runs

🤖 Generated with Claude Code

Closes #1963

github-actions · 2026-06-02T14:22:48Z

Site preview

Preview: https://31601812-site.fullsend-ai.workers.dev

Commit: 4e4df8937645478727811bfe02d0855ea2a786ed

fullsend-ai-review · 2026-06-02T14:37:44Z

Review

Findings

Medium

[protected-path] AGENTS.md, skills/mint-enroll/SKILL.md — These files are under protected paths (AGENTS.md is a governance file; skills/ is infrastructure). The AGENTS.md change expands ADR immutability rules with Note-paragraph guidance (7 lines). The skills/mint-enroll/SKILL.md change adds a 3-line cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go, internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind). See also: [edge-case] finding below.
[edge-case] internal/cli/admin.go — skipMintDispatcher.StoreAgentPEM writes PEMs to disk when dataDir is set but does not update config.json. The comment explains config.json is managed by the WithStoreSecret callback during app creation, which always runs before this dispatch-layer method in the install flow. StoreAgentPEM is not called during normal install — the dispatch layer only calls Provision(). A future caller invoking it directly would produce orphaned PEM files invisible to the dev mint.
[edge-case] internal/cli/admin.go — storePEMToDisk performs a read-modify-write on config.json without file locking or mutex protection. The comment documents this as not concurrency-safe and callers must invoke sequentially. Current callers (the role loop in runAppSetup) are sequential. A future change parallelizing role setup would silently corrupt config.json.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[stale-doc] docs/ADRs/0033-per-repo-installation-mode.md — The frozen Decision section describes --skip-mint-check as skipping "app setup," which ADR 0043 supersedes (Apps are still created). The Status section already correctly cross-references ADR 0043, so no edit is needed — this is the intended ADR immutability pattern.

Previous run

Review

Findings

Medium

[protected-path] AGENTS.md, skills/mint-enroll/SKILL.md — These files are under protected paths (AGENTS.md is a governance file; skills/ is infrastructure). The AGENTS.md change expands ADR immutability rules with Note-paragraph guidance (7 lines). The skills/mint-enroll/SKILL.md change adds a 3-line cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go, internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind). See also: [edge-case] finding below.
[edge-case] internal/cli/admin.go — skipMintDispatcher.StoreAgentPEM writes PEMs to disk when dataDir is set but does not update config.json. The comment explains config.json is managed by the WithStoreSecret callback during app creation. In the current install flow, no caller invokes StoreAgentPEM on skipMintDispatcher (it only satisfies the dispatch.Dispatcher interface). A future caller invoking it directly would produce orphaned PEM files invisible to the dev mint.
[edge-case] internal/cli/admin.go — storePEMToDisk performs a read-modify-write on config.json without file locking or mutex protection. The comment documents this as not concurrency-safe and callers must invoke sequentially. Current callers (the role loop in runAppSetup) are sequential. A future change parallelizing role setup would silently corrupt config.json.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[stale-doc] docs/ADRs/0033-per-repo-installation-mode.md — The frozen Decision section describes --skip-mint-check as skipping "app setup," which ADR 0043 supersedes (Apps are still created). The Status section already correctly cross-references ADR 0043, so no edit is needed — this is the intended ADR immutability pattern.

Previous run (2)

Review

Findings

Medium

[protected-path] AGENTS.md, skills/mint-enroll/SKILL.md — These files are under protected paths (AGENTS.md is a governance file; skills/ is infrastructure). The AGENTS.md change expands ADR immutability rules with Note-paragraph guidance (7 lines). The skills/mint-enroll/SKILL.md change adds a 3-line cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go, internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind). See also: [edge-case] finding below.
[edge-case] internal/cli/admin.go — skipMintDispatcher.StoreAgentPEM writes PEMs to disk when dataDir is set but does not update config.json. The comment explains config.json is managed by the WithStoreSecret callback. In the current install flow, no caller invokes StoreAgentPEM on skipMintDispatcher (it only satisfies the dispatch.Dispatcher interface). A future caller invoking it directly would produce orphaned PEM files invisible to the dev mint.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently. ADR 0043 uses "dev mint" consistently, suggesting that as the canonical term.
[error-handling-idiom] internal/cli/admin.go — The error message "existing config.json is corrupted: %w" uses "corrupted" language, while other parsing errors in the file use neutral terms like "parsing config: %w". Consider aligning with the codebase's neutral error message pattern.

Previous run (3)

Review

Findings

Low

[permissive-defaults] internal/devmint/devmint.go — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. See also: [edge-case] finding below.
[edge-case] internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind). See also: [permissive-defaults] finding above.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently. ADR 0043 uses "dev mint" consistently, suggesting that as the canonical term.

Previous run (4)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. A user running cloudflared externally (bypassing the --tunnel guard) would inherit wildcard access; the server logs a warning in this case.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently.
[test-adequacy] internal/cli/admin_test.go — No test validates that --mint-data-dir without --skip-mint-check is rejected. Both the per-org and per-repo install paths include this validation (--mint-data-dir requires --skip-mint-check), but no test exercises either path. Adding a test would prevent accidental removal of this guard.
[edge-case] internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind).

Previous run

Review

Findings

Medium

[protected-path] AGENTS.md, skills/mint-enroll/SKILL.md — These files are under protected paths (AGENTS.md is a governance file; skills/ is infrastructure). The AGENTS.md change expands ADR immutability rules with Note-paragraph guidance (7 lines). The skills/mint-enroll/SKILL.md change adds a 3-line cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go, internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind). See also: [edge-case] finding below.
[edge-case] internal/cli/admin.go — skipMintDispatcher.StoreAgentPEM writes PEMs to disk when dataDir is set but does not update config.json. The comment explains config.json is managed by the WithStoreSecret callback during app creation. In the current install flow, no caller invokes StoreAgentPEM on skipMintDispatcher (it only satisfies the dispatch.Dispatcher interface). A future caller invoking it directly would produce orphaned PEM files invisible to the dev mint.
[edge-case] internal/cli/admin.go — storePEMToDisk performs a read-modify-write on config.json without file locking or mutex protection. The comment documents this as not concurrency-safe and callers must invoke sequentially. Current callers (the role loop in runAppSetup) are sequential. A future change parallelizing role setup would silently corrupt config.json.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[stale-doc] docs/ADRs/0033-per-repo-installation-mode.md — The frozen Decision section describes --skip-mint-check as skipping "app setup," which ADR 0043 supersedes (Apps are still created). The Status section already correctly cross-references ADR 0043, so no edit is needed — this is the intended ADR immutability pattern.

Previous run (2)

Review

Findings

Medium

[protected-path] AGENTS.md, skills/mint-enroll/SKILL.md — These files are under protected paths (AGENTS.md is a governance file; skills/ is infrastructure). The AGENTS.md change expands ADR immutability rules with Note-paragraph guidance (7 lines). The skills/mint-enroll/SKILL.md change adds a 3-line cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go, internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind). See also: [edge-case] finding below.
[edge-case] internal/cli/admin.go — skipMintDispatcher.StoreAgentPEM writes PEMs to disk when dataDir is set but does not update config.json. The comment explains config.json is managed by the WithStoreSecret callback. In the current install flow, no caller invokes StoreAgentPEM on skipMintDispatcher (it only satisfies the dispatch.Dispatcher interface). A future caller invoking it directly would produce orphaned PEM files invisible to the dev mint.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently. ADR 0043 uses "dev mint" consistently, suggesting that as the canonical term.
[error-handling-idiom] internal/cli/admin.go — The error message "existing config.json is corrupted: %w" uses "corrupted" language, while other parsing errors in the file use neutral terms like "parsing config: %w". Consider aligning with the codebase's neutral error message pattern.

Previous run (3)

Review

Findings

Low

[permissive-defaults] internal/devmint/devmint.go — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. See also: [edge-case] finding below.
[edge-case] internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind). See also: [permissive-defaults] finding above.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently. ADR 0043 uses "dev mint" consistently, suggesting that as the canonical term.

Previous run (4)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. A user running cloudflared externally (bypassing the --tunnel guard) would inherit wildcard access; the server logs a warning in this case.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently.
[test-adequacy] internal/cli/admin_test.go — No test validates that --mint-data-dir without --skip-mint-check is rejected. Both the per-org and per-repo install paths include this validation (--mint-data-dir requires --skip-mint-check), but no test exercises either path. Adding a test would prevent accidental removal of this guard.
[edge-case] internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind).

Previous run (5)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. A user running cloudflared externally (bypassing the --tunnel guard) would inherit wildcard access; the server logs a warning in this case.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently.
[test-adequacy] internal/devmint/devmint_test.go — The OIDC verification tests use a mock verifier that always returns RepositoryOwner: "myorg", matching the stored org. There is no test that verifies the org mismatch path (claims.RepositoryOwner != org returns 403) or the nil-claims fail-closed path (Verify returns nil, nil returns 403). Adding these tests would exercise the defense-in-depth checks added since the prior review.
[edge-case] internal/cli/mint_run.go — Without --tunnel, --allowed-workflows defaults to nil, resolved to ["*"] (wildcard) at verifier construction time. A user running on a non-loopback bind (e.g., --bind 0.0.0.0) gets wildcard workflow access by default. Mitigated by OIDC verification still being enabled (only --insecure-no-auth disables it, which is blocked for non-loopback bind).

Previous run (6)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. A user running cloudflared externally (bypassing the --tunnel guard) would inherit wildcard access; the server logs a warning in this case.
[edge-case] internal/devmint/devmint.go — In handleToken and handleStatus, the OIDC org ownership check is guarded by if claims != nil. If a future OIDCVerifier implementation returns (nil, nil), the org check is silently skipped and token minting proceeds without org validation (fail-open). No current implementation returns (nil, nil) — the JWKSVerifier always returns non-nil claims on success — but the production handler in mintcore/handler.go does not use this nil-guard pattern, preferring fail-closed (nil dereference crash). Consider treating nil claims with nil error as an error to match the production handler's pattern.
[documentation-accuracy] docs/ADRs/0043-skip-mint-check-updated-semantics.md — The ADR describes PEM storage priority as "Secret Manager (if --mint-project is set), disk (if --mint-data-dir is set), or repo secrets (fallback)." The code in runAppSetup requires both mintProject != "" AND mintDataDir == "" for Secret Manager, meaning --mint-data-dir overrides --mint-project when both are set. The stated priority suggests --mint-project always wins, which does not match the actual behavior.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently.
[edge-case] internal/devmint/devmint.go — loadFromDisk validates role names via mintcore.ValidateRoleName but does not validate the org field from config.json. A malformed org value (e.g., from a manually created config) would cause the OIDC verifier to reject all legitimate tokens without a clear error message pointing to the invalid org.

Previous run (7)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. A user running cloudflared externally (bypassing the --tunnel guard) would inherit wildcard access; the server logs a warning in this case.
[design-direction] AGENTS.md — The expanded ADR immutability rules now include guidance for when to use Note paragraphs vs full supersession. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) have Note paragraphs cross-referencing ADR 0043. The Notes follow the stated rule (extensions, not contradictions to the frozen Decision sections). Consider whether a single ADR consolidating the three-tier PEM storage model would provide a cleaner reference point than six scattered Notes.
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently.
[edge-case] internal/devmint/devmint.go — loadFromDisk validates role names via mintcore.ValidateRoleName but does not validate the org field from config.json. A malformed org value (e.g., from a manually created config) would cause the OIDC verifier to reject all legitimate tokens without a clear error message pointing to the invalid org.
[defense-in-depth] internal/devmint/devmint.go — handleStatus discards OIDC claims after verification (_, err := s.oidcVerifier.Verify(...)) and does not perform the explicit claims.RepositoryOwner == s.org check that handleToken does. The JWKSVerifier already enforces org restriction via AllowedOrgs, so this is defense-in-depth only. Adding the explicit check would match handleToken's pattern.

Info

[prior-finding-resolved] internal/devmint/devmint.go — The prior defense-in-depth finding is resolved: handleToken now explicitly verifies claims.RepositoryOwner == s.org after OIDC verification, matching the production handler's pattern.
[prior-finding-resolved] internal/devmint/devmint.go:1 — The prior missing-package-doc finding is resolved: the devmint package now has a package-level godoc comment.
[prior-finding-resolved] docs/ADRs/0033-per-repo-installation-mode.md — ADR 0033's Status field now includes a supersession link to ADR 0043, resolving the prior finding.
[prior-finding-resolved] docs/architecture.md — The three-tier PEM storage description now cites ADR 0043, resolving the prior finding about a missing citation.

Previous run (8)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.
[design-direction] AGENTS.md — The expanded ADR immutability rules permit Note: paragraphs in Status sections for discoverability cross-references. Six Accepted ADRs (0007, 0008, 0014, 0017, 0026, 0029) now have Note paragraphs describing changes to their frozen Decision/Consequences sections (PEM storage location). While technically placed in Status sections, these Notes collectively amend normative content. Consider whether a single ADR (e.g., "ADR 0044: Three-tier PEM storage model") would provide a cleaner supersession path than six scattered Notes.
[design-direction] docs/ADRs/0033-per-repo-installation-mode.md — ADR 0043 states it supersedes the --skip-mint-check description in ADR 0033's Decision section, but ADR 0033's Status field still shows only Accepted without a supersession link. Add: Accepted (--skip-mint-check semantics superseded by [ADR 0043](...)) to match the pattern in ADR 0026.

Low

[permissive-defaults] internal/devmint/devmint.go:340 — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. A user running cloudflared externally (bypassing the --tunnel guard) would inherit wildcard access; the server logs a warning in this case.
[defense-in-depth] internal/devmint/devmint.go:252 — handleToken logs OIDC claims but does not explicitly verify claims.RepositoryOwner == s.org after OIDC verification. The JWKSVerifier already enforces this via AllowedOrgs: []string{s.org}, so this is defense-in-depth rather than a gap. Adding the explicit check would match the production handler's pattern (mintcore/handler.go:205).
[naming-consistency] docs/guides/infrastructure/dev-mint.md — The guide title is "Standalone token mint", the file is dev-mint.md, the package is devmint, and prose alternates between "dev mint" and "standalone mint". Pick one canonical term and use it consistently.
[missing-adr-citation] docs/architecture.md:203 — Describes the three-tier PEM storage model but does not cite ADR 0043 as the authoritative decision, unlike surrounding lines which cite their respective ADRs.
[missing-package-doc] internal/devmint/devmint.go:1 — The devmint package lacks a package-level godoc comment. Other packages in internal/ have package doc comments explaining their purpose.

Info

[prior-finding-resolved] internal/cli/admin_test.go — TestValidateSkipMintCheck now covers the credential-embedding rejection case (http://user:pass@localhost:8321). The prior low finding about missing test coverage is resolved.

Previous run (9)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go:340 — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows. A user running cloudflared externally (bypassing the --tunnel guard) would inherit wildcard access; the server logs a warning in this case.
[stale-doc] docs/guides/dev/cli-internals.md:167 — Phase 6 diagram still states + PEM keys as repo secrets for per-org mode. With --mint-data-dir (dev mint), PEMs are written to disk; with a GCP project, they go to Secret Manager. The same diagram was partially updated at lines 109 and 120 but this line was missed.
[stale-doc] docs/ADRs/0014-admin-install-github-apps-secrets-v1.md:28 — Decision section describes PEMs stored as repository secrets FULLSEND_<ROLE>_APP_PRIVATE_KEY on .fullsend. This was already partially stale (ADR 0029 moved to Secret Manager) and is further extended by ADR 0043 (disk storage for dev mint). Unlike ADRs 0007, 0008, 0026, 0029, and 0033 which received Status-section Notes in this PR, ADR 0014 was not annotated.
[stale-doc] docs/ADRs/0017-credential-isolation-for-sandboxed-agents.md:98 — Consequences section states "The admin CLI's secrets layer stores agent private keys in the .fullsend config repo (as repo secrets)." With ADR 0029 and ADR 0043, PEMs can now be stored in Secret Manager, on disk, or as repo secrets (fallback).

Info

[prior-finding-resolved] internal/cli/admin_test.go — TestValidateSkipMintCheck now covers the credential-embedding rejection case (http://user:pass@localhost:8321). The prior low finding about missing test coverage is resolved.

Previous run (10)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go:340 — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows.
[scope-creep] AGENTS.md — The ADR immutability rules were expanded to allow Note: paragraphs in Status sections for cross-reference discoverability. This meta-policy change is not mentioned in issue Standalone dev mint server (no GCP required) #1963. The change is sensible and directly supports the dev mint work (annotating ADRs 0007, 0029, 0033), but goes beyond the stated scope.
[test-coverage] internal/cli/admin_test.go — TestValidateSkipMintCheck does not cover the credential-embedding rejection case (http://user:pass@localhost:8321). The parsed.User != nil code path is tested transitively through validateMintURLHTTPS tests but has no direct unit test.
[stale-doc] docs/ADRs/0008-workflow-dispatch-for-cross-repo-dispatch.md:31 — States "the App PEMs (repo secrets on .fullsend)" as the only PEM storage location. With the dev mint, PEMs can also be stored on disk (--mint-data-dir). The ADR is frozen (Accepted), but a Status-section Note referencing ADR 0043 (matching the pattern applied to ADRs 0007, 0029, 0033 in this PR) would improve discoverability.
[stale-doc] docs/ADRs/0026-stage-based-dispatch-for-agent-workflow-decoupling.md:137 — Consequences section states "App PEMs stay in the config repo" which is no longer universally true — the dev mint stores PEMs on disk outside any Git repository. A Status-section Note would clarify.

Info

[prior-finding-resolved] internal/devmint/devmint.go:255 — OIDC claims are now logged (claims.Repository, claims.JobWorkflowRef) rather than discarded. The prior finding about missing debuggability is resolved.
[prior-finding-resolved] internal/cli/mint_run.go:56 — The --insecure-no-auth guard now correctly handles --bind localhost by checking bind == "localhost" before net.ParseIP(bind). The prior finding about localhost rejection is resolved.
[prior-finding-resolved] docs/guides/infrastructure/dev-mint.md:73 — Step 3 now includes --allowed-workflows '.github/workflows/dispatch.yml' in the example command. The prior high finding about missing --allowed-workflows is resolved.

Previous run (11)

Review

Findings

High

[documentation/code mismatch] docs/guides/infrastructure/dev-mint.md:73 — Step 3 of the dev-mint guide shows fullsend mint run --data-dir ~/.fullsend-mint --port 8321 --tunnel without --allowed-workflows. The code in internal/cli/mint_run.go enforces that --tunnel requires --allowed-workflows and will fail at startup with: --tunnel requires --allowed-workflows: a public tunnel with wildcard workflow access lets any GitHub Actions workflow in the org mint tokens. Users following this guide will hit an immediate error.
Remediation: Add --allowed-workflows '.github/workflows/dispatch.yml' (or an appropriate workflow path) to every fullsend mint run --tunnel example in the guide.

Low

[scope-creep] AGENTS.md — The ADR immutability rules were expanded to allow Note: paragraphs in Status sections for cross-reference discoverability. This meta-policy change is not mentioned in issue Standalone dev mint server (no GCP required) #1963. The change is sensible but goes beyond the stated scope of adding a dev mint. Consider filing a separate issue for this policy change.
[edge-case] internal/cli/mint_run.go:56 — The --insecure-no-auth guard rejects --bind localhost because net.ParseIP("localhost") returns nil, triggering the loopback check. While the default --bind 127.0.0.1 works and the error message provides correct guidance, a user passing --bind localhost gets a confusing rejection even though localhost resolves to a loopback address.
[debuggability] internal/devmint/devmint.go:255 — In handleToken, when OIDC verification is enabled, the verified claims are discarded (if _, err := ...). The dev mint cannot log which workflow/repo requested the token, reducing debuggability.
[permissive-defaults] internal/devmint/devmint.go:340 — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows.

Info

[test-coverage] internal/cli/admin_test.go — TestValidateSkipMintCheck does not cover the credential-embedding rejection case (http://user:pass@localhost:8321). The parsed.User != nil code path is untested.

Previous run (12)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.
[insecure-default] internal/devmint/devmint.go:157 — The dev mint defaults allowedWorkflows to ["*"] (wildcard) when no --allowed-workflows flag is provided. Since the prior review, the CLI now requires --allowed-workflows when --tunnel is set (mint_run.go line 54), which mitigates the most dangerous scenario (public tunnel + wildcard). However, the server-level default in devmint.go remains ["*"], so any caller constructing a Server directly (tests, future integrations) still inherits the permissive default. The wildcard default for the loopback-only case is an accepted trade-off documented in the security considerations section.

Low

[dead-code] internal/cli/admin.go:740 — skipMintDispatcher.StoreAgentPEM writes PEMs to disk when dataDir is set, but the install flow never calls this method. PEM storage is handled by the WithStoreSecret callback during app creation (which calls storePEMToDisk). The dispatch layer calls Provision(), not StoreAgentPEM(). The code comment acknowledges this. Harmless — satisfies the dispatch.Dispatcher interface contract.
[error-handling] internal/cli/admin.go:776 — In storePEMToDisk, the PEM file is written to disk before config.json is updated. A crash between writes leaves an orphaned PEM. The code comment documents this trade-off and notes self-healing on retry. Config.json write uses atomic rename.
[missing-test] internal/cli/admin_test.go — No tests verify the interaction between --mint-data-dir and the per-org or per-repo install flow end-to-end. Unit tests cover storePEMToDisk, skipMintDispatcher, resolveAppSetupMintProject, and the SecretExists/StoreSecret callbacks individually, but the integration through runAppSetup and needConfigRepo logic is not exercised. New tests for AppID propagation and individual callbacks are good progress.
[edge-case] internal/devmint/devmint.go:258 — The OIDC verification in handleToken discards the claims returned by s.oidcVerifier.Verify(). However, the verifier already enforces org restrictions via AllowedOrgs: []string{s.org} (the JWKS verifier calls ValidateOrgAllowed on claims.RepositoryOwner). The handler trusts the request body for role/repo selection after authentication, which is consistent with the production handler (mintcore.Handler). This is a deliberate trust-boundary decision for a single-org dev server, not a bypass.
[missing-test] internal/devmint/tunnel.go — StartTunnel has no test coverage in CI. Tests are skipped via CI env var check. The function has multiple failure modes (cloudflared not in PATH, process exits before emitting URL, timeout, URL regex mismatch). The "not in PATH" and "early exit" cases are tested locally but skipped in CI; consider testing at minimum the URL extraction via a mock or stub that can run in CI.
[architectural-drift] docs/ADRs/0007-per-role-github-apps.md, docs/ADRs/0029-central-token-mint-secretless-fullsend.md, docs/ADRs/0033-per-repo-installation-mode.md — These accepted ADRs have Note: paragraphs added in the Status section referencing ADR 0043. The AGENTS.md update in this PR codifies the Note pattern as an acceptable edit to Accepted ADRs when the new ADR extends (rather than contradicts) the old decision. This resolves the prior concern about setting an undocumented precedent.

Info

[prior-finding-resolved] internal/devmint/devmint.go:300 — The /v1/status endpoint now sorts roles alphabetically (sort.Slice added since prior review). The prior finding about nondeterministic order has been resolved.

Previous run (13)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update (3 lines) adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.

Low

[permissive-defaults] internal/devmint/devmint.go:340 — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows.
[scope-creep] AGENTS.md — The ADR immutability rules were expanded to allow Note: paragraphs in Status sections for cross-reference discoverability. This meta-policy change is not mentioned in issue Standalone dev mint server (no GCP required) #1963. The change is sensible and directly supports the dev mint work (annotating ADRs 0007, 0029, 0033), but goes beyond the stated scope.
[test-coverage] internal/cli/admin_test.go — TestValidateSkipMintCheck does not cover the credential-embedding rejection case (http://user:pass@localhost:8321). The parsed.User != nil code path is tested transitively through validateMintURLHTTPS tests but has no direct unit test.
[stale-doc] docs/ADRs/0008-workflow-dispatch-for-cross-repo-dispatch.md:31 — States "the App PEMs (repo secrets on .fullsend)" as the only PEM storage location. With the dev mint, PEMs can also be stored on disk (--mint-data-dir). The ADR is frozen (Accepted), but a Status-section Note referencing ADR 0043 (matching the pattern applied to ADRs 0007, 0029, 0033 in this PR) would improve discoverability.
[stale-doc] docs/ADRs/0026-stage-based-dispatch-for-agent-workflow-decoupling.md:137 — Consequences section states "App PEMs stay in the config repo" which is no longer universally true — the dev mint stores PEMs on disk outside any Git repository. A Status-section Note would clarify.

Info

[prior-finding-resolved] internal/devmint/devmint.go:255 — OIDC claims are now logged (claims.Repository, claims.JobWorkflowRef) rather than discarded. The prior finding about missing debuggability is resolved.
[prior-finding-resolved] internal/cli/mint_run.go:56 — The --insecure-no-auth guard now correctly handles --bind localhost by checking bind == "localhost" before net.ParseIP(bind). The prior finding about localhost rejection is resolved.
[prior-finding-resolved] docs/guides/infrastructure/dev-mint.md:73 — Step 3 now includes --allowed-workflows '.github/workflows/dispatch.yml' in the example command. The prior high finding about missing --allowed-workflows is resolved.

Previous run (14)

Review

Findings

High

[documentation/code mismatch] docs/guides/infrastructure/dev-mint.md:73 — Step 3 of the dev-mint guide shows fullsend mint run --data-dir ~/.fullsend-mint --port 8321 --tunnel without --allowed-workflows. The code in internal/cli/mint_run.go enforces that --tunnel requires --allowed-workflows and will fail at startup with: --tunnel requires --allowed-workflows: a public tunnel with wildcard workflow access lets any GitHub Actions workflow in the org mint tokens. Users following this guide will hit an immediate error.
Remediation: Add --allowed-workflows '.github/workflows/dispatch.yml' (or an appropriate workflow path) to every fullsend mint run --tunnel example in the guide.

Low

[scope-creep] AGENTS.md — The ADR immutability rules were expanded to allow Note: paragraphs in Status sections for cross-reference discoverability. This meta-policy change is not mentioned in issue Standalone dev mint server (no GCP required) #1963. The change is sensible but goes beyond the stated scope of adding a dev mint. Consider filing a separate issue for this policy change.
[edge-case] internal/cli/mint_run.go:56 — The --insecure-no-auth guard rejects --bind localhost because net.ParseIP("localhost") returns nil, triggering the loopback check. While the default --bind 127.0.0.1 works and the error message provides correct guidance, a user passing --bind localhost gets a confusing rejection even though localhost resolves to a loopback address.
[debuggability] internal/devmint/devmint.go:255 — In handleToken, when OIDC verification is enabled, the verified claims are discarded (if _, err := ...). The dev mint cannot log which workflow/repo requested the token, reducing debuggability.
[permissive-defaults] internal/devmint/devmint.go:340 — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows.

Info

[test-coverage] internal/cli/admin_test.go — TestValidateSkipMintCheck does not cover the credential-embedding rejection case (http://user:pass@localhost:8321). The parsed.User != nil code path is untested.

Previous run (15)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.
[insecure-default] internal/devmint/devmint.go:157 — The dev mint defaults allowedWorkflows to ["*"] (wildcard) when no --allowed-workflows flag is provided. Since the prior review, the CLI now requires --allowed-workflows when --tunnel is set (mint_run.go line 54), which mitigates the most dangerous scenario (public tunnel + wildcard). However, the server-level default in devmint.go remains ["*"], so any caller constructing a Server directly (tests, future integrations) still inherits the permissive default. The wildcard default for the loopback-only case is an accepted trade-off documented in the security considerations section.

Low

[dead-code] internal/cli/admin.go:740 — skipMintDispatcher.StoreAgentPEM writes PEMs to disk when dataDir is set, but the install flow never calls this method. PEM storage is handled by the WithStoreSecret callback during app creation (which calls storePEMToDisk). The dispatch layer calls Provision(), not StoreAgentPEM(). The code comment acknowledges this. Harmless — satisfies the dispatch.Dispatcher interface contract.
[error-handling] internal/cli/admin.go:776 — In storePEMToDisk, the PEM file is written to disk before config.json is updated. A crash between writes leaves an orphaned PEM. The code comment documents this trade-off and notes self-healing on retry. Config.json write uses atomic rename.
[missing-test] internal/cli/admin_test.go — No tests verify the interaction between --mint-data-dir and the per-org or per-repo install flow end-to-end. Unit tests cover storePEMToDisk, skipMintDispatcher, resolveAppSetupMintProject, and the SecretExists/StoreSecret callbacks individually, but the integration through runAppSetup and needConfigRepo logic is not exercised. New tests for AppID propagation and individual callbacks are good progress.
[edge-case] internal/devmint/devmint.go:258 — The OIDC verification in handleToken discards the claims returned by s.oidcVerifier.Verify(). However, the verifier already enforces org restrictions via AllowedOrgs: []string{s.org} (the JWKS verifier calls ValidateOrgAllowed on claims.RepositoryOwner). The handler trusts the request body for role/repo selection after authentication, which is consistent with the production handler (mintcore.Handler). This is a deliberate trust-boundary decision for a single-org dev server, not a bypass.
[missing-test] internal/devmint/tunnel.go — StartTunnel has no test coverage in CI. Tests are skipped via CI env var check. The function has multiple failure modes (cloudflared not in PATH, process exits before emitting URL, timeout, URL regex mismatch). The "not in PATH" and "early exit" cases are tested locally but skipped in CI; consider testing at minimum the URL extraction via a mock or stub that can run in CI.
[architectural-drift] docs/ADRs/0007-per-role-github-apps.md, docs/ADRs/0029-central-token-mint-secretless-fullsend.md, docs/ADRs/0033-per-repo-installation-mode.md — These accepted ADRs have Note: paragraphs added in the Status section referencing ADR 0043. The AGENTS.md update in this PR codifies the Note pattern as an acceptable edit to Accepted ADRs when the new ADR extends (rather than contradicts) the old decision. This resolves the prior concern about setting an undocumented precedent.

Info

[prior-finding-resolved] internal/devmint/devmint.go:300 — The /v1/status endpoint now sorts roles alphabetically (sort.Slice added since prior review). The prior finding about nondeterministic order has been resolved.

Previous run (16)

Review

Findings

High

[documentation/code mismatch] docs/guides/infrastructure/dev-mint.md:73 — Step 3 of the dev-mint guide shows fullsend mint run --data-dir ~/.fullsend-mint --port 8321 --tunnel without --allowed-workflows. The code in internal/cli/mint_run.go enforces that --tunnel requires --allowed-workflows and will fail at startup with: --tunnel requires --allowed-workflows: a public tunnel with wildcard workflow access lets any GitHub Actions workflow in the org mint tokens. Users following this guide will hit an immediate error.
Remediation: Add --allowed-workflows '.github/workflows/dispatch.yml' (or an appropriate workflow path) to every fullsend mint run --tunnel example in the guide.

Low

[scope-creep] AGENTS.md — The ADR immutability rules were expanded to allow Note: paragraphs in Status sections for cross-reference discoverability. This meta-policy change is not mentioned in issue Standalone dev mint server (no GCP required) #1963. The change is sensible but goes beyond the stated scope of adding a dev mint. Consider filing a separate issue for this policy change.
[edge-case] internal/cli/mint_run.go:56 — The --insecure-no-auth guard rejects --bind localhost because net.ParseIP("localhost") returns nil, triggering the loopback check. While the default --bind 127.0.0.1 works and the error message provides correct guidance, a user passing --bind localhost gets a confusing rejection even though localhost resolves to a loopback address.
[debuggability] internal/devmint/devmint.go:255 — In handleToken, when OIDC verification is enabled, the verified claims are discarded (if _, err := ...). The dev mint cannot log which workflow/repo requested the token, reducing debuggability.
[permissive-defaults] internal/devmint/devmint.go:340 — Without --tunnel, allowedWorkflows defaults to wildcard ["*"], allowing any workflow in the org to request tokens. Acknowledged as accepted trade-off for dev use; the --tunnel path correctly enforces explicit --allowed-workflows.

Info

[test-coverage] internal/cli/admin_test.go — TestValidateSkipMintCheck does not cover the credential-embedding rejection case (http://user:pass@localhost:8321). The parsed.User != nil code path is untested.

Previous run (17)

Review

Findings

Medium

[protected-path] skills/mint-enroll/SKILL.md — This file is under the skills/ protected path. The change is a minor text update adding a cross-reference to the dev mint guide. The PR links to issue Standalone dev mint server (no GCP required) #1963 and the description explains the rationale. Human approval is required for protected-path changes regardless of context.
[insecure-default] internal/devmint/devmint.go:157 — The dev mint defaults allowedWorkflows to ["*"] (wildcard) when no --allowed-workflows flag is provided. Since the prior review, the CLI now requires --allowed-workflows when --tunnel is set (mint_run.go line 54), which mitigates the most dangerous scenario (public tunnel + wildcard). However, the server-level default in devmint.go remains ["*"], so any caller constructing a Server directly (tests, future integrations) still inherits the permissive default. The wildcard default for the loopback-only case is an accepted trade-off documented in the security considerations section.

Low

[dead-code] internal/cli/admin.go:740 — skipMintDispatcher.StoreAgentPEM writes PEMs to disk when dataDir is set, but the install flow never calls this method. PEM storage is handled by the WithStoreSecret callback during app creation (which calls storePEMToDisk). The dispatch layer calls Provision(), not StoreAgentPEM(). The code comment acknowledges this. Harmless — satisfies the dispatch.Dispatcher interface contract.
[error-handling] internal/cli/admin.go:776 — In storePEMToDisk, the PEM file is written to disk before config.json is updated. A crash between writes leaves an orphaned PEM. The code comment documents this trade-off and notes self-healing on retry. Config.json write uses atomic rename.
[missing-test] internal/cli/admin_test.go — No tests verify the interaction between --mint-data-dir and the per-org or per-repo install flow end-to-end. Unit tests cover storePEMToDisk, skipMintDispatcher, resolveAppSetupMintProject, and the SecretExists/StoreSecret callbacks individually, but the integration through runAppSetup and needConfigRepo logic is not exercised. New tests for AppID propagation and individual callbacks are good progress.
[edge-case] internal/devmint/devmint.go:258 — The OIDC verification in handleToken discards the claims returned by s.oidcVerifier.Verify(). However, the verifier already enforces org restrictions via AllowedOrgs: []string{s.org} (the JWKS verifier calls ValidateOrgAllowed on claims.RepositoryOwner). The handler trusts the request body for role/repo selection after authentication, which is consistent with the production handler (mintcore.Handler). This is a deliberate trust-boundary decision for a single-org dev server, not a bypass.
[missing-test] internal/devmint/tunnel.go — StartTunnel has no test coverage in CI. Tests are skipped via CI env var check. The function has multiple failure modes (cloudflared not in PATH, process exits before emitting URL, timeout, URL regex mismatch). The "not in PATH" and "early exit" cases are tested locally but skipped in CI; consider testing at minimum the URL extraction via a mock or stub that can run in CI.
[architectural-drift] docs/ADRs/0007-per-role-github-apps.md, docs/ADRs/0029-central-token-mint-secretless-fullsend.md, docs/ADRs/0033-per-repo-installation-mode.md — These accepted ADRs have Note: paragraphs added in the Status section referencing ADR 0043. The AGENTS.md update in this PR codifies the Note pattern as an acceptable edit to Accepted ADRs when the new ADR extends (rather than contradicts) the old decision. This resolves the prior concern about setting an undocumented precedent.

Info

[prior-finding-resolved] internal/devmint/devmint.go:300 — The /v1/status endpoint now sorts roles alphabetically (sort.Slice added since prior review). The prior finding about nondeterministic order has been resolved.

ggallen · 2026-06-04T23:59:32Z

Rebased on main (which now includes #1783 mintcore extraction). All review findings addressed:

Medium:

[data-exposure] /v1/status now requires OIDC authentication when --insecure-no-auth is not set, matching the Cloud Function handler's behavior.
[stale-doc] Added mint run to the CLI command tree in cli-internals.md.

Low:

[error-handling-idiom] storePEMToDisk now explicitly distinguishes IsNotExist from other read errors for config.json.
[missing-test] Added http://[::1]:8321 test case to TestValidateSkipMintCheck.
[edge-case] loadFromDisk() now wraps shared state writes in s.mu.Lock()/s.mu.Unlock().

Also updated mintcore API calls to match the merged #1783 interface (JWKSVerifierConfig struct, 4-arg ValidateWorkflowRef).

fullsend-ai-review

See the review comment for full details.

ggallen · 2026-06-05T10:12:25Z

All findings addressed:

Medium:

ensureConfigRepoExists gate: Added needConfigRepo guard that also triggers when skipMintCheck && mintDataDir == "" && mintProject == "", so the repo-secrets path has a target repo.
Duplicate PEM writes: Split into writePEMFile (PEM-only) and storePEMToDisk (PEM + config.json). WithStoreSecret and StoreAgentPEM now use writePEMFile; the post-loop uses storePEMToDisk with the correct appID. No more intermediate config.json with empty app_id.
Stale ADR 0033: Updated --skip-mint-check description to match actual behavior.

Low:

Path traversal in devmint: Added mintcore.RolePattern validation in loadFromDisk() + test.
Type duplication: Exported DiskConfig/DiskRoleConfig from devmint package; admin.go imports them.
Naming convention: Removed local rolePattern; parseAgentRoles uses mintcore.RolePattern directly.
Stale doc comment: Fixed runAppSetup comment (was mentioning /v1/pem endpoint).
Incomplete doc: Added fullsend mint run to infrastructure-reference.md.
Architectural coherence (mintcore.Handler reuse): Acknowledged as follow-up work.

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review

See the review comment for full details.

Adds a dev mint server that runs locally without any GCP infrastructure, making it easier to evaluate and develop fullsend. Key changes: - New `internal/devmint` package: HTTP dev mint using OIDC verification and on-disk PEM storage; supports cloudflared tunnel for remote access - New `internal/mintcore` package extracted to share JWT generation and OIDC verification between the dev mint and Cloud Function mint - `fullsend mint run` CLI command to start the dev mint server - `--mint-data-dir` flag on `admin install` writes PEMs directly to disk - `--skip-mint-check` now skips GCP provisioning only; GitHub Apps are still created (HTTP permitted for loopback addresses, 127.0.0.0/8) - ADR 0043 documents the updated `--skip-mint-check` semantics - Terminology standardized on "dev mint" throughout (package, docs, code) Security: - OIDC verification enabled by default (disable with --insecure-no-auth) - Default bind address is 127.0.0.1; tunnel flag uses cloudflared - --insecure-no-auth and --tunnel are mutually exclusive - --insecure-no-auth errors when combined with a non-loopback --bind - --tunnel requires explicit --allowed-workflows; wildcard needs '*' - allowedWorkflows wildcard resolved in Start() not New() - /v1/status and /v1/token require OIDC auth by default - handleToken and handleStatus: fail-closed on nil OIDC claims - handleToken and handleStatus: explicit RepositoryOwner == s.org check - loadFromDisk: validates org and role fields - Role names validated with ValidateRoleName (rejects double-hyphens) - SecretExists closure validates role before building file path - Stdout drained in StartTunnel; cloudflared early-exit via exitCh - Per-repo install path guards GCP calls with !skipMintCheck - --mint-data-dir validated to require --skip-mint-check - validateSkipMintCheck uses net.ParseIP().IsLoopback() - X-Fullsend-Dev-Mint response header removed to avoid fingerprinting Code quality: - Package-level godoc for internal/devmint - resolveAppSetupMintProject helper deduplicates install-path logic - Neutral error message language throughout - ADR 0033 supersession link; ADR 0043 citations in architecture.md - cli-internals.md Phase 6 PEM diagram reflects all storage options - /v1/status roles sorted alphabetically Testing: - TestInstallCmd_MintDataDirRequiresSkipMintCheck_{PerOrg,PerRepo} - TestOrgMismatchReturns403, TestNilClaimsFailClosed - TestStartTunnel_*: CI-safe fake-cloudflared pattern - TestValidateSkipMintCheck, TestStorePEMToDisk - TestResolveAppSetupMintProject, TestMintDataDir{SecretExists,StoreSecret}Callback - Existing devmint and appsetup test suites extended Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

ifireball · 2026-06-08T08:55:00Z

 │   ├── unenroll     <org|owner/repo>        # Remove org/repo from mint
-│   └── status       [org]                   # Inspect mint state and PEM health
+│   ├── status       [org]                   # Inspect mint state and PEM health
+│   └── run                                  # Run standalone token mint (no GCP required)


I kind of assumed we would end up with a production-worthy stand-alone version of the mint, but dev-mint.md lists many limitations that really indicate this is only for dev use, so I wonder if we should indicate this with the command name, because I do want to ultimately reach a point where we have a command that runs a production mint.

Or perhaps we need to aim toward this becoming a prod-ready deployment, file issues for the gaps, and print a loud warning at statup while it isn't.

My goal was a mint that could be standalone and allow someone to easily install and evaluate fullsend, hence all the limitation. We can expand that if necessary. Let me know what you think.

I want to make it so the command set under fullsend mint is good for setting up and managing all kinds of mint architectures, from GCP function, to Cloudflare worker to k8s deployment to local , but its hard for me to figure out what this needs to look like. I'm trying to see how to make the right choices now so we don't need a messy user-visible change later

ifireball · 2026-06-08T09:05:58Z

+Specifically:
+
+- GitHub Apps **are still created** when `--skip-app-setup` is not set.
+- PEM storage is determined by flag combination, evaluated in order:


I wonder if we should introduce variables like --mint-platform=gcp|local and --mint-secret-store=gcp|file that then require other variables as needed. This implicit emergence of the resulting architecture from the variables being set is confusing and will get more confusing when we support other deployment options such as Cloudflare and K8s.

I'm not sure if we should have a simple variable that select the platform and implies all other settings like secret loading backend and OIDC verification backend or wither we should have different setting to let the user select different combinations (though not all combinationsmight be possible or make sense)

rh-hemartin · 2026-06-23T09:34:46Z

@ggallen how does this relates to your standalone mint PR?

rh-hemartin · 2026-06-30T08:33:48Z

Does #2537 supersedes this?

github-actions Bot deployed to site-preview June 2, 2026 14:22 View deployment

ggallen mentioned this pull request Jun 2, 2026

feat: add mintcore shared library and dev mint for local evaluation #1751

Closed

6 tasks

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 2, 2026

rh-hemartin reviewed Jun 3, 2026

View reviewed changes

Comment thread docs/guides/infrastructure/dev-mint.md Outdated

ifireball assigned ggallen Jun 4, 2026

ggallen force-pushed the feat/dev-mint-v2 branch from abbd57b to 7c18c1e Compare June 4, 2026 23:59

github-actions Bot deployed to site-preview June 5, 2026 00:00 View deployment

fullsend-ai-review Bot suggested changes Jun 5, 2026

View reviewed changes

Comment thread internal/cli/admin.go

Comment thread internal/cli/admin.go

fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Jun 5, 2026

ggallen force-pushed the feat/dev-mint-v2 branch from 7c18c1e to 3f60601 Compare June 5, 2026 10:13

github-actions Bot deployed to site-preview June 5, 2026 10:14 View deployment

fullsend-ai-review Bot suggested changes Jun 5, 2026

View reviewed changes

Comment thread internal/cli/admin.go Outdated

Comment thread docs/guides/infrastructure/dev-mint.md

Comment thread docs/guides/getting-started/installation.md

Comment thread internal/devmint/devmint.go

ggallen force-pushed the feat/dev-mint-v2 branch from 3f60601 to eb71331 Compare June 5, 2026 17:53

github-actions Bot deployed to site-preview June 5, 2026 17:55 View deployment

fullsend-ai-review Bot suggested changes Jun 5, 2026

View reviewed changes

ggallen force-pushed the feat/dev-mint-v2 branch from eb71331 to f1b494a Compare June 5, 2026 18:14

github-actions Bot deployed to site-preview June 5, 2026 18:15 View deployment

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 5, 2026

ggallen force-pushed the feat/dev-mint-v2 branch from f1b494a to ad1a404 Compare June 5, 2026 18:44

github-actions Bot deployed to site-preview June 5, 2026 18:46 View deployment

fullsend-ai-triage Bot mentioned this pull request Jun 5, 2026

Standalone dev mint server (no GCP required) #1963

Open

5 tasks

fullsend-ai-review Bot added requires-manual-review Review requires human judgment and removed requires-manual-review Review requires human judgment labels Jun 5, 2026

ggallen force-pushed the feat/dev-mint-v2 branch from ad1a404 to a9d6619 Compare June 5, 2026 19:05

github-actions Bot deployed to site-preview June 5, 2026 19:07 View deployment

fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Jun 5, 2026

github-actions Bot deployed to site-preview June 6, 2026 12:53 View deployment