Skip to content

Validate Firefox publish channels - #607

Closed
rissrice2105-agent wants to merge 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/firefox-channel-validation
Closed

rissrice2105-agent wants to merge 1 commit into
profullstack:masterfrom
rissrice2105-agent:codex/firefox-channel-validation

Conversation

@rissrice2105-agent

Copy link
Copy Markdown
Contributor

Fixes #606.

Changes:

  • validate browser-firefox channel at runtime before package planning or shipping
  • keep package plans and ship logging constrained to listed/unlisted AMO channels
  • add regression tests for invalid channel in dry-run build and ship flows

Validation:

  • vitest run packages/targets/browser-firefox/src/index.test.ts
  • tsc -p packages/targets/browser-firefox/tsconfig.json --noEmit

@greptile-apps

greptile-apps Bot commented Jun 5, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds runtime validation of the Firefox AMO publish channel in both the build and ship paths, throwing immediately on any unsupported value before filesystem or network side-effects occur. Two regression tests cover the invalid-channel case for dry-run build and ship flows.

  • channel() helper centralises default ('listed') and validation logic, replacing inline config.channel ?? 'listed' expressions in packagePlan and ship.
  • The valid-channel set (CHANNELS) is now checked at runtime, but it is defined separately from the Config.channel union type and FirefoxPackagePlan.channel field — a future addition to one without updating the others could cause a mismatch.

Confidence Score: 4/5

Safe to merge; the validation logic is correct, tests cover the new throw paths, and no existing behaviour is broken.

The channel() function works correctly at runtime and the tests confirm it. The only concern is that the valid-channel set is expressed three times independently (CHANNELS, Config.channel, FirefoxPackagePlan.channel), leaving room for silent drift if a channel is added later.

packages/targets/browser-firefox/src/index.ts — the three separate declarations of 'listed' | 'unlisted' are worth unifying.

Important Files Changed

Filename Overview
packages/targets/browser-firefox/src/index.ts Adds a channel() helper that validates and returns the AMO channel before any side effects in both build and ship; the valid channel set is currently duplicated across CHANNELS, the Config type, and FirefoxPackagePlan.
packages/targets/browser-firefox/src/index.test.ts Adds two regression tests covering invalid-channel rejection in both dry-run build and ship paths; imports fakeShipContext as needed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build / ship called] --> B[channel config]
    B --> C{config.channel set?}
    C -- No --> D[default to 'listed']
    C -- Yes --> E[use provided value]
    D --> F{value in CHANNELS?}
    E --> F
    F -- No --> G[throw Error: must be one of listed, unlisted]
    F -- Yes --> H[return selectedChannel]
    H --> I{ship path?}
    H --> J{build path?}
    I --> K[ctx.log sign+submit]
    K --> L{dryRun?}
    L -- Yes --> M[return dry-run]
    L -- No --> N[submit to AMO]
    J --> O[packagePlan created]
    O --> P{dryRun?}
    P -- Yes --> Q[write .firefox-plan.json]
    P -- No --> R[exec web-ext build]
Loading

Comments Outside Diff (1)

  1. packages/targets/browser-firefox/src/index.ts, line 11-19 (link)

    P2 The valid channels are defined in three separate places: the CHANNELS const, the Config.channel union type, and FirefoxPackagePlan.channel. If a new channel is added to only one of them (e.g., adding it to the Config type but forgetting CHANNELS), a TypeScript-valid value would throw at runtime. Deriving the type from the const keeps all three in sync automatically.

Reviews (1): Last reviewed commit: "Validate Firefox publish channels" | Re-trigger Greptile

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

6 similar comments
@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: git fetch upstream master && git rebase upstream/master.

@ralyodio ralyodio closed this Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

browser-firefox accepts unsupported publish channels

2 participants