feat(dockerhub): add Docker Hub plugin - #482
Conversation
Implement @corsair-dev/dockerhub for Hub API v2: repositories, tags, images, organizations, teams, and repository webhooks. Auth via Personal Access Token (Bearer); optional JWT login for create org. Closes corsairdev#481 after Loom + PR.
|
@Mayank-saraswal is attempting to deploy a commit to the corsair Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
core
Greptile SummaryThis PR introduces a complete Docker Hub plugin (
Confidence Score: 5/5Safe to merge. All 26 endpoints are implemented, tested, and scoped correctly within the plugin boundary. The two substantive bugs caught in the prior review round have both been corrected. The implementation follows plugin conventions throughout — error handler chain, Zod validation, pagination on list endpoints, no hardcoded secrets, no boilerplate residue. Files Needing Attention: handlers.test.ts — the organizations.create test assertion could be tightened; not a blocker. Important Files Changed
Reviews (16): Last reviewed commit: "Merge branch 'main' into feat/docker_hub..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Plugin PR scorecard —
|
| Check | Status | Notes |
|---|---|---|
| R1 — Scope: plugin files only | ✅ | |
| R2 — Tests with assertions | ✅ | |
| R3 — Description complete | ✅ | |
| R3 — Linked issue / claim | ✅ | |
| R4 — Demo video / recording | ✅ |
Rules: PLUGIN_PR_RULES.md · re-runs on every push
|
Hey @Mayank-saraswal, thanks for the contribution! 🏴☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push. Must fix
const ImagesGetInputSchema = z.object({ Rule Used: Every endpoint must validate inputs and outputs wi... (source) If anything remains after your next push, a bot commit will clean it up; a maintainer always does the final review and merge. |
Greptile P1: ImagesGetInputSchema no longer exposes page (only pageSize). Handler always starts the digest scan at page 1 so callers cannot skip early tags. Also repair pnpm-lock importer snapshot after main merge.
|
Remaining findings are being fixed by a bot commit — it will be re-reviewed automatically. |
Maintainer review neededAutomated rounds are exhausted. Remaining findings:
const ImagesGetInputSchema = z.object({ Rule Used: Every endpoint must validate inputs and outputs wi... (source) |
github-actions
Bot
added
the
needs-maintainer
Greptile: null-id early return logged completed without attaching hookUrl. Log failed and throw so callers know the pipeline is incomplete.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a first-class ChangesDocker Hub integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant WebhooksCreate
participant req
participant DockerHubAPI
WebhooksCreate->>req: Register webhook pipeline
req->>DockerHubAPI: POST pipeline
DockerHubAPI-->>WebhooksCreate: Return pipeline id
WebhooksCreate->>req: Attach hook URL
req->>DockerHubAPI: POST hook URL
DockerHubAPI-->>WebhooksCreate: Return webhook result
WebhooksCreate->>req: Delete pipeline if attachment fails
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/dockerhub/scripts/demo.mjs (1)
16-21: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the plugin rather than raw HTTP.
This demo can succeed while
dockerhub()wiring, endpoint bindings, key building, or plugin error handling are broken. Instantiate Corsair withdockerhub()and callclient.dockerhub.*so the documented demo validates the distributed package.#!/bin/bash # Find existing plugin demos and their Corsair authentication setup. fd -t f -E node_modules 'demo\.(mjs|js|ts)' packages | while read -r file; do ast-grep outline "$file" --items all done rg -n -C 3 --glob '*.{mjs,js,ts,tsx}' '\bcorsair\s*\(' packages🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dockerhub/scripts/demo.mjs` around lines 16 - 21, Update the demo’s request flow around get to instantiate Corsair with the dockerhub() plugin and invoke the documented client.dockerhub.* methods instead of calling fetch against raw URLs. Preserve the demo’s authentication setup and output while ensuring endpoint bindings, key construction, and plugin error handling are exercised through the distributed package.packages/dockerhub/tsconfig.json (1)
17-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude test files from the published declaration build.
Because
includematches the entire package andtscemits declarations intodist, root-level test files such asapi.test.tsandhandlers.test.tswill generate.d.tsfiles that are published via"files": ["dist"]. Exclude test files or narrow the include list.Proposed fix
- "exclude": ["dist", "node_modules"], + "exclude": ["dist", "node_modules", "**/*.test.ts"],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dockerhub/tsconfig.json` around lines 17 - 18, Update the tsconfig include/exclude configuration so root-level test files such as api.test.ts and handlers.test.ts are omitted from the declaration build and cannot emit files under dist. Preserve compilation of the package source while ensuring the existing dist and node_modules exclusions remain effective.packages/dockerhub/jest.config.cjs (1)
11-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExclude test files from coverage collection.
**/*.tsalso matches the package’s root-level*.test.tsfiles, so coverage metrics can include test code and become misleading. Add an explicit test-file exclusion.Proposed fix
collectCoverageFrom: [ '**/*.ts', + '!**/*.test.ts', '!**/*.d.ts',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dockerhub/jest.config.cjs` around lines 11 - 18, Update the collectCoverageFrom configuration to explicitly exclude root-level and nested test files matching the package’s *.test.ts naming pattern, while preserving the existing source, declaration, dependency, build, and tests/** exclusions.packages/dockerhub/endpoints/organizations.ts (1)
26-61: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPrefer spreading
ctxinstead of constructing a bare{ key: token }.Every other handler in this file (and across the plugin) passes the full
ctxtoreq. Here,createsubstitutes a stripped-down object containing onlykey, discardingdb,keys,authType, andoptions. Ifreq(or anything it calls) ever needs those fields — e.g. for tenant-scoped rate-limit bookkeeping or richer error context — this silently breaks only for the JWT-fallback org-create path.♻️ Proposed fix
- const response = await req({ key: token }, '/orgs/', { + const response = await req({ ...ctx, key: token }, '/orgs/', { method: 'POST', body: {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dockerhub/endpoints/organizations.ts` around lines 26 - 61, Update the req invocation in organizationsCreate to pass the full ctx while overriding only its key with the selected token. Preserve the JWT/PAT token selection logic and ensure db, keys, authType, options, and all other context fields remain available to req.packages/dockerhub/handlers.test.ts (1)
178-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLoose assertion doesn't verify the JWT-fallback flow.
The
.some(p => p === '/orgs/' || p === '/users/login/')check passes as long as either path was called — it wouldn't catch a bug wherecreate()calls/users/login/but never reaches the actual/orgs/POST (or vice versa). Consider asserting both the login call and the final/orgs/POST with its body.♻️ Proposed tightening
- // may call login first when username set — last call is create - const paths = mockReq.mock.calls.map((c) => c[0]); - expect(paths.some((p) => p === '/orgs/' || p === '/users/login/')).toBe( - true, - ); + const paths = mockReq.mock.calls.map((c) => c[0]); + expect(paths).toContain('/users/login/'); + expect(lastCall()[0]).toBe('/orgs/'); + expect(lastCall()[2]?.method).toBe('POST');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dockerhub/handlers.test.ts` around lines 178 - 185, Strengthen the organizations.create test by asserting that the request paths include both the optional /users/login/ call and the final /orgs/ POST, rather than accepting either one; also verify the /orgs/ request carries the expected organization body for acme.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/dockerhub/jest.config.cjs`:
- Line 21: Make the dockerhub Jest setup self-contained by removing its
hard-coded references to packages/corsair, including the yaml transformer and
the related entries around the referenced configuration lines. Move or expose
the required test support through a package-local implementation or stable
workspace API, then update the Jest configuration to use that boundary while
preserving existing test behavior.
In `@packages/dockerhub/README.md`:
- Around line 57-60: Update the “Local test / demo (R4 Loom)” section to remove
the machine-specific cd path, replacing it with a generic repository-path
placeholder or omitting the cd command entirely.
---
Nitpick comments:
In `@packages/dockerhub/endpoints/organizations.ts`:
- Around line 26-61: Update the req invocation in organizationsCreate to pass
the full ctx while overriding only its key with the selected token. Preserve the
JWT/PAT token selection logic and ensure db, keys, authType, options, and all
other context fields remain available to req.
In `@packages/dockerhub/handlers.test.ts`:
- Around line 178-185: Strengthen the organizations.create test by asserting
that the request paths include both the optional /users/login/ call and the
final /orgs/ POST, rather than accepting either one; also verify the /orgs/
request carries the expected organization body for acme.
In `@packages/dockerhub/jest.config.cjs`:
- Around line 11-18: Update the collectCoverageFrom configuration to explicitly
exclude root-level and nested test files matching the package’s *.test.ts naming
pattern, while preserving the existing source, declaration, dependency, build,
and tests/** exclusions.
In `@packages/dockerhub/scripts/demo.mjs`:
- Around line 16-21: Update the demo’s request flow around get to instantiate
Corsair with the dockerhub() plugin and invoke the documented client.dockerhub.*
methods instead of calling fetch against raw URLs. Preserve the demo’s
authentication setup and output while ensuring endpoint bindings, key
construction, and plugin error handling are exercised through the distributed
package.
In `@packages/dockerhub/tsconfig.json`:
- Around line 17-18: Update the tsconfig include/exclude configuration so
root-level test files such as api.test.ts and handlers.test.ts are omitted from
the declaration build and cannot emit files under dist. Preserve compilation of
the package source while ensuring the existing dist and node_modules exclusions
remain effective.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 793ecd1e-1b3d-49b6-a6b9-64a56010fbe9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
packages/corsair/core/constants.tspackages/dockerhub/README.mdpackages/dockerhub/api.test.tspackages/dockerhub/client.tspackages/dockerhub/endpoints/helpers.tspackages/dockerhub/endpoints/images.tspackages/dockerhub/endpoints/index.tspackages/dockerhub/endpoints/organizations.tspackages/dockerhub/endpoints/repositories.tspackages/dockerhub/endpoints/tags.tspackages/dockerhub/endpoints/teams.tspackages/dockerhub/endpoints/types.tspackages/dockerhub/endpoints/webhooks.tspackages/dockerhub/error-handlers.tspackages/dockerhub/handlers.test.tspackages/dockerhub/index.tspackages/dockerhub/jest.config.cjspackages/dockerhub/package.jsonpackages/dockerhub/schema/database.tspackages/dockerhub/schema/index.tspackages/dockerhub/scripts/demo.mjspackages/dockerhub/tsconfig.jsonpackages/dockerhub/tsup.config.tspackages/dockerhub/webhooks/index.tspackages/dockerhub/webhooks/oauth-tenant-link.tspackages/dockerhub/webhooks/tenant-matcher.tspackages/dockerhub/webhooks/types.ts
| ], | ||
| moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json'], | ||
| transform: { | ||
| '^.+\\.yaml$': '<rootDir>/../corsair/jest-yaml-transform.cjs', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep the plugin’s test setup self-contained.
The Jest configuration hard-codes sibling packages/corsair source paths and an internal transformer. This couples the plugin to the monorepo layout and another package’s implementation details; move shared test support behind a stable package-local/workspace API instead.
As per path instructions: Each plugin should remain self-contained within its own packages/<plugin>/ package, except for its required registration in packages/corsair/core/constants.ts.
Also applies to: 46-48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dockerhub/jest.config.cjs` at line 21, Make the dockerhub Jest setup
self-contained by removing its hard-coded references to packages/corsair,
including the yaml transformer and the related entries around the referenced
configuration lines. Move or expose the required test support through a
package-local implementation or stable workspace API, then update the Jest
configuration to use that boundary while preserving existing test behavior.
Source: Path instructions
| ## Local test / demo (R4 Loom) | ||
|
|
||
| ```powershell | ||
| cd D:\opensource\corsair |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the machine-specific checkout path.
D:\opensource\corsair only works on the author’s machine. Use a placeholder such as <path-to-corsair-repo> or omit the cd step.
Proposed fix
-cd D:\opensource\corsair
+cd <path-to-corsair-repo>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Local test / demo (R4 Loom) | |
| ```powershell | |
| cd D:\opensource\corsair | |
| ## Local test / demo (R4 Loom) | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dockerhub/README.md` around lines 57 - 60, Update the “Local test /
demo (R4 Loom)” section to remove the machine-specific cd path, replacing it
with a generic repository-path placeholder or omitting the cd command entirely.
Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Description
Adds a first-class Docker Hub plugin (
@corsair-dev/dockerhub, package folderdockerhub, OSS slugdocker_hub) for Docker Hub API v2: repositories, tags, images, organizations, teams, and repository webhook REST surfaces claimed on the OSS dashboard.Closes #481
What was built
Authorization: Bearer); optionalusernamefor JWT exchange on create-orghttps://hub.docker.com/v2Scope (R1)
packages/dockerhub/**packages/corsair/core/constants.ts(registration + display name Docker Hub)pnpm-lock.yamlChecklist
pnpm lintand all checks passpnpm typecheckand there are no TypeScript errorspnpm buildand all packages build successfullypnpm testand all tests passScreenshots / Demos (if applicable)
R4 Loom (offline tests + live public Hub demo):
https://www.loom.com/share/8321f25934ef45a2bf0bfd23dbd1f7f0
Additional Notes
library/*GETs work without a token; PAT for private/write/org opsSummary by CodeRabbit