fix(#1129): allow consecutive hyphens in GitHub org names

[shairevivo]

Summary

Allow valid GitHub organization and owner names containing consecutive hyphens. The old restriction protected a secret-name format that no longer includes the organization name.

Related Issue

Fixes #1129

Changes

• remove the obsolete consecutive-hyphen rejection from shared organization validation
• use shared mintcore validation in GCF provisioning and CLI repository-owner paths
• synchronize the embedded mintcore source and add regression coverage

Testing

• make lint passes (stage changes first, then run)
• Tests added/updated for new or modified logic
• Affected mintcore, CLI, and GCF package tests pass
• Embedded-source race test, WASM build, vet, and patch-coverage checks pass
• CI / test passes the complete required go test -race -coverprofile=coverage.out ./... command

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• Commits are signed off (DCO) — human and human-directed agent sessions only
• I wrote this contribution myself and can explain all changes in it

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[qodo-code-review]

PR Summary by Qodo

Allow consecutive hyphens in GitHub organization names

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Accept valid GitHub owners and organizations containing consecutive hyphens.
• Reuse mintcore validation across CLI and GCF provisioning paths.
• Synchronize embedded mintcore code and add cross-path regression coverage.

Diagram

graph TD
  A["GitHub Name"] --> B["CLI Paths"] --> D["Mintcore Validator"] --> E{"Valid Name?"} -->|Yes| F["WIF Configuration"]
  A --> C["GCF Provisioner"] --> D
  E -->|No| G["Validation Error"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Fully centralize CLI organization validation

• ➕ Eliminates remaining duplicated boundary and character checks
• ➕ Prevents future behavior drift between CLI and mintcore
• ➖ Would replace the CLI's specific validation messages with mintcore's generic error
• ➖ Requires richer shared errors to preserve current CLI diagnostics

Recommendation: The PR's bounded approach is appropriate: shared mintcore validation governs GCF and repository-owner paths, while the CLI-specific validator retains actionable error messages. Full delegation should only follow if mintcore exposes sufficiently detailed validation errors.

Files changed (8) +30 / -33

Bug fix (4) +11 / -16

admin.goPermit consecutive hyphens in CLI GitHub owners +2/-5

Permit consecutive hyphens in CLI GitHub owners

• Removes the obsolete consecutive-hyphen rejection from organization validation. Repository-owner matching now references the shared mintcore GitHub organization pattern instead of maintaining a stricter local regex.

internal/cli/admin.go

patterns.go.embedSynchronize embedded organization validation +3/-4

Synchronize embedded organization validation

• Updates the embedded mintcore source to allow consecutive hyphens and removes documentation for the obsolete secret-name ambiguity restriction.

internal/dispatch/gcf/mintsrc/mintcore/patterns.go.embed

provisioner.goCentralize GCF organization validation +3/-3

Centralize GCF organization validation

• Replaces repeated pattern and double-hyphen checks with mintcore.ValidateOrgName across full provisioning, repository-scoped WIF, and organization-scoped WIF paths.

internal/dispatch/gcf/provisioner.go

patterns.goRemove obsolete double-hyphen restriction +3/-4

Remove obsolete double-hyphen restriction

• Allows GitHub organization and user names containing consecutive hyphens when they otherwise match GitHubOrgPattern. Updates comments to reflect the relaxed validation contract.

internal/mintcore/patterns.go

Tests (4) +19 / -17

admin_test.goCover consecutive-hyphen CLI organizations +1/-1

Cover consecutive-hyphen CLI organizations

• Adds a double-hyphen organization name to the valid organization-name test cases.

internal/cli/admin_test.go

inference_test.goCover consecutive-hyphen repository owners +3/-3

Cover consecutive-hyphen repository owners

• Updates repository-mode parsing coverage to verify that an owner containing consecutive hyphens is preserved in parsed organization and repository values.

internal/cli/inference_test.go

provisioner_test.goExercise double-hyphen organizations throughout WIF provisioning +11/-12

Exercise double-hyphen organizations throughout WIF provisioning

• Uses consecutive-hyphen organizations in full-flow, organization-scoped, and repository-scoped provisioning tests. Updates expected provider IDs, attribute conditions, environment variables, and IAM principals, and removes the obsolete invalid-owner case.

internal/dispatch/gcf/provisioner_test.go

foreign_test.goRegress foreign-target organization validation +4/-1

Regress foreign-target organization validation

• Confirms foreign target organizations may contain consecutive hyphens while retaining coverage for invalid leading hyphens.

internal/mintcore/foreign_test.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

1. make go-test does not pass 📘 Rule violation ▣ Testability

Description

The PR modifies Go code under internal/, but its description states that make go-test still
fails in two packages. Even if the failures are pre-existing, the required full Go test command has
not completed successfully for this change.

Code

internal/mintcore/patterns.go[R25-27]

+// ValidateOrgName checks that an org name matches GitHubOrgPattern.
func ValidateOrgName(org string) error {
-	if !GitHubOrgPattern.MatchString(org) || strings.Contains(org, "--") {
+	if !GitHubOrgPattern.MatchString(org) {

Relevance

●● Moderate

Pre-existing failures are documented, but the repository rule explicitly requires the full Go test
command to pass.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062046 requires passing Go unit tests for changes under internal/. The cited
file contains modified Go logic, while the PR description explicitly reports that make go-test
reaches two failures rather than exiting successfully.

Rule 1062046: Require Go unit tests to pass before committing changes in cmd/ or internal/
internal/mintcore/patterns.go[25-30]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The required `make go-test` command does not complete successfully for this PR.

## Issue Context
PR Compliance ID 1062046 requires changes under `internal/` to be gated by a successful `make go-test` or `go test ./...` run. Resolve the reported failures or provide an enforced CI run in which the complete required test command passes before merge.

## Fix Focus Areas
- internal/mintcore/patterns.go[25-27]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 65 rules

✅ Skills: writing-user-docs, writing-adrs

Review mode: ⚖️ Balanced: This is a cross-cutting runtime validation change spanning shared mintcore logic, CLI parsing, GCF provisioning, embedded source synchronization, and security-sensitive identity/IAM paths.

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[shairevivo]

Compliance evidence is now recorded in the PR description: the enforced CI / test job passed go test -race -coverprofile=coverage.out ./... on commit 7e302366. No production change is needed for this finding.

[rh-hemartin]

tried to create test--org--hemartin and it stripped the double hypen and replaced with a single hypen. See https://github.com/test-org-hemartin. So the original review fullsend did (#1113 (comment)):

[correctness] internal/dispatch/gcf/provisioner.go:1179 — The strings.Contains(org, "--") check in ProvisionWIF() org validation rejects GitHub org/user names with consecutive hyphens (e.g., my--org), which GitHub allows. The existing githubOrgPattern regex already constrains the character set. This is consistent with the same check in Provision() (line 482) but could cause confusing rejections for valid org names.
Remediation: Remove the -- check from both Provision() and ProvisionWIF(), or document the GCP-side constraint that motivates it.

was (and is) wrong. So the limitation is a GitHub one. About the secret manager, I don't recall currently if the secrets are stored with these double hypens, if I'm correct they are now stored by identity, not by GH org.

[shairevivo]

Thanks @rh-hemartin. just curious ... are you able to run the tests locally? (i want to know for future cases). If it is a github limitation I assume it will be worthless to add this fix. should I close it?

[rh-hemartin]

Thanks @rh-hemartin. just curious ... are you able to run the tests locally? (i want to know for future cases). If it is a github limitation I assume it will be worthless to add this fix. should I close it?

I'm getting problems in the tests (gh pr checkout 6954 && make test):

-- FAIL: TestDummyRuntime_ClearIterationArtifacts (0.24s)
    dummy_test.go:270:
        	Error Trace:	/home/hemartin/git/fullsend-ai/fullsend/internal/runtime/dummy_test.go:270
        	Error:      	An error is expected but got nil.
        	Test:       	TestDummyRuntime_ClearIterationArtifacts
--- FAIL: TestDummyPlaybackRuntime_ClearIterationArtifacts (0.18s)
    dummy_playback_test.go:62:
        	Error Trace:	/home/hemartin/git/fullsend-ai/fullsend/internal/runtime/dummy_playback_test.go:62
        	Error:      	An error is expected but got nil.
        	Test:       	TestDummyPlaybackRuntime_ClearIterationArtifacts
--- FAIL: TestDummyRuntime_Bootstrap (0.05s)
    dummy_test.go:248:
        	Error Trace:	/home/hemartin/git/fullsend-ai/fullsend/internal/runtime/dummy_test.go:248
        	Error:      	An error is expected but got nil.
        	Test:       	TestDummyRuntime_Bootstrap
Agent model resolution: skipped for /tmp/TestAgentDefinitionModel2956912247/001/unterminated.md: agent definition: unterminated frontmatter
--- FAIL: TestDummyPlaybackRuntime_Bootstrap (0.07s)
    dummy_playback_test.go:54:
        	Error Trace:	/home/hemartin/git/fullsend-ai/fullsend/internal/runtime/dummy_playback_test.go:54
        	Error:      	An error is expected but got nil.
        	Test:       	TestDummyPlaybackRuntime_Bootstrap
FAIL

And yes, I think we should close this PR and its issue as it is a GH limitation.

[rh-hemartin]

I filed this issue for the test failures: #6999

[shairevivo]

Closing as this turns to be a github limitation.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 8:10 AM UTC · Completed 8:24 AM UTC

Commit: 7e30236 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.06

[fullsend-ai-retro]

Retro: PR #6954 — allow consecutive hyphens in GitHub org names

Outcome: Closed without merging. A human reviewer discovered the premise was factually wrong.

Timeline

1. May 19: Review agent on PR #1113 filed follow-up issue #1129, asserting that GitHub allows org names with consecutive hyphens like my--org. This is factually incorrect — GitHub strips consecutive hyphens.
2. May 19: Triage agent correctly identified the -- check as intentional (secret-name delimiter parsing) but still labeled ready-to-code because it has no "by design" label path.
3. May 31: Prioritize agent scored RICE 0.1 (very low).
4. Sep 3: Human contributor opened PR #6954 to fix Follow-up from PR #1113: strings.Contains(org, "--") check rejects valid GitHub org names with consecutive hyp... #1129, modifying 8 files across mintcore, CLI, and GCF packages.
5. Sep 3: No fullsend review agent was dispatched (empty dispatch matrix). Only Qodo bot reviewed.
6. Sep 3: Human reviewer @rh-hemartin tested the premise by attempting to create a GitHub org with consecutive hyphens — GitHub strips them. Invalidated the PR.
7. Sep 4: PR closed. Pre-existing test failures separately filed as #6999.

Existing issues with new evidence

• #2478: Issue Follow-up from PR #1113: strings.Contains(org, "--") check rejects valid GitHub org names with consecutive hyp... #1129 is a direct case where the triage agent confirmed the behavior was intentional yet still applied ready-to-code because no "by design" path exists. The triage agent proposed fix options (document or change delimiter), which mapped to ready-to-code even though the underlying report was not a genuine defect.
• #1890: The upstream cause of this wasted effort was the review agent's unverified factual claim about GitHub's platform behavior. Review agent should challenge the factual premise of agent-generated fix PRs #1890 targets premise verification for agent-authored fix PRs; the finding generation step that produced the incorrect claim is a related but broader surface.

Observations

• Issue Follow-up from PR #1113: strings.Contains(org, "--") check rejects valid GitHub org names with consecutive hyp... #1129 remains open with ready-to-code despite its premise being disproven by the PR review. It could attract future code attempts.
• No fullsend review agent coverage: The dispatch returned an empty matrix for this external contributor's PR (only Qodo bot reviewed). The fullsend review agent might have caught the premise issue — or might not — but the absence of coverage is notable.

Proposals filed

• Review agent should qualify claims about external platform behavior as hypotheses (in fullsend-ai/agents)