Skip to content

fix: staging loginのcanonical account_idを保持 - #576

Merged
YoneRai12 merged 1 commit into
mainfrom
codex/fix-staging-login-canonical-account
Jun 24, 2026
Merged

fix: staging loginのcanonical account_idを保持#576
YoneRai12 merged 1 commit into
mainfrom
codex/fix-staging-login-canonical-account

Conversation

@YoneRai12

Copy link
Copy Markdown
Owner

概要

  • PR feat: Firestore同期コストガードをCLIに統合 #575 merge 後の live staging smoke で見つかった Public 側 blocker を修正します。
  • login 時に /v1/account/me が top-level account_id と nested account 表示情報を返す場合でも、top-level canonical account_id を staging session claim に保持します。
  • whoami で後から補正しなくても、sync listener / firebase-token が canonical account_id を参照できます。

検証

  • python -m pytest tests\test_auth_privacy_policy.py tests\test_realtime_sync_client_service.py tests\test_realtime_sync_event_service.py tests\test_official_sync_cli.py tests\test_conversation_sync_policy.py tests\test_native_run_client.py tests\test_control_spine_client.py -q -> 226 passed
  • python -m ruff check clients\cli\yonerai_cli\staging_auth_bridge.py clients\cli\yonerai_cli\services\realtime_sync_client_service.py clients\cli\yonerai_cli\commands\sync.py tests\test_auth_privacy_policy.py tests\test_realtime_sync_client_service.py tests\test_official_sync_cli.py -> passed
  • python -m compileall -q clients\cli\yonerai_cli tests -> passed
  • git diff --check -> passed
  • python scripts\ci_quality_scans.py --changed -> passed

境界

  • staging only。
  • [PUBLIC-SYNC-SMOKE-PREPARED] ではありません。
  • live Web-to-CLI E2E ではありません。
  • release/tag は作りません。
  • Google token / Firebase token / provider key / raw body / private path は出力・保存しません。

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a helper function _account_source_with_canonical_id in staging_auth_bridge.py to preserve the top-level canonical account_id during staging login, along with corresponding unit tests and documentation updates. Feedback on the changes highlights a critical issue in the helper function: if the nested account object already contains a non-canonical account_id, the top-level canonical ID will not overwrite it, leading to a fallback behavior. It is recommended to always overwrite the nested ID with the top-level canonical account_id when it is available.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +389 to +390
if body.get("account_id") is not None and not merged.get("account_id"):
merged["account_id"] = body.get("account_id")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

ネストされた accountidentity オブジェクトに、プロバイダーのユーザーID(例: google-oauth2|...)などの非 canonical な account_id が既に含まれている場合、not merged.get("account_id")False と判定されてしまいます。その結果、トップレベルの canonical な account_idacct_...)が上書きされず、sanitize_staging_account 内で非 canonical なIDが優先されてしまい、最終的にハッシュ化された staging-account-... にフォールバックしてしまいます。トップレベルの account_id は常に canonical なアカウントIDであるため、ネストされたオブジェクトに account_id が存在する場合でも、トップレベルの値を優先して常に上書きするように変更することを推奨します。

Suggested change
if body.get("account_id") is not None and not merged.get("account_id"):
merged["account_id"] = body.get("account_id")
if body.get("account_id") is not None:
merged["account_id"] = body.get("account_id")

@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: c540c79a33

ℹ️ 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".

Comment on lines +389 to +390
if body.get("account_id") is not None and not merged.get("account_id"):
merged["account_id"] = body.get("account_id")

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 Override nested IDs with the canonical account_id

When /v1/account/me returns a top-level canonical account_id alongside a nested display account that also carries a legacy/non-canonical account_id, this guard keeps the nested value. sanitize_staging_account() then rejects or hashes that nested value, so yonerai login still saves a staging-account-* reference and realtime sync continues to hit canonical_account_id_required until a separate whoami repair runs.

Useful? React with 👍 / 👎.

@YoneRai12
YoneRai12 force-pushed the codex/fix-staging-login-canonical-account branch from c540c79 to 4fae1b5 Compare June 24, 2026 16:42
@YoneRai12
YoneRai12 force-pushed the codex/fix-staging-login-canonical-account branch from 4fae1b5 to d44c052 Compare June 24, 2026 16:49
@YoneRai12

Copy link
Copy Markdown
Owner Author

PR #576 review-intake classification after final push d44c052:

  • Gemini high finding on old commit c540c79: valid-now at review time, fixed in d44c052 by always prioritizing the top-level canonical account_id over nested legacy/non-canonical IDs.
  • Codex P2 finding on old commit c540c79: duplicate of the same valid finding, fixed in d44c052.
  • Regression evidence: test_staging_login_preserves_top_level_account_id_from_account_me now includes a nested legacy account.account_id and asserts the saved staging claim uses the top-level canonical account id.
  • Local validation after final push: 47 passed, ruff passed, git diff --check passed, ci_quality_scans.py --changed passed.
  • Remote validation after final push: normal CI / Quality Wall checks passed. Only review-intake requires this classification label.
  • No current P0/P1/security blocker remains for this PR.

@YoneRai12 YoneRai12 added the intake-reviewed Maintainer confirmed PR/review/comment intake for current head. label Jun 24, 2026
@YoneRai12
YoneRai12 merged commit b25b5c3 into main Jun 24, 2026
15 of 16 checks passed
@YoneRai12
YoneRai12 deleted the codex/fix-staging-login-canonical-account branch June 24, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

intake-reviewed Maintainer confirmed PR/review/comment intake for current head.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant