Skip to content

De-duplicate routes/coding.ts — one clone-URL parser, one owner helper, one build-run lookup #304

Description

@serge-ivo

workers/api/src/routes/coding.ts is the busiest route file in the API (1738 lines), and three
things in it have been copy-pasted rather than shared. None of them is a bug today; each is a
place where a future fix lands in one copy and not the others.

The drift

  1. The clone-URL regex, twice, verbatim.
    /github\.com[:/]([\w.-]+\/[\w.-]+?)(?:\.git)?\/?$/i appears at the add-repo path and again
    in detect-github. The second site's own comment already says "same shape as the clone-URL
    path" — the duplication was noticed when it was written.

  2. githubRepo.split("/")[0], five times, in two shapes.
    Three copies of full.split("/")[0] (latestRunFor, /deployment, /deployments) and two
    of repo.githubRepo ? repo.githubRepo.split("/")[0] : "" (delegateToTarget, the
    workflow-start route). "Take the owner out of an owner/repo" is a rule, and it is written
    out five times.

  3. /deployment inlines latestRunFor.
    latestRunFor is defined ~10 lines above GET …/repos/:repoId/deployment, and that route
    then repeats its body verbatim — same guard order, same perPage: 1, same
    .catch(() => null), same "status" in res check. The only difference is the wire shape on
    the unavailable path ({available:false} with no run key), which is a reason to adapt the
    result, not to re-implement the lookup.

Scope — pure de-duplication, nothing else

Extract the shared helpers, change no behaviour and no wire shape. Specifically not in
scope:

  • No provider abstraction. Implement provider-neutral Repo Coder support for GitLab and Bitbucket #221 was deferred on evidence and that decision stands; folding a
    mechanical cleanup into a provider layer would make this look like the start of an
    architecture it is not.
  • Do not unify the two token fetches. delegateToTarget guards
    installationTokenForOwner with .catch(() => null); the workflow-start route does not, so a
    token failure there propagates. That is a real behavioural difference and changing it is a
    separate decision, not a de-dup.
  • /deployments (plural) keeps its unauthenticated fallback. Its guard deliberately omits
    githubAppConfigured so public repos show builds with no App installed (CODER-010, CODER-010: Builds fallback for public repos without the GitHub App (unauthenticated read) #121), so
    it cannot call latestRunFor — it only shares the owner helper.

Unit tests for the extracted helpers, following the pickNextIssue precedent already in this
file (pure helper exported from the route module, tested in routes/coding.test.ts).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendBackend / Worker / API workenhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions