feat: let Binjovi create a repository, and expose it via MCP #1716
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/repository-creation"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Finishes the repository-creation half of Binjovi's Services layer (settings, branch/tag policy and collaborator grants were already generic; creation was explicitly denied at three layers plus a locking test). Repository creation is what Forgejo's own MAX_CREATION_LIMIT=0 makes admin-only, which is why this needed a real design pass rather than just deleting the denial -- see the commit messages for the reasoning at each step.
Five commits, each independently tested against a real throwaway PostgreSQL (tests/with-postgres.sh):
Deliberately out of scope for this PR (each is a distinct, separable piece of follow-up work):
https://claude.ai/code/session_01JGe4pyhQ36AhgDfPJry4d3
Repository creation was denied at three layers plus a locking test that proved zero HTTP calls happened for kind=repository. The denial existed because Forgejo itself refuses non-admin repository creation (MAX_CREATION_LIMIT=0, see tests/forgejo-repo-creation-locked.sh in seanfarm) but nothing in this repo could reach an admin credential safely yet, and the compiled repository catalog had no room for a name that was not one of the ~43 pre-existing repos. This lands the domain and control-layer half of that capability: - ForgejoRepositoryBindings.legacy_names/1 now returns {:ok, []} for a well-formed repository name outside the static catalog (owner "sean", valid characters) instead of {:error, :unsupported_target}. An empty legacy-name list is what lets ServiceLegacyClient.legacy_state/2 short-circuit to "absent" with zero Kubernetes API calls, since a brand-new repository has no legacy CR to check for absence. A malformed name (wrong owner, invalid characters) is still rejected. - The three denial points are gone: ServiceCatalog no longer excludes "create" for repository targets (safe/no-op for the 43 pre-existing, already-present catalog repos, since ServiceManager's "matches" branch fires before provider.create is ever called); ServiceOperation.validate/2 no longer special-cases repository's supported_command; ServiceProvider.create/2 now has a real implementation for kind=repository, ownership=external. - Fixed a real bug found while doing this: service_repository_snapshot/2 routed through service_tag_get/2, which collapsed 404 into the same :denied bucket as 401/403. That made ServiceManager's absent-to-create branch dead code for every repository, even before today. Repository now has its own service_repository_get/2 that reports 404 as :not_found. - Client.service_repository_create/3 creates the repository with auto_init true, so it never reads back empty, which service_repository_snapshot/2 correctly still treats as :invalid_response for an established repository, via a new, distinct opts admin_authorization key kept separate from the ordinary opts authorization key every other write in this client uses, so a worker configured with only the ordinary writer credential fails closed with :denied here instead of silently reusing a token Forgejo would refuse. No release configures that credential yet (next commit). default_branch is deliberately not set: ServiceRepository's schema excludes it as an unverified ref (see ServiceRepositoryTest, "without identity or import effects"), so this always takes the instance's own configured default. Importing from an existing source is a separate, not-yet-built capability. The locking test's actual intent, that creation must be deliberate and auditable and not silently reachable, is preserved rather than weakened: it now asserts the exact bounded request sequence (GET, then POST to /admin/users/{owner}/repos, then the ordinary settings PATCH) happens only for an external, observed-absent repository, through the admin credential, and that a managed resource's drift-queued apply can never reach create at all. Claude-Session: https://claude.ai/code/session_01JGe4pyhQ36AhgDfPJry4d3Binjovi.Store.ServiceOperations.claim/2 (now claim/3) claims the single oldest due operation for whichever worker calls it, with no way to restrict which rows one worker can see. The isolated binjovi_repo_creator release (next commit) needs the opposite of what every other worker needs: it must claim ONLY forgejo/repository/create operations, and the main release must claim everything EXCEPT that one tuple, so the admin credential the repository-create path needs never reaches a worker that was not deliberately configured to hold it. claim/3 takes an optional opts list: only: [{provider, kind, command}, ...] restricts to exactly those tuples; exclude: [...] is the complement. The query now joins service_resources to test provider/kind alongside the operation's own command column. With no opts, behavior is byte-identical to before (same SQL shape, same ordering, same lease semantics) -- this is additive, not a rewrite of the claim path every operation already depends on. Added a repository fixture (a well-formed name outside ForgejoRepositoryBindings' static catalog) since testing this needed a second resource kind alongside the existing branch_protection fixture. Verified against a real throwaway PostgreSQL (tests/with-postgres.sh): the full suite including every postgres-tagged integration test still passes, 392 -> 395 in binjovi_store. Claude-Session: https://claude.ai/code/session_01JGe4pyhQ36AhgDfPJry4d3ForgejoPushMirrorBindings.upstream/1 and legacy_name/1 were closed over two static tables: the 2 mirrors Binjovi already owns, and the ~43 pre-existing catalog repositories (borrowed from ForgejoRepositoryBindings only to resolve upstream naming). A repository outside BOTH tables -- which, after the previous two commits, now includes any brand-new repository Binjovi itself creates -- resolved to :error/:unsupported_target in both, meaning a newly created repository could never get a backup mirror at all. - ForgejoRepositoryBindings gains known?/1 (is this one of the ~43 pre-existing catalog repos) and valid_name?/1 is now public, so ForgejoPushMirrorBindings can reuse the identical name-syntax check rather than duplicating it. - ForgejoPushMirrorBindings.upstream/1: a name outside both tables, if well-formed, now resolves to itself. A brand-new repository has no historical case-mismatch quirk to correct for (that class of quirk -- rpg-hello's mirror is a CR named ibmi-examples -- only exists for repos that predate Binjovi). - ForgejoPushMirrorBindings gains legacy_names/1, mirroring ForgejoRepositoryBindings' shape exactly: a declared mirror resolves its one legacy name; a well-formed name that is NEITHER a declared mirror NOR one of the ~43 catalog repos returns {:ok, []} (no legacy CR to check); anything else (malformed, or a catalog repo not yet migrated to Binjovi ownership) is still refused. legacy_name/1 now derives from legacy_names/1 instead of its own table lookup. - ServiceOperation.legacy_names/1 gets a push_mirror clause delegating to the above, the same shape as the repository and collaborator clauses already there -- without it, the generic wrapper around legacy_name/1 would produce {:ok, [nil]} for a new mirror and crash ServiceLegacyClient's string concatenation, the exact bug already fixed for repository. Updated the two existing tests that asserted the old "always :error/nil outside the declared tables" behavior to cover the three real cases instead: malformed name (still refused), a pre-existing catalog repo not yet declared as a mirror (still refused -- correct, most of those have a live CR), and a brand-new well-formed name (now resolves). Full suite including postgres-tagged integration tests still green (tests/with-postgres.sh), 345 -> 346 in binjovi_domain. Claude-Session: https://claude.ai/code/session_01JGe4pyhQ36AhgDfPJry4d3