diff --git a/.ccs-fork-upstream.env b/.ccs-fork-upstream.env index 780a966f..85b3c9b2 100644 --- a/.ccs-fork-upstream.env +++ b/.ccs-fork-upstream.env @@ -1,2 +1,2 @@ -UPSTREAM_TAG=v7.2.105 -UPSTREAM_COMMIT=4a2eb54dc6bf943196be4fb515e6a9407a4db143 +UPSTREAM_TAG=v7.2.109 +UPSTREAM_COMMIT=928478e4b91533cec05a763bfac3edad9c3e76cf diff --git a/internal/api/handlers/management/auth_files.go b/internal/api/handlers/management/auth_files.go index 80801d7e..04927d61 100644 --- a/internal/api/handlers/management/auth_files.go +++ b/internal/api/handlers/management/auth_files.go @@ -15,6 +15,7 @@ import ( "github.com/gin-gonic/gin" "github.com/router-for-me/CLIProxyAPI/v7/internal/auth/codex" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + "github.com/router-for-me/CLIProxyAPI/v7/internal/credentialweight" "github.com/router-for-me/CLIProxyAPI/v7/internal/registry" coreauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth" log "github.com/sirupsen/logrus" @@ -260,6 +261,20 @@ func (h *Handler) listAuthFilesFromDisk(c *gin.Context) { } } } + if wv := gjson.GetBytes(data, coreauth.AttributeWeight); wv.Exists() { + var rawWeight string + switch wv.Type { + case gjson.Number: + rawWeight = wv.Raw + case gjson.String: + rawWeight = wv.String() + } + if rawWeight != "" { + if weight, errWeight := credentialweight.ParseString(rawWeight); errWeight == nil { + fileData[coreauth.AttributeWeight] = weight + } + } + } if nv := gjson.GetBytes(data, "note"); nv.Exists() && nv.Type == gjson.String { if trimmed := strings.TrimSpace(nv.String()); trimmed != "" { fileData["note"] = trimmed @@ -403,12 +418,34 @@ func (h *Handler) buildAuthFileEntryLocked(auth *coreauth.Auth) gin.H { } } } + if weight, ok := authWeightValue(auth); ok { + entry[coreauth.AttributeWeight] = weight + } if websockets, ok := authWebsocketsValue(auth); ok { entry["websockets"] = websockets } return entry } +func authWeightValue(auth *coreauth.Auth) (int64, bool) { + if auth == nil { + return 0, false + } + if rawWeight := strings.TrimSpace(authAttribute(auth, coreauth.AttributeWeight)); rawWeight != "" { + weight, errWeight := credentialweight.ParseString(rawWeight) + return weight, errWeight == nil + } + if auth.Metadata == nil { + return 0, false + } + rawWeight, ok := auth.Metadata[coreauth.AttributeWeight] + if !ok || rawWeight == nil { + return 0, false + } + weight, errWeight := credentialweight.ParseValue(rawWeight) + return weight, errWeight == nil +} + func authWebsocketsValue(auth *coreauth.Auth) (bool, bool) { if auth == nil { return false, false diff --git a/internal/runtime/executor/antigravity_schema_sanitize_test.go b/internal/runtime/executor/antigravity_schema_sanitize_test.go index 6151ae4c..ff8c0c21 100644 --- a/internal/runtime/executor/antigravity_schema_sanitize_test.go +++ b/internal/runtime/executor/antigravity_schema_sanitize_test.go @@ -274,7 +274,47 @@ func TestSanitizeAntigravityRequestSchemasKeepsResponseSchemasPlaceholderFree(t } } -func TestAntigravityBuildRequestKeepsJSONObjectSchemaPlaceholderFree(t *testing.T) { +func TestSanitizeAntigravityRequestSchemasPreservesResponseUnionAndEnumType(t *testing.T) { + payload := `{"request":{ + "tools":[{"functionDeclarations":[{"name":"tool","parameters":{"type":"object","properties":{ + "choice":{"anyOf":[{"type":"string"},{"type":"null"}]}, + "level":{"type":"number","enum":[1,2]} + }}}]}], + "generationConfig":{"responseSchema":{"type":"object","properties":{ + "action":{"anyOf":[ + {"type":"object","properties":{"name":{"type":"string"}},"required":["name"]}, + {"type":"null"} + ]}, + "conviction":{"type":"number","enum":[0.25,0.5,1]} + }}} + }}` + + got := sanitizeAntigravityRequestSchemas(payload, true) + responseSchema := gjson.Get(got, "request.generationConfig.responseSchema") + union := responseSchema.Get("properties.action.anyOf") + if !union.IsArray() || len(union.Array()) != 2 || union.Get("1.type").String() != "null" { + t.Errorf("response anyOf union was flattened: %s", responseSchema.Raw) + } + conviction := responseSchema.Get("properties.conviction") + if gotType := conviction.Get("type").String(); gotType != "number" { + t.Errorf("response enum type = %q, want number: %s", gotType, responseSchema.Raw) + } + for _, enumValue := range conviction.Get("enum").Array() { + if enumValue.Type != gjson.String { + t.Errorf("response enum value is not a string: %s", conviction.Raw) + } + } + + toolSchema := gjson.Get(got, "request.tools.0.functionDeclarations.0.parameters") + if toolSchema.Get("properties.choice.anyOf").Exists() { + t.Errorf("tool anyOf union was not flattened: %s", toolSchema.Raw) + } + if gotType := toolSchema.Get("properties.level.type").String(); gotType != "string" { + t.Errorf("tool enum type = %q, want string: %s", gotType, toolSchema.Raw) + } +} + +func TestAntigravityBuildRequestKeepsJSONObjectMimeOnly(t *testing.T) { input := []byte(`{"model":"gemini-3.1-pro-low","messages":[{"role":"user","content":"hi"}],"response_format":{"type":"json_object"}}`) translated := antigravitychat.ConvertOpenAIRequestToAntigravity("gemini-3.1-pro-low", input, false) body := buildRequestBodyFromRawPayload(t, "gemini-3.1-pro-low", translated) @@ -283,12 +323,12 @@ func TestAntigravityBuildRequestKeepsJSONObjectSchemaPlaceholderFree(t *testing. t.Fatal(errMarshal) } - schema := gjson.GetBytes(encoded, "request.generationConfig.responseSchema") - if got := schema.Get("type").String(); got != "object" { - t.Fatalf("responseSchema.type = %q, want object: %s", got, encoded) + generationConfig := gjson.GetBytes(encoded, "request.generationConfig") + if got := generationConfig.Get("responseMimeType").String(); got != "application/json" { + t.Fatalf("responseMimeType = %q, want application/json: %s", got, encoded) } - if schema.Get("properties.reason").Exists() || schema.Get("required").Exists() { - t.Fatalf("json_object schema gained tool placeholders: %s", schema.Raw) + if generationConfig.Get("responseSchema").Exists() { + t.Fatalf("responseSchema should not be set for json_object: %s", encoded) } } diff --git a/internal/store/gitstore.go b/internal/store/gitstore.go index 222130a6..7d01363d 100644 --- a/internal/store/gitstore.go +++ b/internal/store/gitstore.go @@ -8,6 +8,7 @@ import ( "io/fs" "os" "path/filepath" + "sort" "strings" "sync" "time" @@ -20,11 +21,16 @@ import ( "github.com/go-git/go-git/v6/plumbing/object" "github.com/go-git/go-git/v6/plumbing/transport" "github.com/go-git/go-git/v6/plumbing/transport/http" + "github.com/go-git/go-git/v6/storage/filesystem/dotgit" cliproxyauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth" ) -// gcInterval defines minimum time between garbage collection runs. -const gcInterval = 5 * time.Minute +const ( + // gcInterval defines minimum time between garbage collection runs. + gcInterval = 5 * time.Minute + // gcPruneGracePeriod keeps recently orphaned objects available for recovery. + gcPruneGracePeriod = 24 * time.Hour +) // GitTokenStore persists token records and auth metadata using git as the backing storage. type GitTokenStore struct { @@ -59,6 +65,9 @@ func NewGitTokenStore(remote, username, password, branch string) *GitTokenStore // SetBaseDir updates the default directory used for auth JSON persistence when no explicit path is provided. func (s *GitTokenStore) SetBaseDir(dir string) { + s.mu.Lock() + defer s.mu.Unlock() + clean := strings.TrimSpace(dir) if clean == "" { s.dirLock.Lock() @@ -100,6 +109,12 @@ func (s *GitTokenStore) ConfigPath() string { // EnsureRepository prepares the local git working tree by cloning or opening the repository. func (s *GitTokenStore) EnsureRepository() error { + s.mu.Lock() + defer s.mu.Unlock() + return s.ensureRepositoryLocked() +} + +func (s *GitTokenStore) ensureRepositoryLocked() error { s.dirLock.Lock() if s.remote == "" { s.dirLock.Unlock() @@ -197,6 +212,26 @@ func (s *GitTokenStore) EnsureRepository() error { s.dirLock.Unlock() return fmt.Errorf("git token store: worktree: %w", errWorktree) } + if errVerify := verifyRepositoryHead(repo); errVerify != nil { + if !isRepositoryCorruptionError(errVerify) { + s.dirLock.Unlock() + return fmt.Errorf("git token store: verify repository before pull: %w", errVerify) + } + if errRecover := s.recoverRepositoryLocked(repoDir, authMethod, nil, nil); errRecover != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: verify repository before pull: %w; recovery failed: %v", errVerify, errRecover) + } + repo, errOpen = git.PlainOpen(repoDir) + if errOpen != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: open recovered repo: %w", errOpen) + } + worktree, errWorktree = repo.Worktree() + if errWorktree != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: recovered worktree: %w", errWorktree) + } + } if s.branch != "" { if errCheckout := s.checkoutConfiguredBranch(repo, worktree, authMethod); errCheckout != nil { s.dirLock.Unlock() @@ -215,12 +250,60 @@ func (s *GitTokenStore) EnsureRepository() error { if s.branch != "" { pullOpts.ReferenceName = plumbing.NewBranchReferenceName(s.branch) } + prePullHead, errPrePullHead := repo.Head() + if errPrePullHead != nil && !errors.Is(errPrePullHead, plumbing.ErrReferenceNotFound) { + s.dirLock.Unlock() + return fmt.Errorf("git token store: get head before pull: %w", errPrePullHead) + } + var prePullTree *object.Tree + if prePullHead != nil { + prePullCommit, errPrePullCommit := repo.CommitObject(prePullHead.Hash()) + if errPrePullCommit != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: inspect head before pull: %w", errPrePullCommit) + } + prePullTree, errPrePullCommit = prePullCommit.Tree() + if errPrePullCommit != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: inspect tree before pull: %w", errPrePullCommit) + } + } + dirtyPaths, errDirtyPaths := worktreeDirtyPaths(worktree) + if errDirtyPaths != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: inspect worktree before pull: %w", errDirtyPaths) + } + repositoryRecovered := false if errPull := worktree.Pull(pullOpts); errPull != nil { switch { - case errors.Is(errPull, git.NoErrAlreadyUpToDate), - errors.Is(errPull, git.ErrUnstagedChanges), - errors.Is(errPull, git.ErrNonFastForwardUpdate): - // Ignore clean syncs, local edits, and remote divergence—local changes win. + case errors.Is(errPull, git.NoErrAlreadyUpToDate): + if errReset := resetIndexToHead(repo, worktree); errReset != nil { + if !isRepositoryCorruptionError(errReset) { + s.dirLock.Unlock() + return fmt.Errorf("git token store: repair index after up-to-date pull: %w", errReset) + } + if errRecover := s.recoverRepositoryLocked(repoDir, authMethod, prePullTree, dirtyPaths); errRecover != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: repair index after up-to-date pull: %w; recovery failed: %v", errReset, errRecover) + } + repositoryRecovered = true + } + case errors.Is(errPull, git.ErrUnstagedChanges), errors.Is(errPull, git.ErrNonFastForwardUpdate): + if prePullHead == nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: reconcile pull without a local branch") + } + if errReconcile := reconcileRemoteWorktree(repo, worktree, repoDir, prePullHead, dirtyPaths); errReconcile != nil { + if !isRepositoryCorruptionError(errReconcile) { + s.dirLock.Unlock() + return fmt.Errorf("git token store: reconcile remote changes: %w", errReconcile) + } + if errRecover := s.recoverRepositoryLocked(repoDir, authMethod, prePullTree, dirtyPaths); errRecover != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: reconcile remote changes: %w; recovery failed: %v", errReconcile, errRecover) + } + repositoryRecovered = true + } case errors.Is(errPull, transport.ErrAuthenticationRequired), errors.Is(errPull, transport.ErrEmptyRemoteRepository): // Ignore authentication prompts and empty remote references on initial sync. @@ -230,11 +313,36 @@ func (s *GitTokenStore) EnsureRepository() error { return fmt.Errorf("git token store: pull: %w", errPull) } // Ignore missing references only when following the remote default branch. + case isRepositoryCorruptionError(errPull): + if errRecover := s.recoverRepositoryLocked(repoDir, authMethod, prePullTree, dirtyPaths); errRecover != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: pull: %w; recovery failed: %v", errPull, errRecover) + } + repositoryRecovered = true default: s.dirLock.Unlock() return fmt.Errorf("git token store: pull: %w", errPull) } } + if !repositoryRecovered { + if errVerify := verifyRepositoryHead(repo); errVerify != nil { + if !isRepositoryCorruptionError(errVerify) { + s.dirLock.Unlock() + return fmt.Errorf("git token store: verify repository after pull: %w", errVerify) + } + if errRecover := s.recoverRepositoryLocked(repoDir, authMethod, prePullTree, dirtyPaths); errRecover != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: verify repository after pull: %w; recovery failed: %v", errVerify, errRecover) + } + repositoryRecovered = true + } + } + if !repositoryRecovered { + if errRestore := restoreMissingTrackedFiles(repo, repoDir); errRestore != nil { + s.dirLock.Unlock() + return fmt.Errorf("git token store: restore tracked worktree files: %w", errRestore) + } + } } if err := disableGitCommitSigning(repoDir); err != nil { s.dirLock.Unlock() @@ -250,11 +358,8 @@ func (s *GitTokenStore) EnsureRepository() error { } s.dirLock.Unlock() if len(initPaths) > 0 { - s.mu.Lock() - err := s.commitAndPushLocked("Initialize git token store", initPaths...) - s.mu.Unlock() - if err != nil { - return err + if errCommit := s.commitAndPushInitialLocked("Initialize git token store", initPaths...); errCommit != nil { + return errCommit } } return nil @@ -269,6 +374,9 @@ func (s *GitTokenStore) Save(_ context.Context, auth *cliproxyauth.Auth) (string return "", fmt.Errorf("auth filestore: %w", errWeight) } + s.mu.Lock() + defer s.mu.Unlock() + path, err := s.resolveAuthPath(auth) if err != nil { return "", err @@ -283,13 +391,13 @@ func (s *GitTokenStore) Save(_ context.Context, auth *cliproxyauth.Auth) (string } } - if err = s.EnsureRepository(); err != nil { + if err = s.ensureRepositoryLocked(); err != nil { return "", err } - - s.mu.Lock() - defer s.mu.Unlock() - + relPath, errRel := s.relativeToRepo(path) + if errRel != nil { + return "", errRel + } if err = os.MkdirAll(filepath.Dir(path), 0o700); err != nil { return "", fmt.Errorf("auth filestore: create dir failed: %w", err) } @@ -312,19 +420,20 @@ func (s *GitTokenStore) Save(_ context.Context, auth *cliproxyauth.Auth) (string if errMarshal != nil { return "", fmt.Errorf("auth filestore: marshal metadata failed: %w", errMarshal) } + contentsMatch := false if existing, errRead := os.ReadFile(path); errRead == nil { - if jsonEqual(existing, raw) { - return path, nil - } + contentsMatch = jsonEqual(existing, raw) } else if !os.IsNotExist(errRead) { return "", fmt.Errorf("auth filestore: read existing failed: %w", errRead) } - tmp := path + ".tmp" - if errWrite := os.WriteFile(tmp, raw, 0o600); errWrite != nil { - return "", fmt.Errorf("auth filestore: write temp failed: %w", errWrite) - } - if errRename := os.Rename(tmp, path); errRename != nil { - return "", fmt.Errorf("auth filestore: rename failed: %w", errRename) + if !contentsMatch { + tmp := path + ".tmp" + if errWrite := os.WriteFile(tmp, raw, 0o600); errWrite != nil { + return "", fmt.Errorf("auth filestore: write temp failed: %w", errWrite) + } + if errRename := os.Rename(tmp, path); errRename != nil { + return "", fmt.Errorf("auth filestore: rename failed: %w", errRename) + } } default: return "", fmt.Errorf("auth filestore: nothing to persist for %s", auth.ID) @@ -340,10 +449,6 @@ func (s *GitTokenStore) Save(_ context.Context, auth *cliproxyauth.Auth) (string auth.FileName = auth.ID } - relPath, errRel := s.relativeToRepo(path) - if errRel != nil { - return "", errRel - } messageID := auth.ID if strings.TrimSpace(messageID) == "" { messageID = filepath.Base(path) @@ -357,7 +462,10 @@ func (s *GitTokenStore) Save(_ context.Context, auth *cliproxyauth.Auth) (string // List enumerates all auth JSON files under the configured directory. func (s *GitTokenStore) List(_ context.Context) ([]*cliproxyauth.Auth, error) { - if err := s.EnsureRepository(); err != nil { + s.mu.Lock() + defer s.mu.Unlock() + + if err := s.ensureRepositoryLocked(); err != nil { return nil, err } dir := s.baseDirSnapshot() @@ -396,24 +504,24 @@ func (s *GitTokenStore) Delete(_ context.Context, id string) error { if id == "" { return fmt.Errorf("auth filestore: id is empty") } + + s.mu.Lock() + defer s.mu.Unlock() + path, err := s.resolveDeletePath(id) if err != nil { return err } - if err = s.EnsureRepository(); err != nil { + if err = s.ensureRepositoryLocked(); err != nil { return err } - - s.mu.Lock() - defer s.mu.Unlock() - - if err = os.Remove(path); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("auth filestore: delete failed: %w", err) - } rel, errRel := s.relativeToRepo(path) if errRel != nil { return errRel } + if err = os.Remove(path); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("auth filestore: delete failed: %w", err) + } messageID := id if errCommit := s.commitAndPushLocked(fmt.Sprintf("Delete auth %s", messageID), rel); errCommit != nil { return errCommit @@ -427,9 +535,9 @@ func (s *GitTokenStore) PersistAuthFiles(_ context.Context, message string, path if len(paths) == 0 { return nil } - if err := s.EnsureRepository(); err != nil { - return err - } + + s.mu.Lock() + defer s.mu.Unlock() filtered := make([]string, 0, len(paths)) for _, p := range paths { @@ -446,13 +554,22 @@ func (s *GitTokenStore) PersistAuthFiles(_ context.Context, message string, path if len(filtered) == 0 { return nil } - - s.mu.Lock() - defer s.mu.Unlock() - if strings.TrimSpace(message) == "" { message = "Sync watcher updates" } + + // Inspect watcher removals before EnsureRepository restores missing tracked + // files so an unexpected filesystem event remains distinguishable from Delete. + if _, errStat := os.Stat(filepath.Join(s.repoDirSnapshot(), ".git")); errStat == nil { + if handled, errGuard := s.guardWatcherAuthRemovalLocked(message, filtered); handled || errGuard != nil { + return errGuard + } + } else if !errors.Is(errStat, fs.ErrNotExist) { + return fmt.Errorf("git token store: stat repository before watcher removal guard: %w", errStat) + } + if err := s.ensureRepositoryLocked(); err != nil { + return err + } if handled, errGuard := s.guardWatcherAuthRemovalLocked(message, filtered); handled || errGuard != nil { return errGuard } @@ -676,17 +793,17 @@ func (s *GitTokenStore) relativeToRepo(path string) (string, error) { if repoDir == "" { return "", fmt.Errorf("git token store: repository path not configured") } - absRepo := repoDir - if abs, err := filepath.Abs(repoDir); err == nil { - absRepo = abs + absRepo, errRepo := filepath.Abs(repoDir) + if errRepo != nil { + return "", fmt.Errorf("git token store: resolve repository path: %w", errRepo) } - cleanPath := path - if abs, err := filepath.Abs(path); err == nil { - cleanPath = abs + absPath, errPath := filepath.Abs(path) + if errPath != nil { + return "", fmt.Errorf("git token store: resolve path: %w", errPath) } - rel, err := filepath.Rel(absRepo, cleanPath) - if err != nil { - return "", fmt.Errorf("git token store: relative path: %w", err) + rel, errRel := filepath.Rel(absRepo, absPath) + if errRel != nil { + return "", fmt.Errorf("git token store: relative path: %w", errRel) } if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { return "", fmt.Errorf("git token store: path outside repository") @@ -821,6 +938,517 @@ func normalizeRemoteBranchReference(name plumbing.ReferenceName) (plumbing.Refer } } +func resetIndexToHead(repo *git.Repository, worktree *git.Worktree) error { + if repo == nil || worktree == nil { + return fmt.Errorf("repository or worktree is nil") + } + head, errHead := repo.Head() + if errHead != nil { + if errors.Is(errHead, plumbing.ErrReferenceNotFound) { + return nil + } + return errHead + } + return worktree.Reset(&git.ResetOptions{Mode: git.MixedReset, Commit: head.Hash()}) +} + +func worktreeDirtyPaths(worktree *git.Worktree) (map[string]struct{}, error) { + if worktree == nil { + return nil, fmt.Errorf("worktree is nil") + } + status, errStatus := worktree.Status() + if errStatus != nil { + return nil, errStatus + } + dirtyPaths := make(map[string]struct{}, len(status)) + for path, fileStatus := range status { + if fileStatus.Staging == git.Unmodified && fileStatus.Worktree == git.Unmodified { + continue + } + dirtyPaths[filepath.ToSlash(filepath.Clean(path))] = struct{}{} + } + return dirtyPaths, nil +} + +func reconcileRemoteWorktree(repo *git.Repository, worktree *git.Worktree, repoDir string, baseRef *plumbing.Reference, dirtyPaths map[string]struct{}) error { + if repo == nil || worktree == nil || baseRef == nil { + return fmt.Errorf("repository, worktree, or base reference is nil") + } + if !baseRef.Name().IsBranch() { + return fmt.Errorf("head %s is not a branch", baseRef.Name()) + } + remoteName := plumbing.NewRemoteReferenceName("origin", baseRef.Name().Short()) + remoteRef, errRemote := repo.Reference(remoteName, true) + if errRemote != nil { + return fmt.Errorf("resolve remote branch %s: %w", remoteName, errRemote) + } + baseCommit, errBaseCommit := repo.CommitObject(baseRef.Hash()) + if errBaseCommit != nil { + return fmt.Errorf("inspect pre-pull commit: %w", errBaseCommit) + } + baseTree, errBaseTree := baseCommit.Tree() + if errBaseTree != nil { + return fmt.Errorf("inspect pre-pull tree: %w", errBaseTree) + } + remoteCommit, errRemoteCommit := repo.CommitObject(remoteRef.Hash()) + if errRemoteCommit != nil { + return fmt.Errorf("inspect remote commit: %w", errRemoteCommit) + } + remoteTree, errRemoteTree := remoteCommit.Tree() + if errRemoteTree != nil { + return fmt.Errorf("inspect remote tree: %w", errRemoteTree) + } + changedPaths, errChangedPaths := changedTreePaths(baseTree, remoteTree) + if errChangedPaths != nil { + return errChangedPaths + } + for _, changedPath := range changedPaths { + if dirtyPath, conflict := overlappingDirtyPath(changedPath, dirtyPaths); conflict { + if errRestore := restoreHeadAndIndex(repo, worktree, baseRef); errRestore != nil { + return errors.Join( + fmt.Errorf("remote path %s conflicts with local change %s", changedPath, dirtyPath), + fmt.Errorf("restore pre-pull head after conflict: %w", errRestore), + ) + } + return fmt.Errorf("remote path %s conflicts with local change %s", changedPath, dirtyPath) + } + } + + // Pull moves HEAD before reporting unstaged changes. Return to the pre-pull + // tree before applying only remote changes that do not overlap local edits. + if errRestore := restoreHeadAndIndex(repo, worktree, baseRef); errRestore != nil { + return fmt.Errorf("restore pre-pull head: %w", errRestore) + } + if errApply := applyTreePaths(remoteTree, repoDir, changedPaths); errApply != nil { + if errRollback := applyTreePaths(baseTree, repoDir, changedPaths); errRollback != nil { + return errors.Join( + fmt.Errorf("apply remote worktree changes: %w", errApply), + fmt.Errorf("restore pre-pull worktree: %w", errRollback), + ) + } + return fmt.Errorf("apply remote worktree changes: %w", errApply) + } + if errReference := repo.Storer.SetReference(plumbing.NewHashReference(baseRef.Name(), remoteRef.Hash())); errReference != nil { + if errRollback := applyTreePaths(baseTree, repoDir, changedPaths); errRollback != nil { + return errors.Join( + fmt.Errorf("update branch %s: %w", baseRef.Name(), errReference), + fmt.Errorf("restore pre-pull worktree: %w", errRollback), + ) + } + return fmt.Errorf("update branch %s: %w", baseRef.Name(), errReference) + } + if errReset := worktree.Reset(&git.ResetOptions{Mode: git.MixedReset, Commit: remoteRef.Hash()}); errReset != nil { + return fmt.Errorf("reset index to remote branch %s: %w", remoteName, errReset) + } + return nil +} + +func changedTreePaths(baseTree, remoteTree *object.Tree) ([]string, error) { + changes, errDiff := baseTree.Diff(remoteTree) + if errDiff != nil { + return nil, fmt.Errorf("compare pre-pull and remote trees: %w", errDiff) + } + paths := make(map[string]struct{}, len(changes)) + for _, change := range changes { + for _, path := range []string{change.From.Name, change.To.Name} { + if path == "" { + continue + } + paths[filepath.ToSlash(filepath.Clean(path))] = struct{}{} + } + } + changedPaths := make([]string, 0, len(paths)) + for path := range paths { + changedPaths = append(changedPaths, path) + } + sort.Strings(changedPaths) + return changedPaths, nil +} + +func overlappingDirtyPath(path string, dirtyPaths map[string]struct{}) (string, bool) { + for dirtyPath := range dirtyPaths { + if path == dirtyPath || strings.HasPrefix(path, dirtyPath+"/") || strings.HasPrefix(dirtyPath, path+"/") { + return dirtyPath, true + } + } + return "", false +} + +func applyTreePaths(tree *object.Tree, repoDir string, paths []string) error { + for _, path := range paths { + destination := filepath.Join(repoDir, filepath.FromSlash(path)) + file, errFile := tree.File(path) + if errors.Is(errFile, object.ErrFileNotFound) { + if errRemove := os.Remove(destination); errRemove != nil && !errors.Is(errRemove, fs.ErrNotExist) { + return fmt.Errorf("remove %s: %w", path, errRemove) + } + continue + } + if errFile != nil { + return fmt.Errorf("inspect %s: %w", path, errFile) + } + contents, errContents := file.Contents() + if errContents != nil { + return fmt.Errorf("read %s: %w", path, errContents) + } + if errMkdir := os.MkdirAll(filepath.Dir(destination), 0o700); errMkdir != nil { + return fmt.Errorf("create parent for %s: %w", path, errMkdir) + } + if errWrite := os.WriteFile(destination, []byte(contents), 0o600); errWrite != nil { + return fmt.Errorf("write %s: %w", path, errWrite) + } + } + return nil +} + +func (s *GitTokenStore) recoverRepositoryLocked(repoDir string, authMethod []client.Option, baselineTree *object.Tree, dirtyPaths map[string]struct{}) (errRecovery error) { + parentDir := filepath.Dir(repoDir) + recoveryRoot, errTemp := os.MkdirTemp(parentDir, ".gitstore-recovery-") + if errTemp != nil { + return fmt.Errorf("create recovery directory: %w", errTemp) + } + cleanupRecovery := true + defer func() { + if !cleanupRecovery { + return + } + if errRemove := os.RemoveAll(recoveryRoot); errRemove != nil { + errCleanup := fmt.Errorf("remove recovery directory: %w", errRemove) + if errRecovery == nil { + errRecovery = errCleanup + } else { + errRecovery = errors.Join(errRecovery, errCleanup) + } + } + }() + + if baselineTree == nil { + inspectedTree, inspectedDirtyPaths, errInspect := inspectRecoveryBaseline(repoDir) + if errInspect != nil { + return fmt.Errorf("inspect recovery baseline: %w", errInspect) + } + baselineTree = inspectedTree + dirtyPaths = inspectedDirtyPaths + } + cloneDir := filepath.Join(recoveryRoot, "clone") + cloneOpts := &git.CloneOptions{ClientOptions: authMethod, URL: s.remote} + if s.branch != "" { + cloneOpts.ReferenceName = plumbing.NewBranchReferenceName(s.branch) + } + clonedRepo, errClone := git.PlainClone(cloneDir, cloneOpts) + if errClone != nil { + return fmt.Errorf("clone remote repository: %w", errClone) + } + if errVerify := verifyRepositoryHead(clonedRepo); errVerify != nil { + return fmt.Errorf("verify cloned repository: %w", errVerify) + } + clonedHead, errHead := clonedRepo.Head() + if errHead != nil { + return fmt.Errorf("get cloned repository head: %w", errHead) + } + clonedCommit, errCommit := clonedRepo.CommitObject(clonedHead.Hash()) + if errCommit != nil { + return fmt.Errorf("inspect cloned repository head: %w", errCommit) + } + remoteTree, errTree := clonedCommit.Tree() + if errTree != nil { + return fmt.Errorf("inspect cloned repository tree: %w", errTree) + } + preservedPaths, errPreserve := recoveryPreservedPaths(baselineTree, remoteTree, dirtyPaths) + if errPreserve != nil { + return errPreserve + } + if errApply := applyRecoveryLocalChanges(repoDir, cloneDir, preservedPaths); errApply != nil { + return fmt.Errorf("preserve local worktree changes: %w", errApply) + } + + backupWorktreeDir := filepath.Join(recoveryRoot, "worktree") + if errBackup := moveWorktreeEntries(repoDir, backupWorktreeDir); errBackup != nil { + return fmt.Errorf("backup existing worktree: %w", errBackup) + } + gitDir := filepath.Join(repoDir, ".git") + clonedGitDir := filepath.Join(cloneDir, ".git") + backupGitDir := filepath.Join(recoveryRoot, "corrupt.git") + retainRecovery, errInstall := installRecoveredGitDirectory(gitDir, clonedGitDir, backupGitDir, os.Rename) + if retainRecovery { + cleanupRecovery = false + } + if errInstall != nil { + if errRestore := moveWorktreeEntries(backupWorktreeDir, repoDir); errRestore != nil { + cleanupRecovery = false + return errors.Join(errInstall, fmt.Errorf("restore worktree; backup retained at %s: %w", backupWorktreeDir, errRestore)) + } + return errInstall + } + if errMove := moveWorktreeEntries(cloneDir, repoDir); errMove != nil { + errMoveWorktree := fmt.Errorf("install recovered worktree: %w", errMove) + if errRollback := rollbackRecoveredRepository(repoDir, gitDir, backupGitDir, backupWorktreeDir); errRollback != nil { + cleanupRecovery = false + return errors.Join(errMoveWorktree, fmt.Errorf("rollback recovered repository; backup retained at %s: %w", recoveryRoot, errRollback)) + } + return errMoveWorktree + } + recoveredRepo, errOpen := git.PlainOpen(repoDir) + if errOpen == nil { + errOpen = verifyRepositoryHead(recoveredRepo) + } + if errOpen != nil { + errRecovered := fmt.Errorf("verify recovered repository: %w", errOpen) + if errRollback := rollbackRecoveredRepository(repoDir, gitDir, backupGitDir, backupWorktreeDir); errRollback != nil { + cleanupRecovery = false + return errors.Join(errRecovered, fmt.Errorf("rollback recovered repository; backup retained at %s: %w", recoveryRoot, errRollback)) + } + return errRecovered + } + return nil +} + +func inspectRecoveryBaseline(repoDir string) (*object.Tree, map[string]struct{}, error) { + repo, errOpen := git.PlainOpen(repoDir) + if errOpen != nil { + return nil, nil, fmt.Errorf("open repository: %w", errOpen) + } + worktree, errWorktree := repo.Worktree() + if errWorktree != nil { + return nil, nil, fmt.Errorf("open worktree: %w", errWorktree) + } + dirtyPaths, errDirty := worktreeDirtyPaths(worktree) + if errDirty != nil { + return nil, nil, fmt.Errorf("inspect worktree changes: %w", errDirty) + } + head, errHead := repo.Head() + if errHead != nil { + return nil, nil, fmt.Errorf("inspect head: %w", errHead) + } + commit, errCommit := repo.CommitObject(head.Hash()) + if errCommit != nil { + return nil, nil, fmt.Errorf("inspect head commit: %w", errCommit) + } + tree, errTree := commit.Tree() + if errTree != nil { + return nil, nil, fmt.Errorf("inspect head tree: %w", errTree) + } + return tree, dirtyPaths, nil +} + +func recoveryPreservedPaths(baselineTree, remoteTree *object.Tree, dirtyPaths map[string]struct{}) (map[string]struct{}, error) { + if baselineTree == nil || len(dirtyPaths) == 0 { + return nil, nil + } + changedPaths, errChanged := changedTreePaths(baselineTree, remoteTree) + if errChanged != nil { + return nil, fmt.Errorf("verify local changes against recovered remote: %w", errChanged) + } + for _, changedPath := range changedPaths { + if dirtyPath, conflict := overlappingDirtyPath(changedPath, dirtyPaths); conflict { + return nil, fmt.Errorf("remote path %s conflicts with local change %s during repository recovery", changedPath, dirtyPath) + } + } + return dirtyPaths, nil +} + +func applyRecoveryLocalChanges(sourceDir, targetDir string, paths map[string]struct{}) error { + sortedPaths := make([]string, 0, len(paths)) + for path := range paths { + sortedPaths = append(sortedPaths, path) + } + sort.Strings(sortedPaths) + for _, path := range sortedPaths { + source := filepath.Join(sourceDir, filepath.FromSlash(path)) + target := filepath.Join(targetDir, filepath.FromSlash(path)) + info, errStat := os.Lstat(source) + if errors.Is(errStat, fs.ErrNotExist) { + if errRemove := os.RemoveAll(target); errRemove != nil { + return fmt.Errorf("preserve deletion %s: %w", path, errRemove) + } + continue + } + if errStat != nil { + return fmt.Errorf("inspect local change %s: %w", path, errStat) + } + if errRemove := os.RemoveAll(target); errRemove != nil { + return fmt.Errorf("replace recovered path %s: %w", path, errRemove) + } + if errMkdir := os.MkdirAll(filepath.Dir(target), 0o700); errMkdir != nil { + return fmt.Errorf("create recovered parent for %s: %w", path, errMkdir) + } + switch { + case info.Mode().IsRegular(): + contents, errRead := os.ReadFile(source) + if errRead != nil { + return fmt.Errorf("read local change %s: %w", path, errRead) + } + if errWrite := os.WriteFile(target, contents, info.Mode().Perm()); errWrite != nil { + return fmt.Errorf("write local change %s: %w", path, errWrite) + } + case info.Mode()&os.ModeSymlink != 0: + linkTarget, errReadlink := os.Readlink(source) + if errReadlink != nil { + return fmt.Errorf("read local symlink %s: %w", path, errReadlink) + } + if errSymlink := os.Symlink(linkTarget, target); errSymlink != nil { + return fmt.Errorf("write local symlink %s: %w", path, errSymlink) + } + default: + return fmt.Errorf("local change %s has unsupported file mode %s", path, info.Mode()) + } + } + return nil +} + +func moveWorktreeEntries(sourceDir, targetDir string) error { + if errMkdir := os.MkdirAll(targetDir, 0o700); errMkdir != nil { + return errMkdir + } + entries, errRead := os.ReadDir(sourceDir) + if errRead != nil { + return errRead + } + moved := make([]string, 0, len(entries)) + for _, entry := range entries { + if entry.Name() == ".git" { + continue + } + source := filepath.Join(sourceDir, entry.Name()) + target := filepath.Join(targetDir, entry.Name()) + if errRename := os.Rename(source, target); errRename != nil { + errMove := fmt.Errorf("move %s: %w", entry.Name(), errRename) + for index := len(moved) - 1; index >= 0; index-- { + name := moved[index] + if errRestore := os.Rename(filepath.Join(targetDir, name), filepath.Join(sourceDir, name)); errRestore != nil { + errMove = errors.Join(errMove, fmt.Errorf("restore %s: %w", name, errRestore)) + } + } + return errMove + } + moved = append(moved, entry.Name()) + } + return nil +} + +func removeWorktreeEntries(repoDir string) error { + entries, errRead := os.ReadDir(repoDir) + if errRead != nil { + return errRead + } + for _, entry := range entries { + if entry.Name() == ".git" { + continue + } + if errRemove := os.RemoveAll(filepath.Join(repoDir, entry.Name())); errRemove != nil { + return errRemove + } + } + return nil +} + +func rollbackRecoveredRepository(repoDir, gitDir, backupGitDir, backupWorktreeDir string) error { + if errRemove := removeWorktreeEntries(repoDir); errRemove != nil { + return fmt.Errorf("remove recovered worktree: %w", errRemove) + } + if errRollback := rollbackRecoveredGitDirectory(gitDir, backupGitDir); errRollback != nil { + return errRollback + } + if errRestore := moveWorktreeEntries(backupWorktreeDir, repoDir); errRestore != nil { + return fmt.Errorf("restore original worktree: %w", errRestore) + } + return nil +} + +func installRecoveredGitDirectory(gitDir, clonedGitDir, backupGitDir string, rename func(string, string) error) (bool, error) { + if errRename := rename(gitDir, backupGitDir); errRename != nil { + return false, fmt.Errorf("backup corrupt git directory: %w", errRename) + } + if errRename := rename(clonedGitDir, gitDir); errRename != nil { + if errRestore := rename(backupGitDir, gitDir); errRestore != nil { + return true, errors.Join( + fmt.Errorf("install recovered git directory: %w", errRename), + fmt.Errorf("restore corrupt git directory; backup retained at %s: %w", backupGitDir, errRestore), + ) + } + return false, fmt.Errorf("install recovered git directory: %w", errRename) + } + return false, nil +} + +func rollbackRecoveredGitDirectory(gitDir, backupGitDir string) error { + if errRemove := os.RemoveAll(gitDir); errRemove != nil { + return fmt.Errorf("remove recovered git directory: %w", errRemove) + } + if errRename := os.Rename(backupGitDir, gitDir); errRename != nil { + return fmt.Errorf("restore original git directory: %w", errRename) + } + return nil +} + +func isRepositoryCorruptionError(err error) bool { + return errors.Is(err, dotgit.ErrPackfileNotFound) || errors.Is(err, plumbing.ErrObjectNotFound) +} + +func verifyRepositoryHead(repo *git.Repository) error { + if repo == nil { + return fmt.Errorf("repository is nil") + } + head, errHead := repo.Head() + if errHead != nil { + if errors.Is(errHead, plumbing.ErrReferenceNotFound) { + return nil + } + return errHead + } + commit, errCommit := repo.CommitObject(head.Hash()) + if errCommit != nil { + return errCommit + } + tree, errTree := commit.Tree() + if errTree != nil { + return errTree + } + files := tree.Files() + return files.ForEach(func(file *object.File) error { + _, errContents := file.Contents() + return errContents + }) +} + +func restoreMissingTrackedFiles(repo *git.Repository, repoDir string) error { + if repo == nil { + return fmt.Errorf("repository is nil") + } + head, errHead := repo.Head() + if errHead != nil { + if errors.Is(errHead, plumbing.ErrReferenceNotFound) { + return nil + } + return errHead + } + commit, errCommit := repo.CommitObject(head.Hash()) + if errCommit != nil { + return errCommit + } + tree, errTree := commit.Tree() + if errTree != nil { + return errTree + } + files := tree.Files() + return files.ForEach(func(file *object.File) error { + destination := filepath.Join(repoDir, filepath.FromSlash(file.Name)) + if _, errStat := os.Lstat(destination); errStat == nil { + return nil + } else if !errors.Is(errStat, fs.ErrNotExist) { + return errStat + } + contents, errContents := file.Contents() + if errContents != nil { + return errContents + } + if errMkdir := os.MkdirAll(filepath.Dir(destination), 0o700); errMkdir != nil { + return errMkdir + } + return os.WriteFile(destination, []byte(contents), 0o600) + }) +} + func shouldFallbackToCurrentBranch(repo *git.Repository, err error) bool { if !errors.Is(err, transport.ErrAuthenticationRequired) && !errors.Is(err, transport.ErrEmptyRemoteRepository) { return false @@ -881,6 +1509,14 @@ func checkoutRemoteDefaultBranch(repo *git.Repository, worktree *git.Worktree, a } func (s *GitTokenStore) commitAndPushLocked(message string, relPaths ...string) error { + return s.commitAndPushWithOptionsLocked(message, false, relPaths...) +} + +func (s *GitTokenStore) commitAndPushInitialLocked(message string, relPaths ...string) error { + return s.commitAndPushWithOptionsLocked(message, true, relPaths...) +} + +func (s *GitTokenStore) commitAndPushWithOptionsLocked(message string, allowMissingRemote bool, relPaths ...string) error { repoDir := s.repoDirSnapshot() if repoDir == "" { return fmt.Errorf("git token store: repository path not configured") @@ -893,11 +1529,26 @@ func (s *GitTokenStore) commitAndPushLocked(message string, relPaths ...string) if err != nil { return fmt.Errorf("git token store: worktree: %w", err) } - added := false - for _, rel := range relPaths { - if strings.TrimSpace(rel) == "" { - continue + managedPaths, errPaths := normalizeManagedPaths(relPaths) + if errPaths != nil { + return fmt.Errorf("git token store: validate commit paths: %w", errPaths) + } + if len(managedPaths) == 0 { + return nil + } + + baseRef, errHead := repo.Head() + if errHead != nil && !errors.Is(errHead, plumbing.ErrReferenceNotFound) { + return fmt.Errorf("git token store: get base head: %w", errHead) + } + if errHead == nil { + if errReset := resetIndexToHead(repo, worktree); errReset != nil { + return fmt.Errorf("git token store: reset index before commit: %w", errReset) } + } + + added := false + for _, rel := range managedPaths { if _, err = worktree.Add(rel); err != nil { if errors.Is(err, gitindex.ErrEntryNotFound) { continue @@ -942,18 +1593,111 @@ func (s *GitTokenStore) commitAndPushLocked(message string, relPaths ...string) } return fmt.Errorf("git token store: commit: %w", err) } - headRef, errHead := repo.Head() - if errHead != nil { - if !errors.Is(errHead, plumbing.ErrReferenceNotFound) { - return fmt.Errorf("git token store: get head: %w", errHead) + if baseRef != nil { + if errValidate := validateManagedTreeChanges(repo, baseRef.Hash(), commitHash, managedPaths); errValidate != nil { + errRestore := restoreHeadAndIndex(repo, worktree, baseRef) + if errRestore != nil { + return errors.Join( + fmt.Errorf("git token store: validate commit tree: %w", errValidate), + fmt.Errorf("git token store: restore head after rejected commit: %w", errRestore), + ) + } + return fmt.Errorf("git token store: validate commit tree: %w", errValidate) } - } else if errRewrite := s.rewriteHeadAsSingleCommit(repo, headRef.Name(), commitHash, message, signature); errRewrite != nil { + } + headRef, errCommittedHead := repo.Head() + if errCommittedHead != nil { + return fmt.Errorf("git token store: get committed head: %w", errCommittedHead) + } + if errRewrite := s.rewriteHeadAsSingleCommit(repo, headRef.Name(), commitHash, message, signature); errRewrite != nil { return errRewrite } - return s.pushRepositoryLocked(repo, repoDir) + if errPush := s.pushRepositoryLocked(repo, repoDir, allowMissingRemote); errPush != nil { + if baseRef == nil { + return errPush + } + if errRestore := restoreHeadAndIndex(repo, worktree, baseRef); errRestore != nil { + return errors.Join(errPush, fmt.Errorf("git token store: restore head after rejected push: %w", errRestore)) + } + return errPush + } + return nil +} + +func normalizeManagedPaths(paths []string) ([]string, error) { + normalized := make([]string, 0, len(paths)) + seen := make(map[string]struct{}, len(paths)) + for _, path := range paths { + trimmed := strings.TrimSpace(path) + if trimmed == "" { + continue + } + clean := filepath.ToSlash(filepath.Clean(trimmed)) + if clean == "." || clean == ".." || strings.HasPrefix(clean, "../") || filepath.IsAbs(trimmed) { + return nil, fmt.Errorf("path %q is not a repository-relative file", path) + } + if _, ok := seen[clean]; ok { + continue + } + seen[clean] = struct{}{} + normalized = append(normalized, clean) + } + return normalized, nil } -func (s *GitTokenStore) pushRepositoryLocked(repo *git.Repository, repoDir string) error { +func validateManagedTreeChanges(repo *git.Repository, baseHash, commitHash plumbing.Hash, managedPaths []string) error { + baseCommit, errBase := repo.CommitObject(baseHash) + if errBase != nil { + return fmt.Errorf("inspect base commit: %w", errBase) + } + baseTree, errBaseTree := baseCommit.Tree() + if errBaseTree != nil { + return fmt.Errorf("inspect base tree: %w", errBaseTree) + } + commit, errCommit := repo.CommitObject(commitHash) + if errCommit != nil { + return fmt.Errorf("inspect candidate commit: %w", errCommit) + } + candidateTree, errCandidateTree := commit.Tree() + if errCandidateTree != nil { + return fmt.Errorf("inspect candidate tree: %w", errCandidateTree) + } + changes, errDiff := baseTree.Diff(candidateTree) + if errDiff != nil { + return fmt.Errorf("compare candidate tree: %w", errDiff) + } + for _, change := range changes { + for _, changedPath := range []string{change.From.Name, change.To.Name} { + if changedPath == "" || isManagedTreePath(changedPath, managedPaths) { + continue + } + return fmt.Errorf("unexpected indexed change outside requested paths: %s", changedPath) + } + } + return nil +} + +func isManagedTreePath(path string, managedPaths []string) bool { + cleanPath := filepath.ToSlash(filepath.Clean(path)) + for _, managedPath := range managedPaths { + if cleanPath == managedPath || strings.HasPrefix(cleanPath, managedPath+"/") { + return true + } + } + return false +} + +func restoreHeadAndIndex(repo *git.Repository, worktree *git.Worktree, head *plumbing.Reference) error { + if repo == nil || worktree == nil || head == nil { + return fmt.Errorf("repository, worktree, or head is nil") + } + if errReference := repo.Storer.SetReference(plumbing.NewHashReference(head.Name(), head.Hash())); errReference != nil { + return errReference + } + return worktree.Reset(&git.ResetOptions{Mode: git.MixedReset, Commit: head.Hash()}) +} + +func (s *GitTokenStore) pushRepositoryLocked(repo *git.Repository, repoDir string, allowMissingRemote bool) error { if repo == nil { return fmt.Errorf("git token store: repository is nil") } @@ -964,18 +1708,33 @@ func (s *GitTokenStore) pushRepositoryLocked(repo *git.Repository, repoDir strin } return fmt.Errorf("git token store: get head for push: %w", errHead) } - pushOpts := &git.PushOptions{ClientOptions: s.gitClientOptions(), Force: true} - if s.branch != "" { - pushOpts.RefSpecs = []config.RefSpec{config.RefSpec("refs/heads/" + s.branch + ":refs/heads/" + s.branch)} - } else { - // When branch is unset, pin push to the currently checked-out branch. - pushOpts.RefSpecs = []config.RefSpec{config.RefSpec(headRef.Name().String() + ":" + headRef.Name().String())} + if !headRef.Name().IsBranch() { + return fmt.Errorf("git token store: head %s is not a branch", headRef.Name()) + } + branchName := headRef.Name() + remoteName := plumbing.NewRemoteReferenceName("origin", branchName.Short()) + pushOpts := &git.PushOptions{ + ClientOptions: s.gitClientOptions(), + RefSpecs: []config.RefSpec{config.RefSpec(branchName.String() + ":" + branchName.String())}, + } + remoteRef, errRemote := repo.Reference(remoteName, true) + switch { + case errRemote == nil: + pushOpts.ForceWithLease = &git.ForceWithLease{RefName: branchName, Hash: remoteRef.Hash()} + case errors.Is(errRemote, plumbing.ErrReferenceNotFound) && allowMissingRemote: + // A normal branch-creation push fails if another initializer wins the race. + case errors.Is(errRemote, plumbing.ErrReferenceNotFound): + return fmt.Errorf("git token store: remote tracking branch %s not found", remoteName) + default: + return fmt.Errorf("git token store: inspect remote tracking branch %s: %w", remoteName, errRemote) } if errPush := repo.Push(pushOpts); errPush != nil { - if errors.Is(errPush, git.NoErrAlreadyUpToDate) { - return nil + if !errors.Is(errPush, git.NoErrAlreadyUpToDate) { + return fmt.Errorf("git token store: push: %w", errPush) } - return fmt.Errorf("git token store: push: %w", errPush) + } + if errReference := repo.Storer.SetReference(plumbing.NewHashReference(remoteName, headRef.Hash())); errReference != nil { + return fmt.Errorf("git token store: update remote tracking branch %s: %w", remoteName, errReference) } s.maybeRunGC(repoDir) return nil @@ -1024,7 +1783,7 @@ func (s *GitTokenStore) maybeRunGC(repoDir string) { } pruneOpts := git.PruneOptions{ - OnlyObjectsOlderThan: now, + OnlyObjectsOlderThan: now.Add(-gcPruneGracePeriod), Handler: repo.DeleteObject, } if err := repo.Prune(pruneOpts); err != nil && !errors.Is(err, git.ErrLooseObjectsNotSupported) { @@ -1035,7 +1794,10 @@ func (s *GitTokenStore) maybeRunGC(repoDir string) { // PersistConfig commits and pushes configuration changes to git. func (s *GitTokenStore) PersistConfig(_ context.Context) error { - if err := s.EnsureRepository(); err != nil { + s.mu.Lock() + defer s.mu.Unlock() + + if err := s.ensureRepositoryLocked(); err != nil { return err } configPath := s.ConfigPath() @@ -1048,8 +1810,6 @@ func (s *GitTokenStore) PersistConfig(_ context.Context) error { } return fmt.Errorf("git token store: stat config: %w", err) } - s.mu.Lock() - defer s.mu.Unlock() rel, err := s.relativeToRepo(configPath) if err != nil { return err diff --git a/internal/store/gitstore_test.go b/internal/store/gitstore_test.go index 0c10c53c..df82ff8f 100644 --- a/internal/store/gitstore_test.go +++ b/internal/store/gitstore_test.go @@ -2,6 +2,7 @@ package store import ( "context" + "encoding/json" "errors" "net/http" "net/http/httptest" @@ -23,6 +24,14 @@ type testBranchSpec struct { contents string } +type callbackTokenStorage struct { + save func(string) error +} + +func (s *callbackTokenStorage) SaveTokenToFile(path string) error { + return s.save(path) +} + func TestEnsureRepositoryUsesRemoteDefaultBranchWhenBranchNotConfigured(t *testing.T) { root := t.TempDir() remoteDir := setupGitRemoteRepository(t, root, "trunk", @@ -376,6 +385,911 @@ func TestGitTokenStoreRepeatedDeleteDoesNotOverwriteRemoteOnlyChanges(t *testing assertRemoteTreePath(t, remoteDir, "master", "auths/b.json", true) } +func TestGitTokenStoreRejectsPathsOutsideRepositoryBeforeMutation(t *testing.T) { + t.Parallel() + + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + store := NewGitTokenStore(remoteDir, "", "", "") + baseDir := filepath.Join(root, "workspace", "auths") + store.SetBaseDir(baseDir) + if err := store.EnsureRepository(); err != nil { + t.Fatalf("EnsureRepository: %v", err) + } + + outsidePath := filepath.Join(root, "outside.json") + outsideContents := []byte("outside\n") + if err := os.WriteFile(outsidePath, outsideContents, 0o600); err != nil { + t.Fatalf("write outside file: %v", err) + } + if err := store.Delete(context.Background(), outsidePath); err == nil { + t.Fatal("Delete outside repository error = nil, want rejection") + } + if got, errRead := os.ReadFile(outsidePath); errRead != nil { + t.Fatalf("read outside file after delete rejection: %v", errRead) + } else if string(got) != string(outsideContents) { + t.Fatalf("outside file contents = %q, want %q", got, outsideContents) + } + + outsideSavePath := filepath.Join(root, "outside-save.json") + auth := &cliproxyauth.Auth{ + ID: "outside-save.json", + FileName: "outside-save.json", + Provider: "codex", + Attributes: map[string]string{ + cliproxyauth.AttributePath: outsideSavePath, + }, + Metadata: map[string]any{"type": "codex", "access_token": "token"}, + } + if _, err := store.Save(context.Background(), auth); err == nil { + t.Fatal("Save outside repository error = nil, want rejection") + } + if _, errStat := os.Stat(outsideSavePath); !errors.Is(errStat, os.ErrNotExist) { + t.Fatalf("outside save path stat error = %v, want not exist", errStat) + } +} + +func TestGitTokenStorePersistConfigDropsUnrelatedStagedDeletions(t *testing.T) { + t.Parallel() + + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + store := NewGitTokenStore(remoteDir, "", "", "") + baseDir := filepath.Join(root, "workspace", "auths") + store.SetBaseDir(baseDir) + if err := store.EnsureRepository(); err != nil { + t.Fatalf("EnsureRepository: %v", err) + } + + auth := &cliproxyauth.Auth{ + ID: "protected.json", + FileName: "protected.json", + Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "token"}, + } + authPath, err := store.Save(context.Background(), auth) + if err != nil { + t.Fatalf("Save: %v", err) + } + configPath := store.ConfigPath() + if err := os.WriteFile(configPath, []byte("version: one\n"), 0o600); err != nil { + t.Fatalf("write initial config: %v", err) + } + if err := store.PersistConfig(context.Background()); err != nil { + t.Fatalf("PersistConfig initial: %v", err) + } + + repo, err := git.PlainOpen(filepath.Join(root, "workspace")) + if err != nil { + t.Fatalf("open workspace repo: %v", err) + } + worktree, err := repo.Worktree() + if err != nil { + t.Fatalf("open workspace worktree: %v", err) + } + if _, err := worktree.Remove("auths/protected.json"); err != nil { + t.Fatalf("stage unexpected auth removal: %v", err) + } + if _, err := os.Stat(authPath); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("removed auth stat error = %v, want not exist", err) + } + if err := os.WriteFile(configPath, []byte("version: two\n"), 0o600); err != nil { + t.Fatalf("write updated config: %v", err) + } + + if err := store.PersistConfig(context.Background()); err != nil { + t.Fatalf("PersistConfig with corrupt index: %v", err) + } + assertRemoteTreePath(t, remoteDir, "master", "auths/protected.json", true) + assertRemoteFileContents(t, remoteDir, "master", "config/config.yaml", "version: two\n") +} + +func TestGitTokenStorePersistConfigRepairsIndexAfterUnstagedPull(t *testing.T) { + t.Parallel() + + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + store := NewGitTokenStore(remoteDir, "", "", "") + store.SetBaseDir(filepath.Join(root, "workspace", "auths")) + if err := store.EnsureRepository(); err != nil { + t.Fatalf("EnsureRepository: %v", err) + } + configPath := store.ConfigPath() + if err := os.WriteFile(configPath, []byte("source: local-config\n"), 0o600); err != nil { + t.Fatalf("write local config: %v", err) + } + advanceRemoteBranch(t, filepath.Join(root, "seed"), remoteDir, "master", "remote branch advanced\n", "advance remote") + + if err := store.PersistConfig(context.Background()); err != nil { + t.Fatalf("PersistConfig after unstaged pull: %v", err) + } + assertRemoteBranchContents(t, remoteDir, "master", "remote branch advanced\n") + assertRemoteFileContents(t, remoteDir, "master", "config/config.yaml", "source: local-config\n") +} + +func TestGitTokenStorePersistConfigPreservesRemoteOnlyAuthAfterDivergence(t *testing.T) { + t.Parallel() + + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + storeA := NewGitTokenStore(remoteDir, "", "", "") + storeA.SetBaseDir(filepath.Join(root, "workspace-a", "auths")) + if err := storeA.EnsureRepository(); err != nil { + t.Fatalf("EnsureRepository A: %v", err) + } + + storeB := NewGitTokenStore(remoteDir, "", "", "") + storeB.SetBaseDir(filepath.Join(root, "workspace-b", "auths")) + if err := storeB.EnsureRepository(); err != nil { + t.Fatalf("EnsureRepository B: %v", err) + } + authB := &cliproxyauth.Auth{ + ID: "remote-only.json", + FileName: "remote-only.json", + Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote"}, + } + if _, err := storeB.Save(context.Background(), authB); err != nil { + t.Fatalf("Save B: %v", err) + } + assertRemoteTreePath(t, remoteDir, "master", "auths/remote-only.json", true) + + configPathA := storeA.ConfigPath() + if err := os.WriteFile(configPathA, []byte("source: store-a\n"), 0o600); err != nil { + t.Fatalf("write config A: %v", err) + } + if err := storeA.PersistConfig(context.Background()); err != nil { + t.Fatalf("PersistConfig A after divergence: %v", err) + } + + assertRemoteTreePath(t, remoteDir, "master", "auths/remote-only.json", true) + assertRemoteFileContents(t, remoteDir, "master", "config/config.yaml", "source: store-a\n") +} + +func TestGitTokenStoreRejectsStaleForcePush(t *testing.T) { + t.Parallel() + + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + storeA := NewGitTokenStore(remoteDir, "", "", "") + storeA.SetBaseDir(filepath.Join(root, "workspace-a", "auths")) + if err := storeA.EnsureRepository(); err != nil { + t.Fatalf("EnsureRepository A: %v", err) + } + storeB := NewGitTokenStore(remoteDir, "", "", "") + storeB.SetBaseDir(filepath.Join(root, "workspace-b", "auths")) + if err := storeB.EnsureRepository(); err != nil { + t.Fatalf("EnsureRepository B: %v", err) + } + + authB := &cliproxyauth.Auth{ + ID: "concurrent.json", + FileName: "concurrent.json", + Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote"}, + } + if _, err := storeB.Save(context.Background(), authB); err != nil { + t.Fatalf("Save B: %v", err) + } + configPathA := storeA.ConfigPath() + if err := os.WriteFile(configPathA, []byte("source: stale-a\n"), 0o600); err != nil { + t.Fatalf("write stale config A: %v", err) + } + + storeA.mu.Lock() + errPush := storeA.commitAndPushLocked("Update stale config", "config/config.yaml") + storeA.mu.Unlock() + if errPush == nil { + t.Fatal("stale force push error = nil, want lease rejection") + } + assertRemoteTreePath(t, remoteDir, "master", "auths/concurrent.json", true) + assertRemoteTreePath(t, remoteDir, "master", "config/config.yaml", false) + + if err := storeA.PersistConfig(context.Background()); err != nil { + t.Fatalf("PersistConfig A after lease rejection: %v", err) + } + assertRemoteTreePath(t, remoteDir, "master", "auths/concurrent.json", true) + assertRemoteFileContents(t, remoteDir, "master", "config/config.yaml", "source: stale-a\n") +} + +func TestGitTokenStoreSaveRetryAfterLeaseConflictCommitsMatchingContent(t *testing.T) { + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + storeA := NewGitTokenStore(remoteDir, "", "", "") + storeA.SetBaseDir(filepath.Join(root, "workspace-a", "auths")) + if errEnsure := storeA.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository A: %v", errEnsure) + } + storeB := NewGitTokenStore(remoteDir, "", "", "") + storeB.SetBaseDir(filepath.Join(root, "workspace-b", "auths")) + if errEnsure := storeB.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository B: %v", errEnsure) + } + + authA := &cliproxyauth.Auth{ + ID: "local.json", + FileName: "local.json", + Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "local"}, + } + remoteAdvanced := false + authA.Storage = &callbackTokenStorage{save: func(path string) error { + raw, errMarshal := json.Marshal(authA.Metadata) + if errMarshal != nil { + return errMarshal + } + if errWrite := os.WriteFile(path, raw, 0o600); errWrite != nil { + return errWrite + } + if remoteAdvanced { + return nil + } + remoteAdvanced = true + _, errSave := storeB.Save(context.Background(), &cliproxyauth.Auth{ + ID: "concurrent.json", + FileName: "concurrent.json", + Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote"}, + }) + return errSave + }} + if _, errSave := storeA.Save(context.Background(), authA); errSave == nil { + t.Fatal("first Save error = nil, want lease rejection") + } + assertRemoteTreePath(t, remoteDir, "master", "auths/local.json", false) + assertRemoteTreePath(t, remoteDir, "master", "auths/concurrent.json", true) + + authA.Storage = nil + if _, errSave := storeA.Save(context.Background(), authA); errSave != nil { + t.Fatalf("second Save after lease rejection: %v", errSave) + } + assertRemoteFileContents(t, remoteDir, "master", "auths/local.json", `{"access_token":"local","disabled":false,"type":"codex"}`) + assertRemoteTreePath(t, remoteDir, "master", "auths/concurrent.json", true) +} + +func TestGitTokenStoreConcurrentInitializationDoesNotOverwriteCreatedBranch(t *testing.T) { + root := t.TempDir() + remoteDir := filepath.Join(root, "remote.git") + remoteRepo, errInitRemote := git.PlainInit(remoteDir, true) + if errInitRemote != nil { + t.Fatalf("init bare remote: %v", errInitRemote) + } + if errHead := remoteRepo.Storer.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.NewBranchReferenceName("master"))); errHead != nil { + t.Fatalf("set remote HEAD: %v", errHead) + } + + workspaceDir := filepath.Join(root, "workspace") + localRepo, errInitLocal := git.PlainInit(workspaceDir, false) + if errInitLocal != nil { + t.Fatalf("init local repository: %v", errInitLocal) + } + if errSigning := disableGitCommitSigning(workspaceDir); errSigning != nil { + t.Fatalf("disable local commit signing: %v", errSigning) + } + if _, errRemote := localRepo.CreateRemote(&gitconfig.RemoteConfig{Name: "origin", URLs: []string{remoteDir}}); errRemote != nil { + t.Fatalf("create local origin: %v", errRemote) + } + for _, path := range []string{"auths/.gitkeep", "config/.gitkeep"} { + fullPath := filepath.Join(workspaceDir, filepath.FromSlash(path)) + if errMkdir := os.MkdirAll(filepath.Dir(fullPath), 0o700); errMkdir != nil { + t.Fatalf("create local placeholder parent: %v", errMkdir) + } + if errWrite := os.WriteFile(fullPath, nil, 0o600); errWrite != nil { + t.Fatalf("write local placeholder: %v", errWrite) + } + } + + winnerDir := filepath.Join(root, "winner") + winnerRepo, errInitWinner := git.PlainInit(winnerDir, false) + if errInitWinner != nil { + t.Fatalf("init winning repository: %v", errInitWinner) + } + if errSigning := disableGitCommitSigning(winnerDir); errSigning != nil { + t.Fatalf("disable winner commit signing: %v", errSigning) + } + if errHead := winnerRepo.Storer.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.NewBranchReferenceName("master"))); errHead != nil { + t.Fatalf("set winner HEAD: %v", errHead) + } + winnerFiles := map[string]string{ + "auths/remote.json": `{"type":"codex","access_token":"remote"}`, + "config/config.yaml": "source: winner\n", + } + winnerWorktree, errWinnerWorktree := winnerRepo.Worktree() + if errWinnerWorktree != nil { + t.Fatalf("open winning worktree: %v", errWinnerWorktree) + } + for path, contents := range winnerFiles { + fullPath := filepath.Join(winnerDir, filepath.FromSlash(path)) + if errMkdir := os.MkdirAll(filepath.Dir(fullPath), 0o700); errMkdir != nil { + t.Fatalf("create winning file parent: %v", errMkdir) + } + if errWrite := os.WriteFile(fullPath, []byte(contents), 0o600); errWrite != nil { + t.Fatalf("write winning file: %v", errWrite) + } + if _, errAdd := winnerWorktree.Add(path); errAdd != nil { + t.Fatalf("add winning file: %v", errAdd) + } + } + if _, errCommit := winnerWorktree.Commit("Initialize complete store", &git.CommitOptions{Author: &object.Signature{ + Name: "CLIProxyAPI", Email: "cliproxy@local", When: time.Unix(1711929600, 0), + }}); errCommit != nil { + t.Fatalf("commit winning repository: %v", errCommit) + } + if _, errRemote := winnerRepo.CreateRemote(&gitconfig.RemoteConfig{Name: "origin", URLs: []string{remoteDir}}); errRemote != nil { + t.Fatalf("create winner origin: %v", errRemote) + } + if errPush := winnerRepo.Push(&git.PushOptions{RemoteName: "origin", RefSpecs: []gitconfig.RefSpec{"refs/heads/master:refs/heads/master"}}); errPush != nil { + t.Fatalf("push winning initialization: %v", errPush) + } + + store := NewGitTokenStore(remoteDir, "", "", "master") + store.SetBaseDir(filepath.Join(workspaceDir, "auths")) + store.mu.Lock() + errInitialize := store.commitAndPushInitialLocked("Initialize git token store", "auths/.gitkeep", "config/.gitkeep") + store.mu.Unlock() + if errInitialize == nil { + t.Fatal("late initialization push error = nil, want branch-creation rejection") + } + assertRemoteFileContents(t, remoteDir, "master", "auths/remote.json", winnerFiles["auths/remote.json"]) + assertRemoteFileContents(t, remoteDir, "master", "config/config.yaml", winnerFiles["config/config.yaml"]) + + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository after initialization race: %v", errEnsure) + } + assertLocalFileContents(t, filepath.Join(workspaceDir, "auths", "remote.json"), winnerFiles["auths/remote.json"]) + assertLocalFileContents(t, filepath.Join(workspaceDir, "config", "config.yaml"), winnerFiles["config/config.yaml"]) +} + +func TestEnsureRepositoryRetryRestoresTrackedAuthOnUpToDatePull(t *testing.T) { + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + store := NewGitTokenStore(remoteDir, "", "", "") + baseDir := filepath.Join(root, "workspace", "auths") + store.SetBaseDir(baseDir) + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository: %v", errEnsure) + } + authPath, errSave := store.Save(context.Background(), &cliproxyauth.Auth{ + ID: "retry.json", + FileName: "retry.json", + Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote"}, + }) + if errSave != nil { + t.Fatalf("Save: %v", errSave) + } + + repo, errOpen := git.PlainOpen(filepath.Join(root, "workspace")) + if errOpen != nil { + t.Fatalf("open workspace repository: %v", errOpen) + } + worktree, errWorktree := repo.Worktree() + if errWorktree != nil { + t.Fatalf("open workspace worktree: %v", errWorktree) + } + if _, errRemove := worktree.Remove("auths/retry.json"); errRemove != nil { + t.Fatalf("stage missing auth: %v", errRemove) + } + cfg, errConfig := repo.Config() + if errConfig != nil { + t.Fatalf("read workspace config: %v", errConfig) + } + cfg.Remotes["origin"].URLs = []string{filepath.Join(root, "missing.git")} + if errSetConfig := repo.SetConfig(cfg); errSetConfig != nil { + t.Fatalf("break workspace origin: %v", errSetConfig) + } + if errEnsure := store.EnsureRepository(); errEnsure == nil { + t.Fatal("EnsureRepository with unavailable remote error = nil, want retryable failure") + } + if _, errStat := os.Stat(authPath); !errors.Is(errStat, os.ErrNotExist) { + t.Fatalf("missing auth stat error = %v, want not exist", errStat) + } + + cfg.Remotes["origin"].URLs = []string{remoteDir} + if errSetConfig := repo.SetConfig(cfg); errSetConfig != nil { + t.Fatalf("restore workspace origin: %v", errSetConfig) + } + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository retry: %v", errEnsure) + } + assertLocalFileContents(t, authPath, `{"access_token":"remote","disabled":false,"type":"codex"}`) + auths, errList := store.List(context.Background()) + if errList != nil { + t.Fatalf("List after retry: %v", errList) + } + if len(auths) != 1 || auths[0].ID != "retry.json" { + t.Fatalf("List after retry = %#v, want retry.json", auths) + } + + if errDelete := store.Delete(context.Background(), authPath); errDelete != nil { + t.Fatalf("explicit Delete after retry: %v", errDelete) + } + assertRemoteTreePath(t, remoteDir, "master", "auths/retry.json", false) + auths, errList = store.List(context.Background()) + if errList != nil { + t.Fatalf("List after explicit Delete: %v", errList) + } + if len(auths) != 0 { + t.Fatalf("List after explicit Delete = %#v, want empty", auths) + } +} + +func TestEnsureRepositoryReconcilesRemoteAuthChangesAroundLocalConfig(t *testing.T) { + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + owner := NewGitTokenStore(remoteDir, "", "", "") + owner.SetBaseDir(filepath.Join(root, "owner", "auths")) + if errEnsure := owner.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository owner: %v", errEnsure) + } + for _, id := range []string{"modified.json", "deleted.json"} { + if _, errSave := owner.Save(context.Background(), &cliproxyauth.Auth{ + ID: id, FileName: id, Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "old"}, + }); errSave != nil { + t.Fatalf("Save owner %s: %v", id, errSave) + } + } + if errWrite := os.WriteFile(owner.ConfigPath(), []byte("source: original\n"), 0o600); errWrite != nil { + t.Fatalf("write owner config: %v", errWrite) + } + if errPersist := owner.PersistConfig(context.Background()); errPersist != nil { + t.Fatalf("PersistConfig owner: %v", errPersist) + } + + storeA := NewGitTokenStore(remoteDir, "", "", "") + storeA.SetBaseDir(filepath.Join(root, "workspace-a", "auths")) + if errEnsure := storeA.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository A: %v", errEnsure) + } + storeB := NewGitTokenStore(remoteDir, "", "", "") + storeB.SetBaseDir(filepath.Join(root, "workspace-b", "auths")) + if errEnsure := storeB.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository B: %v", errEnsure) + } + if errWrite := os.WriteFile(storeA.ConfigPath(), []byte("source: local-a\n"), 0o600); errWrite != nil { + t.Fatalf("write local config A: %v", errWrite) + } + if _, errSave := storeB.Save(context.Background(), &cliproxyauth.Auth{ + ID: "modified.json", FileName: "modified.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "new"}, + }); errSave != nil { + t.Fatalf("Save remote auth update: %v", errSave) + } + if errDelete := storeB.Delete(context.Background(), filepath.Join(storeB.AuthDir(), "deleted.json")); errDelete != nil { + t.Fatalf("Delete remote auth: %v", errDelete) + } + + if errEnsure := storeA.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository A after remote auth changes: %v", errEnsure) + } + assertLocalFileContents(t, storeA.ConfigPath(), "source: local-a\n") + assertLocalJSONValue(t, filepath.Join(storeA.AuthDir(), "modified.json"), "access_token", "new") + if _, errStat := os.Stat(filepath.Join(storeA.AuthDir(), "deleted.json")); !errors.Is(errStat, os.ErrNotExist) { + t.Fatalf("deleted local auth stat error = %v, want not exist", errStat) + } +} + +func TestEnsureRepositoryReconcilesRemoteConfigChangesAroundLocalAuth(t *testing.T) { + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + owner := NewGitTokenStore(remoteDir, "", "", "") + owner.SetBaseDir(filepath.Join(root, "owner", "auths")) + if errEnsure := owner.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository owner: %v", errEnsure) + } + if _, errSave := owner.Save(context.Background(), &cliproxyauth.Auth{ + ID: "local.json", FileName: "local.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "old"}, + }); errSave != nil { + t.Fatalf("Save owner auth: %v", errSave) + } + if errWrite := os.WriteFile(owner.ConfigPath(), []byte("source: original\n"), 0o600); errWrite != nil { + t.Fatalf("write owner config: %v", errWrite) + } + if errPersist := owner.PersistConfig(context.Background()); errPersist != nil { + t.Fatalf("PersistConfig owner: %v", errPersist) + } + + storeA := NewGitTokenStore(remoteDir, "", "", "") + storeA.SetBaseDir(filepath.Join(root, "workspace-a", "auths")) + if errEnsure := storeA.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository A: %v", errEnsure) + } + storeB := NewGitTokenStore(remoteDir, "", "", "") + storeB.SetBaseDir(filepath.Join(root, "workspace-b", "auths")) + if errEnsure := storeB.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository B: %v", errEnsure) + } + localAuthPath := filepath.Join(storeA.AuthDir(), "local.json") + localAuthContents := `{"type":"codex","access_token":"local-dirty"}` + if errWrite := os.WriteFile(localAuthPath, []byte(localAuthContents), 0o600); errWrite != nil { + t.Fatalf("write local dirty auth: %v", errWrite) + } + if errWrite := os.WriteFile(storeB.ConfigPath(), []byte("source: remote-modified\n"), 0o600); errWrite != nil { + t.Fatalf("write remote config update: %v", errWrite) + } + if errPersist := storeB.PersistConfig(context.Background()); errPersist != nil { + t.Fatalf("PersistConfig B: %v", errPersist) + } + + if errEnsure := storeA.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository A after remote config update: %v", errEnsure) + } + assertLocalFileContents(t, storeA.ConfigPath(), "source: remote-modified\n") + assertLocalFileContents(t, localAuthPath, localAuthContents) + + if errRemove := os.Remove(storeB.ConfigPath()); errRemove != nil { + t.Fatalf("remove config B: %v", errRemove) + } + storeB.mu.Lock() + errDeleteConfig := storeB.commitAndPushLocked("Delete config", "config/config.yaml") + storeB.mu.Unlock() + if errDeleteConfig != nil { + t.Fatalf("commit remote config deletion: %v", errDeleteConfig) + } + if errEnsure := storeA.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository A after remote config deletion: %v", errEnsure) + } + if _, errStat := os.Stat(storeA.ConfigPath()); !errors.Is(errStat, os.ErrNotExist) { + t.Fatalf("deleted local config stat error = %v, want not exist", errStat) + } + assertLocalFileContents(t, localAuthPath, localAuthContents) +} + +func TestEnsureRepositoryFailsClosedOnSamePathConflict(t *testing.T) { + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + owner := NewGitTokenStore(remoteDir, "", "", "") + owner.SetBaseDir(filepath.Join(root, "owner", "auths")) + if errEnsure := owner.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository owner: %v", errEnsure) + } + if errWrite := os.WriteFile(owner.ConfigPath(), []byte("source: original\n"), 0o600); errWrite != nil { + t.Fatalf("write owner config: %v", errWrite) + } + if errPersist := owner.PersistConfig(context.Background()); errPersist != nil { + t.Fatalf("PersistConfig owner: %v", errPersist) + } + + storeA := NewGitTokenStore(remoteDir, "", "", "") + storeA.SetBaseDir(filepath.Join(root, "workspace-a", "auths")) + if errEnsure := storeA.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository A: %v", errEnsure) + } + storeB := NewGitTokenStore(remoteDir, "", "", "") + storeB.SetBaseDir(filepath.Join(root, "workspace-b", "auths")) + if errEnsure := storeB.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository B: %v", errEnsure) + } + if errWrite := os.WriteFile(storeA.ConfigPath(), []byte("source: local\n"), 0o600); errWrite != nil { + t.Fatalf("write local config: %v", errWrite) + } + if errWrite := os.WriteFile(storeB.ConfigPath(), []byte("source: remote\n"), 0o600); errWrite != nil { + t.Fatalf("write remote config: %v", errWrite) + } + if errPersist := storeB.PersistConfig(context.Background()); errPersist != nil { + t.Fatalf("PersistConfig B: %v", errPersist) + } + + errEnsure := storeA.EnsureRepository() + if errEnsure == nil || !strings.Contains(errEnsure.Error(), "conflicts with local change") { + t.Fatalf("EnsureRepository conflict error = %v, want fail-closed conflict", errEnsure) + } + assertLocalFileContents(t, storeA.ConfigPath(), "source: local\n") + assertRemoteFileContents(t, remoteDir, "master", "config/config.yaml", "source: remote\n") +} + +func TestInstallRecoveredGitDirectoryRetainsBackupWhenRestoreFails(t *testing.T) { + backupPath := filepath.Join("recovery", "corrupt.git") + installErr := errors.New("install failed") + restoreErr := errors.New("restore failed") + calls := 0 + rename := func(_, _ string) error { + calls++ + switch calls { + case 1: + return nil + case 2: + return installErr + default: + return restoreErr + } + } + + retain, errInstall := installRecoveredGitDirectory("repo/.git", "clone/.git", backupPath, rename) + if !retain { + t.Fatal("retain recovery = false, want true after failed rollback") + } + if !errors.Is(errInstall, installErr) || !errors.Is(errInstall, restoreErr) { + t.Fatalf("install error = %v, want install and restore failures", errInstall) + } + if !strings.Contains(errInstall.Error(), backupPath) { + t.Fatalf("install error = %q, want retained backup path %q", errInstall, backupPath) + } +} + +func TestGitTokenStoreCorruptionRecoveryUsesLatestRemoteAuthTree(t *testing.T) { + tests := []struct { + name string + updateRemote func(*testing.T, *GitTokenStore) + wantExists bool + wantAuthToken string + }{ + { + name: "modification", + updateRemote: func(t *testing.T, store *GitTokenStore) { + t.Helper() + if _, errSave := store.Save(context.Background(), &cliproxyauth.Auth{ + ID: "victim.json", FileName: "victim.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote-new"}, + }); errSave != nil { + t.Fatalf("update remote auth: %v", errSave) + } + }, + wantExists: true, + wantAuthToken: "remote-new", + }, + { + name: "deletion", + updateRemote: func(t *testing.T, store *GitTokenStore) { + t.Helper() + if errDelete := store.Delete(context.Background(), filepath.Join(store.AuthDir(), "victim.json")); errDelete != nil { + t.Fatalf("delete remote auth: %v", errDelete) + } + }, + wantExists: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + owner := NewGitTokenStore(remoteDir, "", "", "") + owner.SetBaseDir(filepath.Join(root, "owner", "auths")) + if errEnsure := owner.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository owner: %v", errEnsure) + } + if _, errSave := owner.Save(context.Background(), &cliproxyauth.Auth{ + ID: "victim.json", FileName: "victim.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote-old"}, + }); errSave != nil { + t.Fatalf("save initial auth: %v", errSave) + } + + store := NewGitTokenStore(remoteDir, "", "", "") + store.SetBaseDir(filepath.Join(root, "workspace", "auths")) + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository workspace: %v", errEnsure) + } + test.updateRemote(t, owner) + removeHeadFileObject(t, filepath.Join(root, "workspace"), "corrupt-object.txt") + + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository recovery: %v", errEnsure) + } + victimPath := filepath.Join(store.AuthDir(), "victim.json") + if test.wantExists { + assertLocalJSONValue(t, victimPath, "access_token", test.wantAuthToken) + } else if _, errStat := os.Stat(victimPath); !errors.Is(errStat, os.ErrNotExist) { + t.Fatalf("deleted local auth stat error = %v, want not exist", errStat) + } + + if _, errSave := store.Save(context.Background(), &cliproxyauth.Auth{ + ID: "unrelated.json", FileName: "unrelated.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "local"}, + }); errSave != nil { + t.Fatalf("Save after recovery: %v", errSave) + } + assertRemoteTreePath(t, remoteDir, "master", "auths/victim.json", test.wantExists) + if test.wantExists { + assertRemoteFileContents(t, remoteDir, "master", "auths/victim.json", `{"access_token":"remote-new","disabled":false,"type":"codex"}`) + } + }) + } +} + +func TestGitTokenStoreCorruptionRecoveryPreservesOnlyNonConflictingLocalChanges(t *testing.T) { + setup := func(t *testing.T) (string, *GitTokenStore, *GitTokenStore) { + t.Helper() + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + owner := NewGitTokenStore(remoteDir, "", "", "") + owner.SetBaseDir(filepath.Join(root, "owner", "auths")) + if errEnsure := owner.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository owner: %v", errEnsure) + } + if _, errSave := owner.Save(context.Background(), &cliproxyauth.Auth{ + ID: "victim.json", FileName: "victim.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote-old"}, + }); errSave != nil { + t.Fatalf("save initial auth: %v", errSave) + } + store := NewGitTokenStore(remoteDir, "", "", "") + store.SetBaseDir(filepath.Join(root, "workspace", "auths")) + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository workspace: %v", errEnsure) + } + return filepath.Join(root, "workspace"), owner, store + } + + t.Run("non-conflicting change", func(t *testing.T) { + workspaceDir, owner, store := setup(t) + if errWrite := os.WriteFile(store.ConfigPath(), []byte("source: local\n"), 0o600); errWrite != nil { + t.Fatalf("write local config: %v", errWrite) + } + if _, errSave := owner.Save(context.Background(), &cliproxyauth.Auth{ + ID: "victim.json", FileName: "victim.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote-new"}, + }); errSave != nil { + t.Fatalf("update remote auth: %v", errSave) + } + removeHeadFileObject(t, workspaceDir, "corrupt-object.txt") + + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository recovery: %v", errEnsure) + } + assertLocalFileContents(t, store.ConfigPath(), "source: local\n") + assertLocalJSONValue(t, filepath.Join(store.AuthDir(), "victim.json"), "access_token", "remote-new") + }) + + t.Run("same-path conflict", func(t *testing.T) { + workspaceDir, owner, store := setup(t) + victimPath := filepath.Join(store.AuthDir(), "victim.json") + localContents := `{"type":"codex","access_token":"local"}` + if errWrite := os.WriteFile(victimPath, []byte(localContents), 0o600); errWrite != nil { + t.Fatalf("write local auth: %v", errWrite) + } + if _, errSave := owner.Save(context.Background(), &cliproxyauth.Auth{ + ID: "victim.json", FileName: "victim.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote-new"}, + }); errSave != nil { + t.Fatalf("update remote auth: %v", errSave) + } + removeHeadFileObject(t, workspaceDir, "corrupt-object.txt") + + errEnsure := store.EnsureRepository() + if errEnsure == nil || !strings.Contains(errEnsure.Error(), "conflicts with local change") { + t.Fatalf("EnsureRepository conflict error = %v, want fail-closed conflict", errEnsure) + } + assertLocalFileContents(t, victimPath, localContents) + assertRemoteFileContents(t, owner.remote, "master", "auths/victim.json", `{"access_token":"remote-new","disabled":false,"type":"codex"}`) + }) +} + +func TestGitTokenStoreFullPackfileCorruptionFailsClosedWithDirtyManagedFile(t *testing.T) { + setup := func(t *testing.T) (string, string, *GitTokenStore) { + t.Helper() + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + workspaceDir := filepath.Join(root, "workspace") + store := NewGitTokenStore(remoteDir, "", "", "") + store.SetBaseDir(filepath.Join(workspaceDir, "auths")) + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository: %v", errEnsure) + } + return remoteDir, workspaceDir, store + } + + t.Run("config", func(t *testing.T) { + remoteDir, workspaceDir, store := setup(t) + configPath := store.ConfigPath() + if errWrite := os.WriteFile(configPath, []byte("source: remote\n"), 0o600); errWrite != nil { + t.Fatalf("write initial config: %v", errWrite) + } + if errPersist := store.PersistConfig(context.Background()); errPersist != nil { + t.Fatalf("PersistConfig initial config: %v", errPersist) + } + + localContents := "source: local-dirty\n" + if errWrite := os.WriteFile(configPath, []byte(localContents), 0o600); errWrite != nil { + t.Fatalf("write dirty config: %v", errWrite) + } + corruptGitRepository(t, workspaceDir) + + errPersist := store.PersistConfig(context.Background()) + if errPersist == nil || !strings.Contains(errPersist.Error(), "inspect recovery baseline") { + t.Fatalf("PersistConfig error = %v, want fail-closed recovery baseline error", errPersist) + } + assertLocalFileContents(t, configPath, localContents) + assertRemoteFileContents(t, remoteDir, "master", "config/config.yaml", "source: remote\n") + }) + + t.Run("auth", func(t *testing.T) { + remoteDir, workspaceDir, store := setup(t) + authPath, errSave := store.Save(context.Background(), &cliproxyauth.Auth{ + ID: "dirty.json", FileName: "dirty.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote"}, + }) + if errSave != nil { + t.Fatalf("Save initial auth: %v", errSave) + } + + localContents := `{"type":"codex","access_token":"local-dirty"}` + if errWrite := os.WriteFile(authPath, []byte(localContents), 0o600); errWrite != nil { + t.Fatalf("write dirty auth: %v", errWrite) + } + corruptGitRepository(t, workspaceDir) + + _, errSave = store.Save(context.Background(), &cliproxyauth.Auth{ + ID: "unrelated.json", FileName: "unrelated.json", Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "unrelated"}, + }) + if errSave == nil || !strings.Contains(errSave.Error(), "inspect recovery baseline") { + t.Fatalf("Save error = %v, want fail-closed recovery baseline error", errSave) + } + assertLocalFileContents(t, authPath, localContents) + assertRemoteFileContents(t, remoteDir, "master", "auths/dirty.json", `{"access_token":"remote","disabled":false,"type":"codex"}`) + assertRemoteTreePath(t, remoteDir, "master", "auths/unrelated.json", false) + }) +} + +func TestGitTokenStoreMissingPackfileRecoveryFailsClosedWithoutBaseline(t *testing.T) { + root := t.TempDir() + remoteDir := setupGitRemoteRepository(t, root, "master", + testBranchSpec{name: "master", contents: "remote master branch\n"}, + ) + store := NewGitTokenStore(remoteDir, "", "", "") + baseDir := filepath.Join(root, "workspace", "auths") + store.SetBaseDir(baseDir) + if errEnsure := store.EnsureRepository(); errEnsure != nil { + t.Fatalf("EnsureRepository: %v", errEnsure) + } + auth := &cliproxyauth.Auth{ + ID: "recover.json", + FileName: "recover.json", + Provider: "codex", + Metadata: map[string]any{"type": "codex", "access_token": "remote"}, + } + authPath, errSave := store.Save(context.Background(), auth) + if errSave != nil { + t.Fatalf("Save: %v", errSave) + } + + repo := corruptGitRepository(t, filepath.Join(root, "workspace")) + if errRemove := os.Remove(authPath); errRemove != nil { + t.Fatalf("remove local auth before recovery: %v", errRemove) + } + if errVerify := verifyRepositoryHead(repo); !isRepositoryCorruptionError(errVerify) { + t.Fatalf("verifyRepositoryHead error = %v, want repository corruption", errVerify) + } + + errEnsure := store.EnsureRepository() + if errEnsure == nil || !strings.Contains(errEnsure.Error(), "inspect recovery baseline") { + t.Fatalf("EnsureRepository error = %v, want fail-closed recovery baseline error", errEnsure) + } + if _, errStat := os.Stat(authPath); !errors.Is(errStat, os.ErrNotExist) { + t.Fatalf("local deleted auth stat error = %v, want not exist", errStat) + } + assertRemoteTreePath(t, remoteDir, "master", "auths/recover.json", true) +} + func TestCommitAndPushLockedPushesBeforeRunningGC(t *testing.T) { root := t.TempDir() remoteDir := setupGitRemoteRepository(t, root, "master", @@ -482,6 +1396,91 @@ func TestEnsureRepositoryKeepsCurrentBranchWhenRemoteDefaultCannotBeResolved(t * assertRepositoryHeadBranch(t, filepath.Join(root, "workspace"), "develop") } +func removeHeadFileObject(t *testing.T, repoDir, path string) { + t.Helper() + + repo, errOpen := git.PlainOpen(repoDir) + if errOpen != nil { + t.Fatalf("open repository before object removal: %v", errOpen) + } + worktree, errWorktree := repo.Worktree() + if errWorktree != nil { + t.Fatalf("open worktree before object removal: %v", errWorktree) + } + fullPath := filepath.Join(repoDir, filepath.FromSlash(path)) + if errWrite := os.WriteFile(fullPath, []byte("corrupt me\n"), 0o600); errWrite != nil { + t.Fatalf("write corruption marker: %v", errWrite) + } + if _, errAdd := worktree.Add(path); errAdd != nil { + t.Fatalf("add corruption marker: %v", errAdd) + } + if _, errCommit := worktree.Commit("Add corruption marker", &git.CommitOptions{Author: &object.Signature{ + Name: "CLIProxyAPI", Email: "cliproxy@local", When: time.Unix(1711929600, 0), + }}); errCommit != nil { + t.Fatalf("commit corruption marker: %v", errCommit) + } + head, errHead := repo.Head() + if errHead != nil { + t.Fatalf("read repository head: %v", errHead) + } + commit, errCommit := repo.CommitObject(head.Hash()) + if errCommit != nil { + t.Fatalf("read repository commit: %v", errCommit) + } + tree, errTree := commit.Tree() + if errTree != nil { + t.Fatalf("read repository tree: %v", errTree) + } + file, errFile := tree.File(path) + if errFile != nil { + t.Fatalf("read repository file %s: %v", path, errFile) + } + objectPath := filepath.Join(repoDir, ".git", "objects", file.Hash.String()[:2], file.Hash.String()[2:]) + if errRemove := os.Remove(objectPath); errRemove != nil { + t.Fatalf("remove repository object for %s: %v", path, errRemove) + } + if errVerify := verifyRepositoryHead(repo); !isRepositoryCorruptionError(errVerify) { + t.Fatalf("verifyRepositoryHead error = %v, want repository corruption", errVerify) + } +} + +func corruptGitRepository(t *testing.T, repoDir string) *git.Repository { + t.Helper() + + repo, errOpen := git.PlainOpen(repoDir) + if errOpen != nil { + t.Fatalf("open repository before corruption: %v", errOpen) + } + if errRepack := repo.RepackObjects(&git.RepackConfig{}); errRepack != nil { + t.Fatalf("repack repository objects: %v", errRepack) + } + objectsDir := filepath.Join(repoDir, ".git", "objects") + objectEntries, errReadDir := os.ReadDir(objectsDir) + if errReadDir != nil { + t.Fatalf("read object directory: %v", errReadDir) + } + for _, entry := range objectEntries { + if entry.IsDir() && len(entry.Name()) == 2 { + if errRemove := os.RemoveAll(filepath.Join(objectsDir, entry.Name())); errRemove != nil { + t.Fatalf("remove loose object directory %s: %v", entry.Name(), errRemove) + } + } + } + packfiles, errGlob := filepath.Glob(filepath.Join(objectsDir, "pack", "*.pack")) + if errGlob != nil { + t.Fatalf("glob packfiles: %v", errGlob) + } + if len(packfiles) == 0 { + t.Fatal("no packfiles found to corrupt") + } + for _, packfile := range packfiles { + if errRemove := os.Remove(packfile); errRemove != nil { + t.Fatalf("remove packfile %s: %v", filepath.Base(packfile), errRemove) + } + } + return repo +} + func setupGitRemoteRepository(t *testing.T, root, defaultBranch string, branches ...testBranchSpec) string { t.Helper() @@ -634,6 +1633,34 @@ func findBranchSpec(branches []testBranchSpec, name string) (testBranchSpec, boo return testBranchSpec{}, false } +func assertLocalFileContents(t *testing.T, path, wantContents string) { + t.Helper() + + contents, errRead := os.ReadFile(path) + if errRead != nil { + t.Fatalf("read local file %s: %v", path, errRead) + } + if string(contents) != wantContents { + t.Fatalf("local file %s contents = %q, want %q", path, contents, wantContents) + } +} + +func assertLocalJSONValue(t *testing.T, path, key, wantValue string) { + t.Helper() + + contents, errRead := os.ReadFile(path) + if errRead != nil { + t.Fatalf("read local JSON file %s: %v", path, errRead) + } + metadata := make(map[string]any) + if errUnmarshal := json.Unmarshal(contents, &metadata); errUnmarshal != nil { + t.Fatalf("unmarshal local JSON file %s: %v", path, errUnmarshal) + } + if gotValue, _ := metadata[key].(string); gotValue != wantValue { + t.Fatalf("local JSON file %s value %s = %q, want %q", path, key, gotValue, wantValue) + } +} + func assertRemoteTreePath(t *testing.T, remoteDir, branch, path string, want bool) { t.Helper() @@ -663,6 +1690,38 @@ func assertRemoteTreePath(t *testing.T, remoteDir, branch, path string, want boo } } +func assertRemoteFileContents(t *testing.T, remoteDir, branch, path, wantContents string) { + t.Helper() + + repo, err := git.PlainOpen(remoteDir) + if err != nil { + t.Fatalf("open remote repo: %v", err) + } + ref, err := repo.Reference(plumbing.NewBranchReferenceName(branch), true) + if err != nil { + t.Fatalf("read remote branch %s: %v", branch, err) + } + commit, err := repo.CommitObject(ref.Hash()) + if err != nil { + t.Fatalf("read remote commit: %v", err) + } + tree, err := commit.Tree() + if err != nil { + t.Fatalf("read remote tree: %v", err) + } + file, err := tree.File(filepath.ToSlash(path)) + if err != nil { + t.Fatalf("read remote file %s: %v", path, err) + } + contents, err := file.Contents() + if err != nil { + t.Fatalf("read remote file %s contents: %v", path, err) + } + if contents != wantContents { + t.Fatalf("remote file %s contents = %q, want %q", path, contents, wantContents) + } +} + func assertRepositoryBranchAndContents(t *testing.T, repoDir, branch, wantContents string) { t.Helper() diff --git a/internal/translator/antigravity/openai/chat-completions/antigravity_openai_request.go b/internal/translator/antigravity/openai/chat-completions/antigravity_openai_request.go index c0a953e5..6c99515f 100644 --- a/internal/translator/antigravity/openai/chat-completions/antigravity_openai_request.go +++ b/internal/translator/antigravity/openai/chat-completions/antigravity_openai_request.go @@ -82,10 +82,10 @@ func ConvertOpenAIRequestToAntigravity(modelName string, inputRawJSON []byte, _ out, _ = sjson.DeleteBytes(out, "request.generationConfig."+schemaKey) } out, _ = sjson.SetBytes(out, "request.generationConfig.responseMimeType", "application/json") - if responseFormatType == "json_object" { - out, _ = sjson.SetRawBytes(out, "request.generationConfig.responseSchema", []byte(`{"type":"object"}`)) - } else if schema := responseFormat.Get("json_schema.schema"); schema.Exists() { - out, _ = sjson.SetRawBytes(out, "request.generationConfig.responseSchema", []byte(schema.Raw)) + if responseFormatType == "json_schema" { + if schema := responseFormat.Get("json_schema.schema"); schema.Exists() { + out, _ = sjson.SetRawBytes(out, "request.generationConfig.responseSchema", []byte(schema.Raw)) + } } } } diff --git a/internal/translator/antigravity/openai/chat-completions/antigravity_openai_request_test.go b/internal/translator/antigravity/openai/chat-completions/antigravity_openai_request_test.go index 845e7b63..81907cbb 100644 --- a/internal/translator/antigravity/openai/chat-completions/antigravity_openai_request_test.go +++ b/internal/translator/antigravity/openai/chat-completions/antigravity_openai_request_test.go @@ -332,12 +332,8 @@ func TestConvertOpenAIRequestToAntigravityMapsResponseFormatJSONObject(t *testin if got := gjson.GetBytes(out, "request.generationConfig.responseMimeType").String(); got != "application/json" { t.Fatalf("responseMimeType = %q, want application/json. Output: %s", got, out) } - schema := gjson.GetBytes(out, "request.generationConfig.responseSchema") - if got := schema.Get("type").String(); got != "object" { - t.Fatalf("responseSchema.type = %q, want object. Output: %s", got, out) - } - if schema.Get("description").Exists() { - t.Fatalf("stale responseSchema survived. Output: %s", out) + if gjson.GetBytes(out, "request.generationConfig.responseSchema").Exists() { + t.Fatalf("responseSchema should not be set for json_object. Output: %s", out) } assertNoResponseSchemaAliases(t, out) } diff --git a/internal/util/gemini_schema.go b/internal/util/gemini_schema.go index 51a414f7..f3b99d29 100644 --- a/internal/util/gemini_schema.go +++ b/internal/util/gemini_schema.go @@ -24,50 +24,67 @@ const placeholderReasonDescription = "Brief explanation of why you are calling t // and replacements such as "enum" and "type" are fabricated. That regression reached production // once already; scope every call site to the schema itself. +type jsonSchemaCleanOptions struct { + addPlaceholder bool + removeGeminiMetadata bool + flattenUnions bool + forceEnumStringType bool +} + // CleanJSONSchemaForAntigravity transforms a tool schema to be compatible with Antigravity API. // It handles unsupported keywords, type flattening, and schema simplification while preserving // semantic information as description hints and adding placeholders required by VALIDATED mode. func CleanJSONSchemaForAntigravity(jsonStr string) string { - return cleanJSONSchema(jsonStr, true, false) + return cleanJSONSchema(jsonStr, jsonSchemaCleanOptions{ + addPlaceholder: true, + flattenUnions: true, + forceEnumStringType: true, + }) } -// CleanJSONSchemaForAntigravityResponse transforms a response schema without adding tool-only -// placeholders that would alter the client's structured output contract. +// CleanJSONSchemaForAntigravityResponse transforms a response schema without applying tool-only +// compatibility rewrites that would alter the client's structured output contract. func CleanJSONSchemaForAntigravityResponse(jsonStr string) string { - return cleanJSONSchema(jsonStr, false, false) + return cleanJSONSchema(jsonStr, jsonSchemaCleanOptions{}) } // CleanJSONSchemaForGemini transforms a JSON schema to be compatible with Gemini tool calling. // It removes unsupported keywords and simplifies schemas, without adding empty-schema placeholders. func CleanJSONSchemaForGemini(jsonStr string) string { - return cleanJSONSchema(jsonStr, false, true) + return cleanJSONSchema(jsonStr, jsonSchemaCleanOptions{ + removeGeminiMetadata: true, + flattenUnions: true, + forceEnumStringType: true, + }) } // cleanJSONSchema performs the core cleaning operations on the JSON schema. -func cleanJSONSchema(jsonStr string, addPlaceholder, removeGeminiMetadata bool) string { +func cleanJSONSchema(jsonStr string, options jsonSchemaCleanOptions) string { // Phase 1: Convert and add hints jsonStr = convertRefsToHints(jsonStr) jsonStr = convertConstToEnum(jsonStr) - jsonStr = convertEnumValuesToStrings(jsonStr) + jsonStr = convertEnumValuesToStrings(jsonStr, options.forceEnumStringType) jsonStr = addEnumHints(jsonStr) jsonStr = addAdditionalPropertiesHints(jsonStr) jsonStr = moveConstraintsToDescription(jsonStr) // Phase 2: Flatten complex structures jsonStr = mergeAllOf(jsonStr) - jsonStr = flattenAnyOfOneOf(jsonStr) + if options.flattenUnions { + jsonStr = flattenAnyOfOneOf(jsonStr) + } jsonStr = flattenTypeArrays(jsonStr) // Phase 3: Cleanup jsonStr = removeUnsupportedKeywords(jsonStr) - if removeGeminiMetadata { + if options.removeGeminiMetadata { // Gemini schema cleanup: remove nullable/title and placeholder-only fields. jsonStr = removeKeywords(jsonStr, []string{"nullable", "title"}) jsonStr = removePlaceholderFields(jsonStr) } jsonStr = cleanupRequiredFields(jsonStr) // Phase 4: Add placeholder for empty object schemas (Claude VALIDATED mode requirement) - if addPlaceholder { + if options.addPlaceholder { jsonStr = addEmptySchemaPlaceholder(jsonStr) } @@ -201,9 +218,10 @@ func convertConstToEnum(jsonStr string) string { return jsonStr } -// convertEnumValuesToStrings ensures all enum values are strings and the schema type is set to string. -// Gemini API requires enum values to be of type string, not numbers or booleans. -func convertEnumValuesToStrings(jsonStr string) string { +// convertEnumValuesToStrings ensures all enum values use the string representation required by +// Gemini's proto schema. Tool schemas also require a string type, while response schemas preserve +// their declared type because the upstream decoder uses it to select the emitted JSON value type. +func convertEnumValuesToStrings(jsonStr string, forceStringType bool) string { for _, p := range findPaths(jsonStr, "enum") { arr := gjson.Get(jsonStr, p) if !arr.IsArray() { @@ -215,13 +233,13 @@ func convertEnumValuesToStrings(jsonStr string) string { stringVals = append(stringVals, item.String()) } - // Always update enum values to strings and set type to "string" - // This ensures compatibility with Antigravity Gemini which only allows enum for STRING type updated, _ := sjson.SetBytes([]byte(jsonStr), p, stringVals) jsonStr = string(updated) - parentPath := trimSuffix(p, ".enum") - updated, _ = sjson.SetBytes([]byte(jsonStr), joinPath(parentPath, "type"), "string") - jsonStr = string(updated) + if forceStringType { + parentPath := trimSuffix(p, ".enum") + updated, _ = sjson.SetBytes([]byte(jsonStr), joinPath(parentPath, "type"), "string") + jsonStr = string(updated) + } } return jsonStr } diff --git a/internal/util/gemini_schema_test.go b/internal/util/gemini_schema_test.go index 20d10b4d..50a66bbd 100644 --- a/internal/util/gemini_schema_test.go +++ b/internal/util/gemini_schema_test.go @@ -764,6 +764,76 @@ func TestCleanJSONSchemaForAntigravityResponseDoesNotAddToolPlaceholders(t *test } } +func TestCleanJSONSchemaForAntigravityResponsePreservesUnions(t *testing.T) { + input := `{ + "type":"object", + "properties":{ + "action":{"anyOf":[ + {"type":"object","properties":{"name":{"type":"string"}},"required":["name"]}, + {"type":"null"} + ]}, + "label":{"oneOf":[{"type":"string"},{"type":"null"}]} + } + }` + + result := gjson.Parse(CleanJSONSchemaForAntigravityResponse(input)) + for _, testCase := range []struct { + path string + wantTypes []string + }{ + {path: "properties.action.anyOf", wantTypes: []string{"object", "null"}}, + {path: "properties.label.oneOf", wantTypes: []string{"string", "null"}}, + } { + union := result.Get(testCase.path) + if !union.IsArray() { + t.Errorf("response union %s was flattened: %s", testCase.path, result.Raw) + continue + } + var gotTypes []string + for _, branch := range union.Array() { + gotTypes = append(gotTypes, branch.Get("type").String()) + } + if !reflect.DeepEqual(gotTypes, testCase.wantTypes) { + t.Errorf("response union %s types = %v, want %v: %s", testCase.path, gotTypes, testCase.wantTypes, result.Raw) + } + } +} + +func TestCleanJSONSchemaForAntigravityResponsePreservesEnumType(t *testing.T) { + input := `{ + "type":"object", + "properties":{ + "conviction":{"type":"number","enum":[0.25,0.5,1]}, + "count":{"type":"integer","enum":[1,2]} + } + }` + + result := gjson.Parse(CleanJSONSchemaForAntigravityResponse(input)) + for _, testCase := range []struct { + path string + wantType string + wantValues []string + }{ + {path: "properties.conviction", wantType: "number", wantValues: []string{"0.25", "0.5", "1"}}, + {path: "properties.count", wantType: "integer", wantValues: []string{"1", "2"}}, + } { + schema := result.Get(testCase.path) + if gotType := schema.Get("type").String(); gotType != testCase.wantType { + t.Errorf("%s type = %q, want %q: %s", testCase.path, gotType, testCase.wantType, result.Raw) + } + var gotValues []string + for _, enumValue := range schema.Get("enum").Array() { + if enumValue.Type != gjson.String { + t.Errorf("%s enum value is not a string: %s", testCase.path, enumValue.Raw) + } + gotValues = append(gotValues, enumValue.String()) + } + if !reflect.DeepEqual(gotValues, testCase.wantValues) { + t.Errorf("%s enum values = %v, want %v: %s", testCase.path, gotValues, testCase.wantValues, result.Raw) + } + } +} + // ============================================================================ // Format field handling (ad-hoc patch removal) // ============================================================================ @@ -862,6 +932,14 @@ func TestCleanJSONSchemaForAntigravity_NumericEnumToString(t *testing.T) { }` result := CleanJSONSchemaForAntigravity(input) + parsed := gjson.Parse(result) + + // Tool enum schemas require both string values and a string type. + for _, path := range []string{"properties.priority", "properties.level"} { + if gotType := parsed.Get(path + ".type").String(); gotType != "string" { + t.Errorf("Tool enum type at %s = %q, want string: %s", path, gotType, result) + } + } // Numeric enum values should be converted to strings if strings.Contains(result, `"enum":[0,1,2]`) { diff --git a/sdk/cliproxy/config_model_display_name_test.go b/sdk/cliproxy/config_model_display_name_test.go index 452dbae1..f7e78dc9 100644 --- a/sdk/cliproxy/config_model_display_name_test.go +++ b/sdk/cliproxy/config_model_display_name_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + "github.com/router-for-me/CLIProxyAPI/v7/internal/registry" ) func TestBuildConfigModelsDisplayName(t *testing.T) { @@ -68,31 +69,32 @@ func TestBuildConfigModelsDisplayName(t *testing.T) { } } -func TestBuildCodexConfigModelsPreservesBuiltinDisplayNames(t *testing.T) { - models := buildCodexConfigModels(&config.CodexKey{Models: []config.CodexModel{ - {Name: "gpt-image-1.5", DisplayName: "Configured Image 1.5"}, - {Name: "gpt-image-2", DisplayName: "Configured Image 2"}, - }}) +func TestBuildCodexConfigModelsSelectsDefaultsOrConfiguredModels(t *testing.T) { + configured := buildCodexConfigModels(&config.CodexKey{Models: []config.CodexModel{{ + Name: "upstream-codex", Alias: "configured-codex", + }}}) + if len(configured) != 1 { + t.Fatalf("configured model count = %d, want 1", len(configured)) + } + if configured[0].ID != "configured-codex" { + t.Fatalf("configured model ID = %q, want configured-codex", configured[0].ID) + } - wantDisplayNames := map[string]string{ - "gpt-image-1.5": "Configured Image 1.5", - "gpt-image-2": "Configured Image 2", + defaults := buildCodexConfigModels(&config.CodexKey{}) + wantDefaults := registry.GetCodexProModels() + if len(defaults) != len(wantDefaults) { + t.Fatalf("default model count = %d, want %d", len(defaults), len(wantDefaults)) } - for _, model := range models { - wantDisplayName, ok := wantDisplayNames[model.ID] - if !ok { - continue - } - if model.DisplayName != wantDisplayName { - t.Errorf("%s DisplayName = %q, want %q", model.ID, model.DisplayName, wantDisplayName) + defaultIDs := make(map[string]struct{}, len(defaults)) + for _, model := range defaults { + if model != nil { + defaultIDs[model.ID] = struct{}{} } - if model.Object != "model" || model.OwnedBy != "openai" || model.Type != "openai" || model.Created != 1704067200 || model.Version != model.ID || model.UserDefined { - t.Errorf("%s builtin metadata was not preserved: %#v", model.ID, model) - } - delete(wantDisplayNames, model.ID) } - for modelID := range wantDisplayNames { - t.Errorf("missing builtin model %s", modelID) + for _, modelID := range []string{"gpt-image-1.5", "gpt-image-2"} { + if _, ok := defaultIDs[modelID]; !ok { + t.Errorf("missing default model %q", modelID) + } } } diff --git a/sdk/cliproxy/service_codex_models_test.go b/sdk/cliproxy/service_codex_models_test.go new file mode 100644 index 00000000..ae5abc34 --- /dev/null +++ b/sdk/cliproxy/service_codex_models_test.go @@ -0,0 +1,281 @@ +package cliproxy + +import ( + "context" + "fmt" + "testing" + + internalconfig "github.com/router-for-me/CLIProxyAPI/v7/internal/config" + internalregistry "github.com/router-for-me/CLIProxyAPI/v7/internal/registry" + coreauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth" + "github.com/router-for-me/CLIProxyAPI/v7/sdk/config" +) + +func TestRegisterModelsForAuthCodexAPIKeyModels(t *testing.T) { + defaultModels := internalregistry.GetCodexProModels() + if len(defaultModels) == 0 { + t.Fatal("expected Codex Pro default models") + } + + excludedModelID := defaultModels[0].ID + tests := []struct { + name string + entry config.CodexKey + wantIDs map[string]struct{} + wantPresent []string + wantAbsent []string + }{ + { + name: "defaults without explicit models", + entry: config.CodexKey{APIKey: "default-key"}, + wantIDs: codexModelIDSet(defaultModels), + wantPresent: []string{"gpt-image-1.5", "gpt-image-2"}, + }, + { + name: "only explicitly configured models", + entry: config.CodexKey{ + APIKey: "configured-key", + Models: []internalconfig.CodexModel{{ + Name: "upstream-codex", Alias: "configured-codex", + }}, + }, + wantIDs: map[string]struct{}{"configured-codex": {}}, + wantAbsent: []string{"gpt-image-1.5", "gpt-image-2"}, + }, + { + name: "exclusions apply to defaults", + entry: config.CodexKey{ + APIKey: "excluded-key", + ExcludedModels: []string{excludedModelID}, + }, + wantIDs: codexModelIDSet(defaultModels[1:]), + }, + } + + for index := range tests { + testCase := tests[index] + t.Run(testCase.name, func(t *testing.T) { + authID := fmt.Sprintf("codex-api-key-models-%d", index) + modelRegistry := internalregistry.GetGlobalRegistry() + modelRegistry.UnregisterClient(authID) + t.Cleanup(func() { modelRegistry.UnregisterClient(authID) }) + + service := &Service{cfg: &config.Config{CodexKey: []config.CodexKey{testCase.entry}}} + auth := &coreauth.Auth{ + ID: authID, + Provider: "codex", + Status: coreauth.StatusActive, + Attributes: map[string]string{ + coreauth.AttributeAPIKey: testCase.entry.APIKey, + coreauth.AttributeConfigIndex: "0", + coreauth.AttributeSource: "config:codex:test", + }, + } + + service.registerModelsForAuth(context.Background(), auth) + gotIDs := codexModelIDSet(modelRegistry.GetModelsForClient(authID)) + if len(gotIDs) != len(testCase.wantIDs) { + t.Fatalf("registered model IDs = %#v, want %#v", gotIDs, testCase.wantIDs) + } + for modelID := range testCase.wantIDs { + if _, ok := gotIDs[modelID]; !ok { + t.Errorf("missing registered model %q", modelID) + } + } + for _, modelID := range testCase.wantPresent { + if _, ok := gotIDs[modelID]; !ok { + t.Errorf("missing required registered model %q", modelID) + } + } + for _, modelID := range testCase.wantAbsent { + if _, ok := gotIDs[modelID]; ok { + t.Errorf("unexpected registered model %q", modelID) + } + } + }) + } +} + +func TestRegisterModelsForAuthCodexAPIKeyDefaultRequiresConfigMatch(t *testing.T) { + defaultIDs := codexModelIDSet(internalregistry.GetCodexProModels()) + tests := []struct { + name string + config config.Config + attributes map[string]string + wantIDs map[string]struct{} + }{ + { + name: "valid index with unmatched API key", + config: config.Config{CodexKey: []config.CodexKey{{ + APIKey: "configured-key", + }}}, + attributes: map[string]string{ + coreauth.AttributeAPIKey: "stale-key", + coreauth.AttributeConfigIndex: "0", + coreauth.AttributeSource: "config:codex:stale", + }, + wantIDs: map[string]struct{}{}, + }, + { + name: "valid index with unmatched base URL", + config: config.Config{CodexKey: []config.CodexKey{{ + APIKey: "configured-key", BaseURL: "https://new.example.com", + }}}, + attributes: map[string]string{ + coreauth.AttributeAPIKey: "configured-key", + coreauth.AttributeConfigIndex: "0", + coreauth.AttributeSource: "config:codex:stale", + "base_url": "https://old.example.com", + }, + wantIDs: map[string]struct{}{}, + }, + { + name: "stale index falls back to matching credentials", + config: config.Config{CodexKey: []config.CodexKey{ + { + APIKey: "wrong-key", + Models: []internalconfig.CodexModel{{Name: "wrong-model"}}, + }, + {APIKey: "configured-key"}, + }}, + attributes: map[string]string{ + coreauth.AttributeAPIKey: "configured-key", + coreauth.AttributeConfigIndex: "0", + coreauth.AttributeSource: "config:codex:stale", + }, + wantIDs: defaultIDs, + }, + { + name: "API key ignores OAuth plan type", + config: config.Config{CodexKey: []config.CodexKey{{ + APIKey: "configured-key", + }}}, + attributes: map[string]string{ + coreauth.AttributeAPIKey: "configured-key", + coreauth.AttributeConfigIndex: "0", + coreauth.AttributeSource: "config:codex:test", + "plan_type": "free", + }, + wantIDs: defaultIDs, + }, + } + + for index := range tests { + testCase := tests[index] + t.Run(testCase.name, func(t *testing.T) { + authID := fmt.Sprintf("codex-api-key-config-match-%d", index) + modelRegistry := internalregistry.GetGlobalRegistry() + modelRegistry.UnregisterClient(authID) + modelRegistry.RegisterClient(authID, "codex", []*internalregistry.ModelInfo{{ID: "stale-model"}}) + t.Cleanup(func() { modelRegistry.UnregisterClient(authID) }) + + service := &Service{cfg: &testCase.config} + auth := &coreauth.Auth{ + ID: authID, + Provider: "codex", + Status: coreauth.StatusActive, + Attributes: testCase.attributes, + } + + service.registerModelsForAuth(context.Background(), auth) + gotIDs := codexModelIDSet(modelRegistry.GetModelsForClient(authID)) + if len(gotIDs) != len(testCase.wantIDs) { + t.Fatalf("registered model IDs = %#v, want %#v", gotIDs, testCase.wantIDs) + } + for modelID := range testCase.wantIDs { + if _, ok := gotIDs[modelID]; !ok { + t.Errorf("missing registered model %q", modelID) + } + } + }) + } +} + +func TestRegisterConfigAPIKeyAuthsCodexModelModes(t *testing.T) { + defaultIDs := codexModelIDSet(internalregistry.GetCodexProModels()) + tests := []struct { + name string + models []internalconfig.CodexModel + wantIDs map[string]struct{} + wantImages bool + }{ + { + name: "empty models uses defaults with images", + wantIDs: defaultIDs, + wantImages: true, + }, + { + name: "configured models replace defaults", + models: []internalconfig.CodexModel{{ + Name: "runtime-upstream", Alias: "runtime-configured", + }}, + wantIDs: map[string]struct{}{"runtime-configured": {}}, + }, + } + + for index := range tests { + testCase := tests[index] + t.Run(testCase.name, func(t *testing.T) { + cfg := &config.Config{CodexKey: []config.CodexKey{{ + APIKey: fmt.Sprintf("runtime-key-%d", index), + Models: testCase.models, + }}} + manager := coreauth.NewManager(nil, nil, nil) + service := &Service{cfg: cfg, coreManager: manager} + service.registerConfigAPIKeyAuths(context.Background(), cfg) + + auths := manager.List() + modelRegistry := internalregistry.GetGlobalRegistry() + for _, auth := range auths { + if auth != nil { + authID := auth.ID + t.Cleanup(func() { modelRegistry.UnregisterClient(authID) }) + } + } + if len(auths) != 1 { + t.Fatalf("runtime auth count = %d, want 1", len(auths)) + } + + registeredIDs := codexModelIDSet(modelRegistry.GetModelsForClient(auths[0].ID)) + if len(registeredIDs) != len(testCase.wantIDs) { + t.Fatalf("registered model IDs = %#v, want %#v", registeredIDs, testCase.wantIDs) + } + for modelID := range testCase.wantIDs { + if _, ok := registeredIDs[modelID]; !ok { + t.Errorf("missing registered model %q", modelID) + } + } + for _, modelID := range []string{"gpt-image-1.5", "gpt-image-2"} { + _, registered := registeredIDs[modelID] + if registered != testCase.wantImages { + t.Errorf("registered model %q = %t, want %t", modelID, registered, testCase.wantImages) + } + if testCase.wantImages { + if _, available := openAIModelIDSet(modelRegistry.GetAvailableModels("openai"))[modelID]; !available { + t.Errorf("/v1/models source is missing %q", modelID) + } + } + } + }) + } +} + +func codexModelIDSet(models []*internalregistry.ModelInfo) map[string]struct{} { + ids := make(map[string]struct{}, len(models)) + for _, model := range models { + if model != nil && model.ID != "" { + ids[model.ID] = struct{}{} + } + } + return ids +} + +func openAIModelIDSet(models []map[string]any) map[string]struct{} { + ids := make(map[string]struct{}, len(models)) + for _, model := range models { + if modelID, ok := model["id"].(string); ok && modelID != "" { + ids[modelID] = struct{}{} + } + } + return ids +} diff --git a/sdk/cliproxy/service_models.go b/sdk/cliproxy/service_models.go index 3ae86e26..474c8ce6 100644 --- a/sdk/cliproxy/service_models.go +++ b/sdk/cliproxy/service_models.go @@ -111,6 +111,15 @@ func (s *Service) registerModelsForAuthWithCache(ctx context.Context, a *coreaut } models = applyExcludedModels(models, excluded) case "codex": + if authKind == "apikey" { + if entry := s.resolveConfigCodexKey(a); entry != nil { + models = buildCodexConfigModels(entry) + excluded = entry.ExcludedModels + } + models = applyExcludedModels(models, excluded) + break + } + codexPlanType := "" if a.Attributes != nil { codexPlanType = strings.TrimSpace(a.Attributes["plan_type"]) @@ -127,14 +136,6 @@ func (s *Service) registerModelsForAuthWithCache(ctx context.Context, a *coreaut default: models = registry.GetCodexProModels() } - if entry := s.resolveConfigCodexKey(a); entry != nil { - if len(entry.Models) > 0 { - models = buildCodexConfigModels(entry) - } - if authKind == "apikey" { - excluded = entry.ExcludedModels - } - } models = applyExcludedModels(models, excluded) case "kimi": models = registry.GetKimiModels() @@ -493,39 +494,41 @@ func (s *Service) resolveConfigCodexKey(auth *coreauth.Auth) *config.CodexKey { if s == nil || s.cfg == nil { return nil } - return resolveConfigCodexStyleKey(auth, s.cfg.CodexKey) + return resolveConfigCodexStyleKey(auth, s.cfg.CodexKey, true) } func (s *Service) resolveConfigXAIKey(auth *coreauth.Auth) *config.XAIKey { if s == nil || s.cfg == nil { return nil } - return resolveConfigCodexStyleKey(auth, s.cfg.XAIKey) + return resolveConfigCodexStyleKey(auth, s.cfg.XAIKey, false) } -func resolveConfigCodexStyleKey(auth *coreauth.Auth, entries []config.CodexKey) *config.CodexKey { +func resolveConfigCodexStyleKey(auth *coreauth.Auth, entries []config.CodexKey, validateIndexCredentials bool) *config.CodexKey { if auth == nil { return nil } - if entry := configEntryForAuthIndex(auth, entries); entry != nil { - return entry - } var attrKey, attrBase string if auth.Attributes != nil { attrKey = strings.TrimSpace(auth.Attributes["api_key"]) attrBase = strings.TrimSpace(auth.Attributes["base_url"]) } - for i := range entries { - entry := &entries[i] + matchesCredentials := func(entry *config.CodexKey) bool { + if entry == nil { + return false + } cfgKey := strings.TrimSpace(entry.APIKey) cfgBase := strings.TrimSpace(entry.BaseURL) - if attrKey != "" && strings.EqualFold(cfgKey, attrKey) { - if cfgBase == "" || strings.EqualFold(cfgBase, attrBase) { - return entry - } - continue + if attrKey != "" { + return strings.EqualFold(cfgKey, attrKey) && (cfgBase == "" || strings.EqualFold(cfgBase, attrBase)) } - if attrKey == "" && attrBase != "" && strings.EqualFold(cfgBase, attrBase) { + return attrBase != "" && strings.EqualFold(cfgBase, attrBase) + } + if entry := configEntryForAuthIndex(auth, entries); entry != nil && (!validateIndexCredentials || matchesCredentials(entry)) { + return entry + } + for i := range entries { + if entry := &entries[i]; matchesCredentials(entry) { return entry } } @@ -852,8 +855,11 @@ func buildCodexConfigModels(entry *config.CodexKey) []*ModelInfo { if entry == nil { return nil } + if len(entry.Models) == 0 { + return registry.GetCodexProModels() + } - models := registry.WithCodexBuiltins(buildConfigModels(entry.Models, "openai", "openai")) + models := buildConfigModels(entry.Models, "openai", "openai") configuredDisplayNames := make(map[string]string, len(entry.Models)) seenConfiguredModels := make(map[string]struct{}, len(entry.Models)) for i := range entry.Models {