Skip to content

switch to fiber/v3, add in project configuration#4

Merged
matthewpeterkort merged 46 commits into
mainfrom
feature/project-config
Jun 8, 2026
Merged

switch to fiber/v3, add in project configuration#4
matthewpeterkort merged 46 commits into
mainfrom
feature/project-config

Conversation

@matthewpeterkort

@matthewpeterkort matthewpeterkort commented May 19, 2026

Copy link
Copy Markdown
Contributor

PR Notes: feature/project-config

What This Branch Actually Does

This branch is not a small project-config patch. It changes Gecko from a relatively flat config and vector service into a multi-surface backend that now owns:

  • typed project configuration records
  • project-oriented config CRUD routes
  • Git-backed project repository state and read APIs
  • GitHub/Fence integration points for installation and token brokering
  • organization-level Git connect/reconcile flows
  • upload session persistence and PR-oriented file submission flows
  • thumbnail storage for projects
  • a full internal package reorganization
  • CI and runtime changes required by the new Git subsystem

The branch is large because it combines feature delivery with a substantial internal re-layout. The biggest review question is not any one handler. It is whether the new package boundaries, auth model, data model, and route surface still line up cleanly with the CALYPR frontend and deployment stack.

Scope by Area

git diff --dirstat against main shows the branch is concentrated in these areas:

  • internal/git/ ~10%
  • internal/server/http/git/ ~9%
  • internal/server/middleware/ ~8%
  • gecko/ legacy removal/replacement ~8%
  • internal/server/http/config/ ~7%
  • internal/db/ ~6%
  • config/ ~5%
  • internal/thumbnail/ ~5%
  • tests/integration/ ~4%

That matches the real shape of the work: this is mostly a Git/project backend branch, plus the refactor needed to support it.

Architecture Reorganization

Before

The old service logic lived mostly in the top-level gecko/ package with mixed concerns:

  • HTTP handlers
  • middleware
  • DB logic
  • vector logic
  • config logic
  • response helpers

After

The branch moves Gecko toward a clearer package layout:

  • config/
    • typed config models and validation
  • internal/db/
    • config and Git persistence
  • internal/git/
    • repository domain logic, reconciliation, setup, sync, upload workflows
  • internal/integrations/fence/
    • Fence broker client for GitHub App operations
  • internal/integrations/github/
    • GitHub API client wrapper built on go-github
  • internal/server/http/
    • route families split by surface:
      • config
      • directory
      • git
      • health
      • vector
      • shared
  • internal/server/middleware/
    • request auth, access checks, logging, resource helpers
  • internal/thumbnail/
    • thumbnail storage and validation
  • internal/httputil/
    • shared JSON/error response utilities
  • internal/logging/
    • service logging wrapper/helpers
  • internal/vectoradapter/
    • Qdrant request/response translation

This is the right direction structurally. The tradeoff is that the branch mixes architectural reorganization with product behavior changes, so code review needs to separate “moved” from “changed.”

Runtime / Bootstrap Changes

main.go now builds Gecko as a composable server with optional integrations:

  • PostgreSQL
  • Git service
  • thumbnail store
  • Qdrant
  • Grip
  • JWKS-backed JWT validation

The important runtime additions are:

  • --git-data-dir / GIT_DATA_DIR
  • --fence-base-url / FENCE_BASE_URL
  • --github-api-base-url / GITHUB_API_BASE_URL

Once DB connectivity is available, Gecko now also constructs:

  • git.NewGitService(...)
  • thumbnail.NewFilesystemStore(gitDataDir)

That makes gitDataDir a hard runtime dependency for the Git-enabled server path.

CI impact

Because Gecko now requires a Git data directory, CI had to be updated. The branch modifies:

  • .github/workflows/tests.yaml

to launch Gecko with:

-git-data-dir /tmp/gecko-git

Without that, the service exits before health checks come up.

Config Model Changes

Project config becomes first-class

This branch adds a typed ProjectConfig model in:

  • config/projectConfig.go

Supported fields:

  • title
  • contact_email
  • src_repo
  • org_title
  • description
  • project_title

Validation includes:

  • required-field checks
  • email validation
  • repository URL normalization into a GitHub-style host/owner/repo form

This is a real shift from using generic config blobs everywhere. Project metadata now has a stronger contract and normalization behavior.

Explorer config compatibility

This branch also carries forward explorer config compatibility work, including the richer fileActions shape:

{
  "extensions": {"ext": ["action"]},
  "actions": {"action": "/path"}
}

That matters because the frontend expects this richer form, and Gecko has to unmarshal it correctly for CALYPR configs to load.

Database / Persistence Changes

The database layer is no longer only about simple config_schema.<type> JSON tables.

Existing typed config tables

The init path now ensures config tables for:

  • explorer
  • nav
  • file_summary
  • project
  • projects

New Git-related state tables

internal/db/EnsureGitProjectStateTable expands the DB footprint significantly. This branch adds persistence for:

  1. config_schema.git_project_state

    • repository identity
    • installation metadata
    • mirror path
    • sync/default branch/error state
  2. config_schema.git_organization_state

    • organization installation/configuration status
    • target metadata
    • timestamps and last error
  3. config_schema.git_upload_session

    • upload/PR submission session metadata
  4. config_schema.git_upload_session_file

    • per-file status inside an upload session
  5. config_schema.git_pending_repository

    • repositories discovered but not yet reconciled into project config
    • supports both webhook-originated and user-scoped pending records
  6. config_schema.git_setup_session

    • setup snapshot used to compare repository sets before/after install/connect flows

This is one of the biggest branch changes. Gecko is no longer stateless around Git operations; it is now persisting lifecycle and reconciliation state explicitly.

HTTP Surface Changes

The route surface is much broader than before.

Top-level registration

The new entrypoint is:

  • internal/server/http/register.go

Registered route families:

  • /health
  • /Dir...
  • /config...
  • /git...
  • vector routes
  • swagger JSON route

Config routes

internal/server/http/config/register.go now exposes:

Generic config routes

  • GET /config/types
  • GET /config/list

Typed config groups:

  • /config/explorer
  • /config/nav
  • /config/file_summary
  • /config/project

Per-type operations include combinations of:

  • GET /list
  • GET /:configId
  • PUT /:configId
  • DELETE /:configId

Project config routes

This branch adds a dedicated project config surface:

  • GET /config/projects
  • GET /config/projects/list
  • GET /config/projects/summary
  • GET /config/projects/:orgTitle/:projectTitle
  • PUT /config/projects/:orgTitle/:projectTitle
  • DELETE /config/projects/:orgTitle/:projectTitle
  • DELETE /config/projects/:orgTitle

This is materially different from the old “config row by arbitrary key” model. The route shape now matches organization/project semantics directly.

Git routes

internal/server/http/git/register.go is one of the largest new route families in the service.

Organization-level Git routes

  • GET /git/projects
  • GET /git/organizations/status
  • POST /git/organizations/reconcile
  • POST /git/organizations/:orgTitle/init-connect
  • POST /git/organizations/:orgTitle/connect
  • GET /git/organizations/:orgTitle/status
  • POST /git/organizations/:orgTitle/reconcile

These routes cover installation status, connect flows, and organization-wide repository reconciliation.

Project-level Git read routes

  • GET /git/projects/:orgTitle/:projectTitle
  • GET /git/projects/:orgTitle/:projectTitle/refs
  • GET /git/projects/:orgTitle/:projectTitle/tree
  • GET /git/projects/:orgTitle/:projectTitle/tree/*
  • GET /git/projects/:orgTitle/:projectTitle/file/*
  • GET /git/projects/:orgTitle/:projectTitle/download/*
  • GET /git/projects/:orgTitle/:projectTitle/thumbnail

These expose Gecko as a repository-backed project read service, not just a config API.

Project-level Git write/workflow routes

  • PUT /git/projects/:orgTitle/:projectTitle/setup
  • PUT /git/projects/:orgTitle/:projectTitle/storage
  • PUT /git/projects/:orgTitle/:projectTitle/thumbnail
  • DELETE /git/projects/:orgTitle/:projectTitle/thumbnail
  • POST /git/projects/:orgTitle/:projectTitle/update
  • POST /git/projects/:orgTitle/:projectTitle/uploads/session
  • GET /git/projects/:orgTitle/:projectTitle/uploads/session/:sessionID
  • POST /git/projects/:orgTitle/:projectTitle/uploads/session/:sessionID/files
  • POST /git/projects/:orgTitle/:projectTitle/uploads/session/:sessionID/finalize

This is a major expansion of product surface. Gecko is now responsible for project repository setup, update, artifact flow staging, and PR-style finalization.

Auth and Access Model

The middleware layer has been significantly expanded.

Resource path model

The branch standardizes project authorization around Arborist-style paths:

/programs/{organization}/projects/{project}

Helper functions in internal/server/middleware/access.go normalize and check these resource paths.

Config auth model

ConfigAuth now treats explorer configs differently from base/global config routes:

  • explorer config access is project-scoped
  • non-explorer GET routes are broadly readable
  • non-explorer write/delete routes require route-specific authorization

Project config auth model

ProjectConfigAuth checks direct access on:

/programs/{org}/projects/{project}

and also allows certain broader admin-like resource paths such as:

  • *
  • /programs
  • /programs/{org}
  • /programs/{org}/projects

Git auth model

Git reads and organization reads are protected separately:

  • GitProjectAuth
  • GitOrganizationAuth

The important boundary in this branch is:

  • caller authorization still comes from the request Authorization token and Arborist/Fence checks
  • Git remote access is not the same thing as caller auth and is handled separately through the Git integration flow

That split is correct, but reviewers should confirm it stays consistent across all write routes.

Fence / GitHub Integration Model

This branch adds explicit integration clients instead of spreading ad hoc HTTP logic through handlers.

Fence integration

internal/integrations/fence/client.go turns Fence into a GitHub App broker that Gecko calls for:

  • install URL requests
  • organization installation status
  • repository installation status
  • installation repository listing
  • installation token minting

The request target is:

  • POST {FENCE_BASE_URL}/credentials/github

with action-based payloads.

This is architecturally important. Gecko is moving away from owning GitHub App secrets directly and toward asking Fence for short-lived GitHub access on demand.

GitHub integration

internal/integrations/github/client.go uses:

  • github.com/google/go-github/v87/github

for GitHub API metadata reads.

At minimum, it currently centralizes repository metadata lookup:

  • default branch
  • HTML URL

This is the right direction and avoids more hand-written GitHub REST client code.

Git Service and Repository Semantics

The branch adds a large internal/git/ package that now owns:

  • repository identity/domain types
  • setup and reconcile flows
  • repository state persistence coordination
  • update/sync operations
  • upload workflows
  • response shaping
  • error mapping

The key product shift is that Gecko is no longer just proxying config or metadata. It now maintains local repository state under a configured data directory and serves project Git views from there.

Reviewers should pay special attention to:

  • when local repository state is created
  • how update behaves when the local repo is missing vs already present
  • how default branch information is sourced and persisted
  • where Fence tokens are requested and how long they are retained in memory

Upload and Thumbnail Workflows

Two entirely new concerns land in this branch.

Upload sessions

Upload state is now explicit and persistent:

  • session creation
  • file list replacement/storage
  • session lookup
  • finalize flow
  • PR metadata persistence

This means Gecko now participates in a staged contribution workflow rather than just reading repository state.

Thumbnails

internal/thumbnail/ adds filesystem-backed thumbnail storage plus validation.

Route support includes:

  • GET /git/projects/:orgTitle/:projectTitle/thumbnail
  • PUT /git/projects/:orgTitle/:projectTitle/thumbnail
  • DELETE /git/projects/:orgTitle/:projectTitle/thumbnail

That is a durable product-surface change and should be reviewed as such, not as a minor helper addition.

Legacy Code Removal

A large part of the diff is deletion of the old flat handlers from gecko/, including legacy files such as:

  • handleConfig.go
  • handleDir.go
  • handleVector.go
  • middleware.go
  • response.go
  • server.go

This is not dead-code cleanup alone. These deletions are paired with replacements under internal/server/http/..., internal/server/middleware/..., and supporting packages.

Build / Tooling Changes

This branch also touches:

  • Dockerfile
  • Makefile
  • .dockerignore
  • go.mod
  • go.sum
  • swagger/docs artifacts

Those are not side noise. They are part of the fallout from:

  • root build compatibility
  • package reorganization
  • new GitHub client dependency
  • swagger generation drift

Review should include a sanity pass on container build assumptions and root go build . behavior.

Testing Changes

The branch adds or updates tests across multiple layers:

  • config tests
  • project config tests
  • middleware tests
  • Git service tests
  • upload tests
  • Fence integration tests
  • thumbnail tests
  • integration tests

Notable files include:

  • config/explorerConfig_test.go
  • config/projectConfig_test.go
  • internal/git/service_test.go
  • internal/git/upload_test.go
  • internal/integrations/fence/client_test.go
  • internal/server/middleware/git_test.go
  • internal/thumbnail/store_test.go
  • tests/integration/*

Given the branch size, the main review question is coverage shape rather than raw test count:

  • do route tests reflect current auth semantics?
  • do config tests lock frontend/backend schema compatibility?
  • do Git tests cover first-time setup, update, and persistence transitions?

Highest-Risk Areas

If reviewing this branch for merge readiness, focus here first:

  1. Auth correctness

    • project vs organization vs generic config auth
    • read vs write route consistency
  2. Route compatibility

    • does the route surface still match the frontend and revproxy expectations?
  3. DB/state lifecycle

    • are new tables initialized everywhere Gecko runs?
    • are deletes/updates cleaning up related Git state correctly?
  4. Git/Fence boundary

    • does Gecko request the right thing from Fence?
    • is Gecko still assuming too much GitHub App behavior locally?
  5. Config schema compatibility

    • especially explorer config and project config alignment with the frontend
  6. Startup behavior

    • new hard dependency on git-data-dir
    • optional integrations degrading cleanly when unset

Bottom Line

This branch should be read as a backend expansion and service re-platforming branch, not as a narrow project-config feature.

The durable outcomes are:

  • Gecko now has a first-class project config model.
  • Gecko now has a real Git-backed project API surface.
  • Gecko now persists Git lifecycle state instead of treating repo operations as transient.
  • Gecko now relies on Fence as the GitHub App broker boundary.
  • Gecko’s internal organization is materially better, but the branch is broad enough that compatibility review has to be disciplined.

@matthewpeterkort matthewpeterkort merged commit 85aef06 into main Jun 8, 2026
2 checks passed
@bwalsh

bwalsh commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

MERGE STATUS: ALREADY MERGED ✓

This PR is already merged (merged 21 hours ago). The review is post-facto,


Overview

106 files changed, +10,939 additions, -2,809 deletions across 46 commits

The PR combines three major initiatives:

  1. Fiber v3 migration (HTTP framework)
  2. Project configuration as a first-class concern (typed config model)
  3. Git-backed project API surface (repository management, uploads, reconciliation)

This is NOT a small feature patch. It's a fundamental restructuring of Gecko's architecture.


Assessment Against Your Focus Areas

1. Auth Correctness ⚠️ CRITICAL REVIEW AREAS

Status: Implemented but needs verification

Key changes:

  • New Arborist-style resource path model: /programs/{org}/projects/{project}
  • Distinct auth models:
    • ConfigAuth: Project-scoped for explorer configs; broad read access for others; write/delete requires route-specific auth
    • ProjectConfigAuth: Direct access check + fallback to broader paths (*, /programs, /programs/{org}, /programs/{org}/projects)
    • GitProjectAuth & GitOrganizationAuth: Separate authorization layers

Potential gaps:

  • Read vs write route consistency: Explorer vs. non-explorer config split is clear
  • ⚠️ Caller auth vs Git remote auth boundary: The split between request authorization (Arborist/Fence) and Git remote access (handled separately) must stay consistent across all write routes. Check:
    • /git/projects/:orgTitle/:projectTitle/update
    • /git/projects/:orgTitle/:projectTitle/uploads/session/:sessionID/finalize
    • /git/projects/:orgTitle/:projectTitle/setup

Files to review:

  • internal/server/middleware/access.go (normalize/check resource paths)
  • internal/server/http/config/register.go (config route auth dispatch)
  • internal/server/http/git/register.go (Git route auth dispatch)

2. Route Compatibility ⚠️ MAJOR SURFACE EXPANSION

Status: Significantly expanded; must verify frontend/revproxy expectations

New route surface is now:

Config routes (new structure):

GET /config/types              → list known types
GET /config/list               → list all configs (generic)
GET /config/projects           → new dedicated project endpoint
GET /config/projects/list      → project list
GET /config/projects/summary   → project summary
GET /config/projects/:orgTitle/:projectTitle
PUT /config/projects/:orgTitle/:projectTitle
DELETE /config/projects/:orgTitle/:projectTitle
DELETE /config/projects/:orgTitle  → cascade delete org projects

Git routes (entirely new):

Organization-level:
  GET /git/projects
  GET /git/organizations/status
  POST /git/organizations/reconcile
  POST /git/organizations/:orgTitle/init-connect
  POST /git/organizations/:orgTitle/connect
  GET /git/organizations/:orgTitle/status
  POST /git/organizations/:orgTitle/reconcile

Project-level (read):
  GET /git/projects/:orgTitle/:projectTitle
  GET /git/projects/:orgTitle/:projectTitle/refs
  GET /git/projects/:orgTitle/:projectTitle/tree
  GET /git/projects/:orgTitle/:projectTitle/tree/*
  GET /git/projects/:orgTitle/:projectTitle/file/*
  GET /git/projects/:orgTitle/:projectTitle/download/*
  GET /git/projects/:orgTitle/:projectTitle/thumbnail

Project-level (write/workflow):
  PUT /git/projects/:orgTitle/:projectTitle/setup
  PUT /git/projects/:orgTitle/:projectTitle/storage
  PUT /git/projects/:orgTitle/:projectTitle/thumbnail
  DELETE /git/projects/:orgTitle/:projectTitle/thumbnail
  POST /git/projects/:orgTitle/:projectTitle/update
  POST /git/projects/:orgTitle/:projectTitle/uploads/session
  GET /git/projects/:orgTitle/:projectTitle/uploads/session/:sessionID
  POST /git/projects/:orgTitle/:projectTitle/uploads/session/:sessionID/files
  POST /git/projects/:orgTitle/:projectTitle/uploads/session/:sessionID/finalize

ISSUE: No frontend/revproxy expectations documented in this review. Verify with frontend team that these routes match their expectations.


3. DB/State Lifecycle ⚠️ CRITICAL

Status: New persistent state model; must verify initialization everywhere

New tables created:

config_schema.git_project_state          → repository identity, install metadata, mirror path, sync state
config_schema.git_organization_state     → org installation/config status, target metadata, timestamps
config_schema.git_upload_session         → upload/PR submission session metadata
config_schema.git_upload_session_file    → per-file status inside upload session
config_schema.git_pending_repository     → repositories discovered but not yet reconciled
config_schema.git_setup_session          → setup snapshot for comparing repo sets before/after

Critical questions (may not be fully answered in code):

  • ✓ Initialization: internal/db/EnsureGitProjectStateTable() is called, but is it called in all deployment contexts (CI, helm, dev)?
    • See .github/workflows/tests.yaml: CI now passes -git-data-dir /tmp/gecko-git
    • But: Verify helm/production init logic
  • ✓ Delete/cascade: Does deleting a project/org cascade cleanup of:
    • corresponding git_project_state rows?
    • corresponding git_upload_session* rows?
    • Not explicitly shown in code review. Check internal/server/http/git/* delete handlers.
  • ⚠️ Git state orphaning: What if a git_project_state row exists but the local mirror directory is missing? Recovery logic?

4. Git/Fence Boundary ⚠️ CRITICAL DEPENDENCY

Status: Explicit integration model introduced; significant behavior change

New model:

  • Gecko no longer owns GitHub App secrets directly
  • Gecko calls Fence for:
    • Install URL requests → POST {FENCE_BASE_URL}/credentials/github with action-based payloads
    • Organization installation status
    • Repository installation status
    • Installation repository listing
    • Installation token minting (short-lived tokens on demand)

Critical dependencies:

  • FENCE_BASE_URL is now required at runtime (no fallback shown)
    • See main.go startup: Need to verify this is hard-failed if missing, not silently skipped
  • Fence token lifetime & caching: Where are tokens cached in memory? How long? Refresh strategy?
    • This is a behavioral change from previous local GitHub App handling
    • Risk: Token expiration during long-running operations (e.g., large git clone)

How does Gecko stay in sync with Fence?

  • ✓ Initial reconciliation: /git/organizations/:orgTitle/reconcile calls Fence to list repos
  • ✓ User-initiated: Setup/connect flows pull fresh state from Fence
  • ⚠️ Webhook-driven: Does Gecko receive webhooks when Fence detects new GitHub App installations? Not shown in code. If not:
    • Gecko may have stale git_organization_state until next manual reconcile
    • Users may be confused about why new repos don't appear

Critical files:

  • internal/integrations/fence/client.go (Fence broker)
  • main.go (where FENCE_BASE_URL is configured)

5. Config Schema Compatibility ⚠️ MIGRATION REQUIRED

Status: New typed config model; backward compat unclear

ProjectConfig model (new first-class concern):

type ProjectConfig struct {
  Title        string
  ContactEmail string
  SrcRepo      string
  OrgTitle     string
  Description  string
  ProjectTitle string
}

Validation:

  • ✓ Required-field checks (all fields required except SrcRepo during initialization)
  • ✓ Email validation (RFC 5322 via net/mail.ParseAddress)
  • ✓ Repository URL normalization into host/owner/repo form
    • Supports HTTPS, SSH, bare GitHub paths
    • Normalizes ssh.github.com & altssh.github.comgithub.com

Explorer config compatibility:

  • ✓ Tests confirm round-trip serialization (explorer config with fileActions)
  • Structure preserved for richer fileActions: {extensions: {ext: [action]}, actions: {action: "/path"}}

Question: How does existing project config in the database migrate to this new model?

  • Assuming old configs stored as generic JSON blobs?
  • No migration script shown. Is this breaking for existing deployments?

Files:

  • config/projectConfig.go (validation + normalization)
  • config/projectConfig_test.go (tests verify round-trip)

6. Startup Behavior ⚠️ HARD DEPENDENCIES

Status: New hard dependency introduced; optional integrations may not degrade cleanly

Hard dependency:

  • --git-data-dir / GIT_DATA_DIR is now required for the Git service path
    • Without it, Gecko exits before health checks (no graceful degradation)
    • Updated in CI: .github/workflows/tests.yaml now passes -git-data-dir /tmp/gecko-git

Optional integrations (graceful degradation):

  • --qdrant-* → Skips Qdrant endpoints if not provided ✓
  • --grip-* → Skips Grip endpoints if not provided ✓
  • --db → Skips DB endpoints if not provided (warning logged) ✓

Question: What if --git-data-dir points to a non-existent or non-writable directory?

  • Not clear from code. Check internal/git/service.go initialization.

Dockerfile & Container changes:

  • Updated to golang:1.26.3-alpine3.22 (was 1.24.2)
  • Multi-stage build now uses BuildKit syntax (syntax=docker/dockerfile:1.7)
  • Runs as non-root user gecko
  • Copies docs/swagger.json into image ✓

High-Risk Code Patterns

  1. Cascade delete logic: /config/projects/:orgTitle deletes all projects in an org. Verify related state (git_project_state, git_upload_session) is cleaned up.*

  2. Token lifecycle: Fence integration caches tokens in memory. What's the TTL? Refresh strategy on expiration?

  3. Repository mirror management: Local git mirrors stored under git-data-dir. What's the cleanup strategy? Orphaned mirrors? Disk space limits?

  4. Upload session finalization: Needs to atomically:

    • Commit staged files to git
    • Create PR via Fence/GitHub
    • Update session state
    • Verify transaction boundaries.

Suggestions for Post-Merge Validation

  1. E2E test: Walk through full project setup → file upload → PR creation workflow with Fence integration
  2. Migration plan: Document how existing project configs (if any) migrate to new typed model
  3. Fence contract: Sync with Fence team on token lifecycle, webhook notifications, rate limits
  4. Frontend handoff: Confirm new route surface matches frontend expectations (especially project-oriented paths)
  5. Operational readiness: Document git-data-dir sizing, backup strategy, recovery procedures

Summary

Merged successfully. The branch is architecturally sound but introduces significant behavioral changes:

  • Auth model is more explicit but requires consistency verification across all routes
  • Route surface is much broader (Git operations are now a first-class Gecko concern, not delegated)
  • State is no longer ephemeral; new persistence requirements across all deployments
  • Fence is now a hard dependency for Git operations; token lifecycle needs care
  • Startup has a new hard requirement (git-data-dir)

The code is well-organized and testable, but integration with frontend, Fence, and operational procedures needs explicit sign-off before this is production-ready.

@bwalsh

bwalsh commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Findings

--
 

1. API contract is incomplete for the new authorization mapping fields

 
The broker now accepts organization, project, and resource_path for installation_token, and accepts organization, project, and resource_path for repository_installation.
 
However, the OpenAPI schema for GitHubInstallationTokenRequest still only documents action, owner, repo, and access. The schema for GitHubRepositoryInstallationRequest still only documents action, owner, and repo.
 
This is a maintainability and client-compatibility issue: the main purpose of the PR appears to be decoupling GitHub org/repo identifiers from Calypr/Arborist org/project authorization, but two of the affected request shapes do not expose the fields clients need to use that feature.
 
Recommendation: Document the accepted organization, project, and, if truly intended as public API, resource_path fields for both request schemas. If resource_path is only an internal escape hatch, avoid exposing it to clients and prefer a narrower organization/project mapping.
 

 

2. Test coverage only exercises the new project override on repository_installation, not installation_token

 
The route applies the same project and resource_path authorization behavior to both installation_token and repository_installation. The new focused test verifies project is used for Arborist authz only through the repository_installation action.
 
The installation_token success tests still use the default owner/repo mapping and do not assert the Arborist resource path generated for organization, project, or resource_path.
 
Because installation_token is the sensitive path that actually mints a GitHub token, this is the most important path to cover.
 
Recommendation: Add tests for:
 

  • installation_token with organization + project, asserting Arborist checks /programs/<organization>/projects/<project>.
  • installation_token with resource_path, if that field remains supported.
  • Exact service, methods, and resources sent to Arborist, rather than using a blanket auth: True mock for the new mapping paths.
     

 

3. resource_path adds significant flexibility but also weakens simplicity and needs stronger guardrails

 
_authorize_repository now lets the request body supply an arbitrary resource_path, and when present it bypasses the constructed /programs/<org>/projects/<project> path entirely.
 
That may be intentional for non-standard Arborist resource layouts, but it makes the authorization contract harder to reason about because the client can choose the exact Arborist resource that gates a GitHub token for owner/repo. There is currently no validation in this layer that the supplied resource path is related to the requested GitHub owner and repo.
 
Recommendation: For simplicity and maintainability, prefer the narrower organization + project override unless arbitrary Arborist resources are required. If resource_path is required, document the expected trust model and add explicit tests around it.
 

 

4. There is a redundant decorator in the GitHub credential tests

 
tests/credentials/github/test_credentials.py has two @responses.activate decorators separated by blank lines immediately before test_github_organization_installation_success.
 
This does not appear to break parsing, but it is noisy and confusing.
 
Recommendation: Remove the stray duplicate decorator to keep the test file simple.
 

 

Positive Notes

 

  • The route logic keeps the default behavior backward-compatible by falling back from organization to owner and from project to repo.
  • The install_url and organization_installation paths now clearly authorize against the Calypr organization while still calling GitHub with the GitHub owner/login.
  • The OpenAPI updates for install_url and organization_installation are clear and describe the GitHub owner versus Calypr organization split.
     

@matthewpeterkort

matthewpeterkort commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

feedback addressed in new branch. See #5

@bwalsh

bwalsh commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Bottom line

Gecko and git-drs now overlap in the creation of Git LFS pointer files, but they use GitHub very differently.

git-drs is a local developer/worktree tool: it writes a pointer file into the local checkout, ensures LFS tracking, stores local DRS metadata, and expects later git drs push/Git workflows to register or promote metadata.

Gecko is a server-side GitHub App workflow: it stages upload metadata in Postgres, requests GitHub App installation tokens through Fence, then creates GitHub trees, commits, branches, and PRs directly through the GitHub API.

Direct duplication

Both generate this same LFS pointer body:

version https://git-lfs.github.com/spec/v1
oid sha256:<oid>
size <size>

git-drs writes it to the local worktree via writePointerFile.
Gecko builds the same content in BuildLFSPointerContent and inserts it as GitHub tree blob content.

Key architectural difference

git-drs add-url also creates local DRS metadata after writing the pointer; if SHA256 is absent, it derives a deterministic placeholder OID from ETag and source URL.

Gecko stores checksum and drs_object_id in upload session state, but the committed pointer only contains SHA256 and size.

Areas of conflict / risk

  1. Two sources of truth for pointer creation
    If pointer format, checksum normalization, or validation changes, Gecko and git-drs can drift.

  2. DRS identity is not in the LFS pointer
    git-drs couples the pointer with local DRS metadata. Gecko commits only pointer content and stores DRS object IDs in Gecko DB state. That means the repository alone may not carry enough DRS identity unless another process reconciles it.

  3. Different commit paths
    git-drs expects local Git add/commit/push behavior. Gecko bypasses local Git and creates GitHub trees/commits/PRs directly through the API.

  4. Different credential/audit models
    git-drs is user-local. Gecko uses GitHub App installation tokens brokered by Fence. The resulting commits/PRs may have different authorship and audit semantics.

  5. Potential bypass of git drs push semantics
    git-drs push registers metadata and can upload/register DRS objects. Gecko’s PR flow creates pointer commits but does not, by itself, appear to run the git-drs push registration lifecycle.

Recommendation

Create a shared small library/package for:

  • LFS pointer rendering/parsing
  • SHA256 validation/normalization
  • DRS-object-to-pointer contract
  • test vectors used by both Gecko and git-drs

Then make a clear product decision:

Either Gecko is only a PR/pointer author and git-drs owns DRS registration, or Gecko must fully implement the same DRS registration contract as git-drs push.

@bwalsh

bwalsh commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

1. Ask Fence for a write-scoped installation token

Call site

File: internal/git/upload.go
Lines: 163-166

accessToken, err := service.RequestInstallationToken(
    ctx,
    authorizationHeader,
    organization,
    project,
    identity,
    "write",
)

Source: internal/git/upload.go:163-166

Git service wrapper

File: internal/git/service.go
Lines: 212-216

func (service *GitService) RequestInstallationToken(
    ctx context.Context,
    authorizationHeader string,
    organization string,
    project string,
    identity GitRepositoryIdentity,
    access string,
) (string, error) {
    return service.fenceAPI.RequestInstallationToken(
        ctx,
        authorizationHeader,
        organization,
        project,
        identity,
        access,
    )
}

Source: internal/git/service.go:212-216

Fence broker request

File: internal/integrations/fence/client.go
Lines: 202-218

if err := c.requestFenceGitHubBroker(
    ctx,
    authorizationHeader,
    map[string]any{
        "action":       "installation_token",
        "owner":        identity.Owner,
        "repo":         identity.Repo,
        "organization": organization,
        "project":      strings.TrimSpace(project),
        "access":       requestedAccess,
    },
    &payload,
); err != nil {

Source: internal/integrations/fence/client.go:202-218

HTTP call to Fence

File: internal/integrations/fence/client.go
Lines: 75-85

req, err := http.NewRequestWithContext(
    ctx,
    http.MethodPost,
    strings.TrimRight(c.config.BaseURL, "/")+"/credentials/github",
    bytes.NewReader(requestBody),
)

Source: internal/integrations/fence/client.go:75-85


2. Create GitHub client

File: internal/git/upload.go
Lines: 167-169

client, err := service.githubClient(accessToken)

Source: internal/git/upload.go:167-169

Client construction

File: internal/git/service.go
Lines: 20-33

func (service *GitService) githubClient(accessToken string) (*github.Client, error) {
    options := []github.ClientOptionsFunc{
        github.WithAuthToken(accessToken),
        github.WithHTTPClient(service.client),
    }

Source: internal/git/service.go:20-33


3. Create Git LFS pointer files

LFS pointer generation function

File: internal/git/upload.go
Lines: 53-55

func BuildLFSPointerContent(checksum string, size int64) string {
    return fmt.Sprintf(
        "version https://git-lfs.github.com/spec/v1\noid sha256:%s\nsize %d\n",
        strings.ToLower(strings.TrimSpace(checksum)),
        size,
    )
}

Source: internal/git/upload.go:53-55

Pointer file creation during upload finalization

File: internal/git/upload.go
Lines: 180-190

entries = append(entries, &github.TreeEntry{
    Path:    github.Ptr(file.TargetPath),
    Mode:    github.Ptr("100644"),
    Type:    github.Ptr("blob"),
    Content: github.Ptr(
        BuildLFSPointerContent(
            file.Checksum.String,
            file.Size,
        ),
    ),
})

Source: internal/git/upload.go:180-190

Important observation

Gecko does not write pointer files into a local Git worktree.

Instead, it creates the LFS pointer text in memory and sends it directly to GitHub as blob content inside a Git tree.


4. Create GitHub tree

Build tree entries

File: internal/git/upload.go
Lines: 180-190

Source: internal/git/upload.go:180-190

Create tree API call

File: internal/git/upload.go
Lines: 192-195

tree, response, err := client.Git.CreateTree(
    ctx,
    identity.Owner,
    identity.Repo,
    baseCommit.GetTree().GetSHA(),
    entries,
)

Source: internal/git/upload.go:192-195

GitHub REST equivalent

POST /repos/{owner}/{repo}/git/trees

5. Create commit

Load base commit

File: internal/git/upload.go
Lines: 175-178

baseCommit, response, err := client.Git.GetCommit(
    ctx,
    identity.Owner,
    identity.Repo,
    baseCommitSHA,
)

Source: internal/git/upload.go:175-178

Create commit API call

File: internal/git/upload.go
Lines: 196-200

commit, response, err := client.Git.CreateCommit(
    ctx,
    identity.Owner,
    identity.Repo,
    github.Commit{
        Message: github.Ptr(title),
        Tree:    &github.Tree{SHA: github.Ptr(tree.GetSHA())},
        Parents: []*github.Commit{
            {SHA: github.Ptr(baseCommitSHA)},
        },
    },
    nil,
)

Source: internal/git/upload.go:196-200

GitHub REST equivalent

POST /repos/{owner}/{repo}/git/commits

6. Create branch

File: internal/git/upload.go
Lines: 204-207

_, response, err = client.Git.CreateRef(
    ctx,
    identity.Owner,
    identity.Repo,
    github.CreateRef{
        Ref: "refs/heads/" + branchName,
        SHA: commit.GetSHA(),
    },
)

Source: internal/git/upload.go:204-207

GitHub REST equivalent

POST /repos/{owner}/{repo}/git/refs

7. Open pull request

File: internal/git/upload.go
Lines: 211-216

pr, response, err := client.PullRequests.Create(
    ctx,
    identity.Owner,
    identity.Repo,
    &github.NewPullRequest{
        Title: github.Ptr(title),
        Body:  github.Ptr(body),
        Base:  github.Ptr(baseBranch),
        Head:  github.Ptr(branchName),
    },
)

Source: internal/git/upload.go:211-216

GitHub REST equivalent

POST /repos/{owner}/{repo}/pulls

Complete call graph

HTTP POST /git/upload/{session}/finalize
    │
    ▼
internal/server/http/git/upload.go

    ▼

GitService.CreateGitHubUploadPullRequest()
internal/git/upload.go:151-220

    ├── RequestInstallationToken()
    │      internal/git/service.go:212-216
    │
    ├── Fence POST /credentials/github
    │      internal/integrations/fence/client.go:202-218
    │
    ├── BuildLFSPointerContent()
    │      internal/git/upload.go:53-55
    │
    ├── Git.GetRef()
    │      internal/git/upload.go:171-174
    │
    ├── Git.GetCommit()
    │      internal/git/upload.go:175-178
    │
    ├── Git.CreateTree()
    │      internal/git/upload.go:192-195
    │
    ├── Git.CreateCommit()
    │      internal/git/upload.go:196-200
    │
    ├── Git.CreateRef()
    │      internal/git/upload.go:204-207
    │
    └── PullRequests.Create()
           internal/git/upload.go:211-216

This is the complete server-side GitHub App workflow introduced in PR #4. Unlike git-drs, Gecko never performs a local git add, git commit, or git push; it constructs Git objects directly through the GitHub Git Data API and then opens a pull request.

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.

2 participants