Skip to content

fix(openai): exclude user-owned alias overlays from canonical seed validation (rebase of #3121) - #3189

Merged
lidge-jun merged 3 commits into
devfrom
codex/3121-alias-overlay
Sep 1, 2026
Merged

fix(openai): exclude user-owned alias overlays from canonical seed validation (rebase of #3121)#3189
lidge-jun merged 3 commits into
devfrom
codex/3121-alias-overlay

Conversation

@lidge-jun

Copy link
Copy Markdown
Owner

Summary

Maintainer rebase of #3121 by @Flowershangfromthebranches onto current dev — all three commits cherry-picked with author credit preserved, no conflicts.

Canonical seed validation was rejecting provider writes because it counted user-owned alias overlays as if they were canonical entries. An operator who had added their own alias could no longer save unrelated provider changes. The overlays are now excluded from canonical seed validation, and alias API ownership is preserved across provider writes rather than being flattened into the seed.

Verification

Exact head 821b3c9f9:

  • bun test ./tests/management-provider-validation.test.ts91 pass, 0 fail, 600 expect() calls. The diff adds 265 lines to that file, so most of that count is the author's own new coverage.

Full-suite and typecheck coverage is left to CI on this exact head.

Checklist

  • Targets dev
  • Author credit preserved across all three commits
  • No unresolved review threads on the original
  • Management-route change is covered by the provider-validation suite
  • No credential, auth, workflow, or release-automation surface touched

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 1, 2026 17:29
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T17:33:56.918709Z 821b3c9 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun
lidge-jun merged commit 5557772 into dev Sep 1, 2026
19 of 20 checks passed
@lidge-jun
lidge-jun deleted the codex/3121-alias-overlay branch September 1, 2026 17:30
@github-actions github-actions Bot added the bug Something isn't working label Sep 1, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 821b3c9f94

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

// DNS validation above awaits. Re-read the live row so a dedicated alias write that
// completed during that wait remains authoritative instead of being overwritten by the
// older ownership snapshot used to admit this POST.
restorePersistedAliasOverlays(prov, config.providers[name]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Revalidate restored aliases against the edited catalog

When a full provider POST changes provider.models, this restores the old modelAliases only after validation, so an alias can now collide with a newly added native model ID. The dedicated alias route would reject that collision, while sanitizeAliasesForLoad in src/config.ts:2106-2113 deletes it on the next restart, causing the supposedly preserved alias to disappear. Validate the restored overlays against the resulting provider catalog before saving, or reject the provider edit when it invalidates an existing alias.

AGENTS.md reference: src/AGENTS.md:L10-L10

Useful? React with 👍 / 👎.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 66 / 80

지금 dev HEAD는 4a382beed 입니다. 바로 앞 머지는 #3207 (opt-in authless Codex Desktop routing, #1107) 이고, 그 앞은 #2986 xAI Imagine image_gen 릴레이입니다. 이 PR #3189는 이미 5557772b7dev 에 들어가 있습니다. HEAD보다 15커밋 앞에 있는 지연 오픈 트리거입니다. 원본은 기여자 @Flowershangfromthebranches#3121 이고, 메인테이너가 세 커밋을 현재 dev 위에 체리픽한 리베이스입니다. 작성자 크레딧은 유지됐고, #3121 은 이미 Landed via maintainer rebase #3189 댓글로 닫혀 있습니다.

고치는 문제는 간단합니다. 전용 alias API(src/server/management/model-routes.ts/api/aliases, /api/default-aliases, /api/providers/{provider}/alias, /api/providers/{provider}/model-aliases)가 alias / modelAliases / defaultAliases 를 합법적으로 저장한 뒤, 대시보드의 일반 provider POST/PATCH(예: modelContextWindows)가 src/server/auth-cors.tsproviderManagementConfigError 에서 provider openai must equal the canonical built-in provider seed 로 막혔습니다. 그 함수는 modelCosts / requestPacing / contextWindow / modelContextWindows / modelAutoCompactTokenLimits / annotateEmptyToolOutputs 는 candidate에서 지우지만, alias 세 필드는 지우지 않습니다. 그래서 라우트 쪽에서 미리 벗기지 않으면 seed 비교가 사용자 오버레이를 canonical 로 오해합니다.

이 PR은 src/server/management/provider-routes.ts 에 세 도우미를 넣습니다. PROVIDER_ALIAS_OVERLAY_FIELDS 로 목록을 한곳에 모읍니다. providerAliasOverlayOwnershipError 는 전체 provider POST가 오버레이를 만들거나 지우거나 바꾸지 못하게 합니다. 이미 저장된 값과 바이트/엔트리 단위로 같을 때만 round-trip을 허용합니다. providerTransportValidationCandidate 는 검증 직전에 세 필드를 벗긴 복사본을 만듭니다. restorePersistedAliasOverlays 는 DNS await 뒤 최신 live row에서 권위 있는 오버레이를 다시 붙입니다. 전용 alias 쓰기가 그 대기 중에 끝났을 때, 오래된 ownership 스냅샷으로 덮어쓰지 않게 하려는 의도입니다. PATCH는 필드 마스크 앞에서 alias 키를 outright 400으로 거절하고 전용 API로 보냅니다. reload 경로도 같은 strip 뒤 providerManagementConfigError 를 돌립니다.

테스트는 tests/management-provider-validation.test.ts 에 약 265줄을 더합니다. 전체 편집이 오버레이를 보존하는지, 일반 쓰기가 충돌/예약/잘못된 model alias를 못 넣는지, 잘못된 shape가 4xx로 막히고 디스크가 안 바뀌는지, overlay가 있어도 baseUrl/adapter/authMode 변조가 거절되는지, 비-OpenAI 편집이 오버레이를 남기는지, defaultAliases 가 있어도 modelContextWindows PATCH가 되는지 잠급니다. destination-policy spy도 복구합니다. 본문 기준 리베이스 헤드에서 이 파일만 91 pass / 0 fail 입니다. types.ts/config.ts 를 다시 합치는 PR이 아닙니다. close-don't-rebase 대상이 아닙니다. 중복 원본 #3121 은 이미 닫혀 있습니다.

이미 랜딩된 코드라 머지 여부를 다시 물을 필요는 없습니다. 남은 정리만 보면 됩니다. CI rollup에서 gates / ci 가 failure로 찍혀 있었지만 테스트 샤드와 typecheck 계열은 성공했습니다. gates 로그에 Bun 버전 해석 실패 문구가 보여, 이 PR 파일 회귀로 보기 어렵습니다. 후속 열차(#3194 이후 HEAD까지)가 계속 올라간 점도 그 판단을 받쳐 줍니다.

src/server/management/provider-routes.ts:146 - 주석은 "ownership이 이미 확립된 오버레이만 제거"라고 쓰여 있지만, providerTransportValidationCandidate 는 세 필드를 조건 없이 지웁니다. 동작은 안전하고 호출 전 ownership 검사가 따로 있습니다. 주석만 실제와 어긋납니다.
src/server/auth-cors.ts:594-617 - seed 비교 candidate에서 alias 세 필드를 여전히 안 지웁니다. 이 PR은 라우트에서 벗기는 쪽으로 해결했습니다. 다른 호출부가 strip 없이 providerManagementConfigError 만 부르면 같은 교착이 다시 날 수 있습니다.
src/server/management/provider-routes.ts:657-658 - POST는 providerEmptyToolOutputConfigError 에도 strip된 candidate를 넘깁니다. reload(593줄)는 empty-tool 검사에 원본 provider를 씁니다. alias는 그 검사와 무관해서 지금은 해가 작습니다. 대칭만 조금 다릅니다.
tests/management-provider-validation.test.ts (신규 블록) - destinationPolicy spy를 mockRestore 하는 회귀를 다시 넣었습니다. 가짜 초록을 막는 중요한 테스트 위생입니다.
원본 #3121 라벨 - landing 댓글은 있지만 landed-via-maintainer 라벨은 아직 없습니다. standing leftover 규칙상 붙이는 편이 맞습니다.

메인테이너의 판단이 필요한 지점

  • alias 오버레이 strip을 계속 provider-routes 레이어에 둘지, auth-cors.ts 의 seed 비교에도 같은 삭제 목록을 넣을지 정해야 합니다. 라우트만 두면 호출 누락 위험이 남고, auth-cors에 넣으면 일반 쓰기 경로의 ownership 가드와 책임이 섞일 수 있습니다.
  • 146줄 주석을 코드에 맞게 고칠지, 후속 정리로 둘지 정해야 합니다.
  • fix(openai): exclude user-owned alias overlays from canonical seed validation #3121landed-via-maintainer 를 지금 붙일지 정해야 합니다. 댓글 landing은 이미 있습니다.
  • 이 PR의 gates/ci failure를 무시할지, Bun 해석 쪽 인프라를 따로 볼지 정해야 합니다. 코드 회귀 신호로는 약합니다.

너의 추천
이미 dev 에 들어가 있으니 추가 머지 작업은 없습니다. 이 댓글은 지연 오픈 트리거에 대한 사후 리뷰입니다. leftover #3121 은 landing 댓글이 있으니 landed-via-maintainer 라벨만 보강하면 됩니다. types.ts/config.ts 분리와는 무관하고, 중복 재오픈도 필요 없습니다. 후속으로 146줄 주석과 auth-cors strip 일관성만 보면 충분합니다.

이 댓글은 grok-bot이 작성했습니다

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants