fix: skip auth check in extension push --dry-run

[stack72]

Summary

• swamp extension push --dry-run now works without authentication
• Auth loading (step 3) and collective membership validation (step 4) are wrapped in a !dryRun guard so they only run for real pushes
• Unblocks CI validation workflows that don't have swamp-club credentials

What changed

In src/cli/commands/extension_push.ts, the auth check and collective membership validation previously ran unconditionally before the dry-run exit point. Since --dry-run only builds the archive locally and never contacts the registry, authentication is unnecessary.

The credentials variable is now typed as | undefined and only populated when not in dry-run mode. The existing non-dry-run code paths (steps 13 and 17) already have their own if (!options.dryRun) guards, so credentials are never accessed in dry-run mode.

Trade-off

Dry-run will skip collective ownership validation. This is acceptable because:

• Dry-run validates the build, not permissions — its purpose is to verify the extension compiles and packages correctly
• The real push still enforces collective membership — no security bypass
• CI pipelines just need to verify the extension archives correctly without needing registry credentials

Test plan

• deno check passes
• deno lint passes
• deno fmt passes
• deno run test passes
• deno run compile passes
• swamp extension push <manifest> --dry-run succeeds without auth
• swamp extension push <manifest> without auth still fails with "Not authenticated"

Fixes #717

🤖 Generated with Claude Code

[github-actions]

Review: APPROVED

This PR correctly implements skipping authentication for --dry-run mode in extension push. The logic is sound and type-safe.

Analysis

Code Quality: The changes maintain TypeScript strict mode compliance. The credentials variable is properly typed as | undefined and the non-null assertions (credentials!) are only used within code paths that are guaranteed to be non-dry-run (either inside explicit if (!options.dryRun) blocks or after the dry-run early return).

Security: The trade-off is correctly assessed in the PR description. Dry-run only validates the build locally and never contacts the registry, so authentication is genuinely unnecessary. The real push path still enforces full authentication and collective membership validation.

DDD: This is an appropriate application-layer change—dry-run is an orchestration concern, not domain logic.

Suggestions (non-blocking)

1. Test coverage for the new behavior: The existing --dry-run integration test still creates auth.json. Consider adding a dedicated test that explicitly verifies --dry-run succeeds without any auth credentials, which would document and protect the new behavior. Example:

   Deno.test("extension push --dry-run succeeds without auth credentials", async () => {
     const tmpDir = await initTempRepo();
     const fakeHome = await Deno.makeTempDir(); // No auth.json
     try {
       // ... create model and manifest ...
       const { code } = await runCli(
         ["extension", "push", manifestPath, "--repo-dir", tmpDir, "--dry-run", "-y"],
         { HOME: fakeHome, XDG_CONFIG_HOME: join(fakeHome, ".config") },
       );
       assertEquals(code, 0);
     } finally { /* cleanup */ }
   });

2. Minor style consideration: The credentials! assertions are type-safe given the control flow, but if you prefer to avoid non-null assertions, you could extract the non-dry-run logic into a helper or use an early return pattern. This is purely stylistic—the current approach is clear and correct.

---

LGTM - the change is well-reasoned and correctly implemented.

[github-actions]

Adversarial Review

I traced every code path in the PR and attempted to find logic errors, security issues, and failure modes. Here are my findings:

Critical / High

None.

Medium

1. Missing test coverage for new behavior (integration/extension_push_test.ts)
   • The PR claims --dry-run now works without auth, but all existing dry-run tests (lines 218-394, 452-535, 649-751) create auth.json before running. There's no test that verifies dry-run succeeds when auth is absent.
   • Impact: If a future change breaks this behavior, CI won't catch it.
   • Suggested fix: Add a test case that runs --dry-run with HOME pointing to a directory without auth.json and asserts success.

Low

1. Non-null assertions mask future refactoring risks (extension_push.ts:343, 346, 499, 516, 530)
   • The credentials! assertions are currently safe because:
     • Lines 343, 346: Inside if (!options.dryRun) where credentials was populated earlier
     • Lines 499, 516, 530: After the if (options.dryRun) return early exit at line 484
   • However, TypeScript's flow analysis can't prove this across the async function boundary. If someone moves code around without understanding the invariant, they could introduce a runtime null dereference.
   • Impact: Low - TypeScript will catch most issues, and the control flow is straightforward.
   • Mitigation: The code structure is clear and the comment at step 3 documents the intent.

Security Assessment

The trade-off documented in the PR is acceptable:

• Dry-run only builds locally and never contacts the registry
• Server-side collective membership validation (step 4) is skipped, but local content collective validation (step 9c) still runs
• The real push path still enforces all authentication and authorization checks

Verdict

PASS - The control flow is sound, the non-null assertions are safe given the current structure, and the security trade-off is acceptable for the intended use case (CI validation without credentials). The missing test coverage is the main gap but doesn't block the PR.