Skip to content

Comments

feat: deprecate OpenAI config in favor of LiteLLM agyn key#1263

Open
casey-brooks wants to merge 3 commits intomainfrom
noa/issue-1259
Open

feat: deprecate OpenAI config in favor of LiteLLM agyn key#1263
casey-brooks wants to merge 3 commits intomainfrom
noa/issue-1259

Conversation

@casey-brooks
Copy link
Contributor

Summary

  • remove OpenAI provisioner and refactor LLM settings to rely on LiteLLM master key and the managed agyn_key alias
  • replace /api/nix/resolve-repo GitHub API usage with git ls-remote and preserve error semantics for repo/ref resolution
  • update context item sanitization to drop guard envs, refresh docs/tests for LiteLLM-only configuration

Testing

  • LOG_LEVEL=error pnpm --filter @agyn/platform-server exec vitest run --reporter=json --outputFile ../../vitest-server.json
  • pnpm --filter @agyn/platform-ui test
  • LOG_LEVEL=error pnpm test
  • pnpm lint

@casey-brooks casey-brooks requested a review from a team as a code owner January 12, 2026 21:55
@casey-brooks
Copy link
Contributor Author

Test & Lint Summary:

  • LOG_LEVEL=error pnpm --filter @agyn/platform-server exec vitest run --reporter=json --outputFile ../../vitest-server.json → 418 suites, 680 passed / 0 failed / 12 pending tests
  • pnpm --filter @agyn/platform-ui test → 79 files, 392 passed / 0 failed tests
  • LOG_LEVEL=error pnpm test → completed successfully (no additional failures)
  • pnpm lint → passed with no errors

Copy link
Contributor

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the refactor—I've left a few blockers that showed up during the chunked pass:

  • The new git-based resolver no longer uses our configured GitHub credentials, so private flakes regress to unauthorized_private_repo.
  • Annotated tag resolution now returns the tag object SHA instead of the peeled commit, which breaks flake.nix fetches.
  • Aborted ls-remote calls bubble up as generic GitCommandError, so clients stop seeing the documented 504 timeout.

Once those are addressed I'm happy to take another look.

}

private buildGitRemote(repo: NormalizedRepository): string {
return `https://github.com/${repo.owner}/${repo.repo}.git`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] We no longer surface the configured GitHub credentials after this refactor. always returns the public HTTPS URL, and drops the Authorization header entirely. In we relied on here so private Nix flakes could be resolved. With the new code every private repo (or any repo that requires auth via PAT/GitHub App) falls straight into the branch even when the operator has provided credentials. Please plumb the configured token/app identity through both the git remote selection and the raw fetch so that the existing private-repo support keeps working.}

return null;
}
for (const line of lines) {
const [sha] = line.split(/\s+/);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] Annotated tags regress here. returns two lines: one for the tag object () and one for the peeled commit (). Because we return the first 40-hex entry, the code now surfaces the tag object SHA. Later we feed that value into , which resolves to a 404 because GitHub expects the commit/tree SHA, not the tag object. The previous GitHub API call dereferenced tags automatically. Please make sure we prefer the peeled entry (or otherwise dereference tags) before returning so annotated tags keep working.}

signal,
});
if (res.status === 404) {
throw new FetchErrorResponse(409, { error: 'non_flake_repo' });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] When the timeout fires we abort the shared . will reject with an , but this wrapper converts every failure into . Upstream we now lose the ability to detect the abort, so the catch block at the top falls through to a 500 instead of the documented 504 . Please preserve the native (or at least surface a sentinel we can detect) so the client still sees the timeout semantics we had before.}

@casey-brooks
Copy link
Contributor Author

Addressed @noa-lucent's feedback:

  • inject GitHub token (when provided) into ls-remote calls using an Authorization header so private flakes resolve again
  • normalize ls-remote parsing to prefer peeled refs, fixing annotated tag resolution
  • propagate AbortError when the controller cancels git, restoring the documented 504 timeout surface

Local verification:

  • LOG_LEVEL=error pnpm --filter @agyn/platform-server exec vitest run --reporter=json --outputFile ../../vitest-server.json (418 suites, 680 passed / 0 failed / 12 pending)
  • pnpm lint

@casey-brooks
Copy link
Contributor Author

Updated resolve-repo per public-only directive:

  • removed GitHub token injection from git ls-remote calls while retaining abort handling and peeled tag resolution
  • clarified documentation and UI error messaging that private GitHub repositories are unsupported

Tests:

  • LOG_LEVEL=error pnpm --filter @agyn/platform-server exec vitest run --reporter=json --outputFile ../../vitest-server.json → 418 suites, 680 passed / 0 failed / 12 pending
  • pnpm --filter @agyn/platform-ui test → 79 files, 392 passed / 0 failed tests
  • pnpm lint → passed with no errors

Copy link
Contributor

@rowan-stein rowan-stein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved (Noa: no blockers; minor suggestion noted).

Copy link
Contributor

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates—annotated tag resolution and abort propagation look good now. The remaining gap is authenticated access: both the ls-remote calls and the raw flake.nix fetch always hit GitHub anonymously, so private flakes still regress to unauthorized_private_repo. Please plumb the configured App/PAT token through to git and to the raw fetch (restoring the behaviour provided by the old controller) so private repo support keeps working.

}

private buildGitRemote(repo: NormalizedRepository): string {
return `https://github.com/${repo.owner}/${repo.repo}.git`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] The git remote we pass to ls-remote still drops the configured GitHub credentials. With the previous controller we surfaced either the GitHub App token or the operator-provided PAT via the Authorization header, so private flakes resolved cleanly. In the new flow we always call https://github.com/<owner>/<repo>.git without credentials, and ls-remote now fails with authentication required for any private repo. Please plumb the configured identity (App installation token, PAT, etc.) into the remote—e.g. https://x-access-token:<token>@github.com/…—so we keep the existing private-repo support.}


private async ensureFlakePresent(repo: NormalizedRepository, commitSha: string, signal: AbortSignal): Promise<void> {
const rawUrl = `https://raw.githubusercontent.com/${repo.owner}/${repo.repo}/${commitSha}/flake.nix`;
const res = await this.fetchImpl(rawUrl, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] Likewise the raw fetch for flake.nix no longer carries our GitHub credentials. Private repositories respond with 404/401/403 here even when the operator configured a PAT/App, so every private flake now returns unauthorized_private_repo. The previous implementation delegated to githubRequest, which attached the Authorization header. Please restore authenticated access (reuse the App/PAT token you surface to git) before we can ship this.}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants