Skip to content

feat: Firestore同期コストガードをCLIに統合 - #575

Merged
YoneRai12 merged 1 commit into
mainfrom
codex/firestore-cost-guard-sync-smoke
Jun 24, 2026
Merged

feat: Firestore同期コストガードをCLIに統合#575
YoneRai12 merged 1 commit into
mainfrom
codex/firestore-cost-guard-sync-smoke

Conversation

@YoneRai12

Copy link
Copy Markdown
Owner

概要

Firestore realtime sync の closed-alpha cost guard を Public CLI に統合します。

このPRで追加すること:

  • yonerai.firestore_usage_policy.v1 の受け入れ検証
  • initial query limit 20 / absolute limit 50 / reconnect cooldown 30秒 / CLI listener 1本の fail-closed 検証
  • account-rooted query / offset禁止 / collection-group禁止 / client writes禁止 / AWS-only body fetch の検証
  • sync_mode=off を最優先の hard stop として扱う挙動
  • sync_mode=off 中の projection write 許可を拒否
  • sync listener firestore-poll の limit cap と cooldown guard
  • Public sync checkpoint

検証

  • python -m pytest 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 → 129 passed
  • python -m ruff check clients\cli\yonerai_cli\services\realtime_sync_client_service.py clients\cli\yonerai_cli\commands\sync.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
  • changed/untracked secret/local-path scan → live secret/token/account/private path/raw bodyなし

レビュー/同期状態

非主張

  • [PUBLIC-SYNC-SMOKE-PREPARED] はこのPR作成時点では未送信です。merge後に live staging の sync-off gate を再検証してから送ります。
  • [PUBLIC-SYNC-CLIENT-READY] ではありません。
  • Web-to-CLI E2E は未完了です。
  • sync_mode=off / firestore_sync_enabled=false の間は listener を開始しません。
  • 本番ログイン、本番同期、本番クラウド、本番デプロイ、release/tag は含みません。
  • token値、Firebase custom token値、account ID、raw body、provider key、private path、internal runtime detail は出していません。

@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 integrates a Firestore cost guard client and usage policy validation into the realtime sync service. It introduces policy sanitization, query limit capping, and reconnect cooldown enforcement, along with corresponding CLI formatting updates, tests, and documentation. The reviewer feedback highlights a critical bug where state is saved at the end of polling, potentially overwriting newer cursor data, and suggests cleaning up an unused sync_enabled parameter in the usage policy sanitization function.

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 +578 to +579
_record_firestore_poll(state, str(account_id))
_save_state(state_file, state)

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

バグ修正 (状態の上書き防止)

前述の通り、ループの前に _record_firestore_poll_save_state を実行するように移動したため、ここでの呼び出しは削除してください。

ここで古い state オブジェクトを保存してしまうと、ループ内の build_realtime_sync_listener_once_report がディスクに書き込んだ最新のカーソル情報やイベントIDが、古い状態データで上書きされて消失してしまいます。

effective_sync_enabled = bool(sync_enabled and sync_mode != "off")
firestore_summary = _sanitize_firestore_public_config(firestore, sync_enabled=sync_enabled)
usage_policy = payload.get("usage_policy") if isinstance(payload.get("usage_policy"), Mapping) else None
usage_summary = _sanitize_firestore_usage_policy(usage_policy, sync_enabled=sync_enabled)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

クリーンアップの提案

_sanitize_firestore_usage_policy から未使用の sync_enabled パラメータを削除することに伴い、呼び出し側も以下のように修正してください。

Suggested change
usage_summary = _sanitize_firestore_usage_policy(usage_policy, sync_enabled=sync_enabled)
usage_summary = _sanitize_firestore_usage_policy(usage_policy)

}


def _sanitize_firestore_usage_policy(policy: Mapping[str, object] | None, *, sync_enabled: bool) -> dict[str, object]:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

クリーンアップの提案

引数 sync_enabled は関数内で使用されていません。コードのクリーンアップと可読性向上のため、このパラメータを削除することを検討してください。

Suggested change
def _sanitize_firestore_usage_policy(policy: Mapping[str, object] | None, *, sync_enabled: bool) -> dict[str, object]:
def _sanitize_firestore_usage_policy(policy: Mapping[str, object] | None) -> dict[str, object]:

@YoneRai12

Copy link
Copy Markdown
Owner Author

Review intake for current head 40f5100:

Classification:

  • valid-now P0/P1/security: none found in current PR scan.
  • P2/P3/UX/dependency: nonblocking for Web-to-CLI sync lane.
  • blocker remaining before [PUBLIC-SYNC-SMOKE-PREPARED]: merge this PR, rerun live staging sync-off gate, then post prepared evidence to issue coord: Web-to-CLI realtime sync contract v1 #552.

Non-claims: not [PUBLIC-SYNC-CLIENT-READY], no live Web-to-CLI E2E, no release/tag, no production login/sync/cloud claim.

@YoneRai12 YoneRai12 added the intake-reviewed Maintainer confirmed PR/review/comment intake for current head. label Jun 24, 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: 40f5100aa4

ℹ️ 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 +578 to +579
_record_firestore_poll(state, str(account_id))
_save_state(state_file, state)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve cursor state before saving Firestore cooldown

When a Firestore poll processes events, each build_realtime_sync_listener_once_report call loads and saves the cursor/idempotency updates independently, but this final _save_state writes the parent’s stale pre-loop state back over that file after adding only last_firestore_poll_at. In any successful Firestore poll with events, the state file loses the saved cursor and duplicate history, so the next poll after cooldown resumes without the cursor and can re-read/reprocess old events.

Useful? React with 👍 / 👎.

@YoneRai12
YoneRai12 force-pushed the codex/firestore-cost-guard-sync-smoke branch from 40f5100 to facdd97 Compare June 24, 2026 16:20
@YoneRai12 YoneRai12 removed the intake-reviewed Maintainer confirmed PR/review/comment intake for current head. label Jun 24, 2026
@YoneRai12

Copy link
Copy Markdown
Owner Author

Review intake update for current head facdd97:

Classification:

  • valid-now P0/P1/security: none remaining.
  • valid-but-already-fixed: Gemini/Codex old-commit state overwrite and unused parameter comments.
  • P2/P3/UX/dependency: tracked/nonblocking.

Non-claims: not [PUBLIC-SYNC-CLIENT-READY], no live Web-to-CLI E2E, no release/tag, no production login/sync/cloud claim.

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

ℹ️ 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 +1518 to +1520
"token_issuance_allowed",
"projection_write_allowed",
"kill_switch",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Honor deny flags in Firestore usage policy

When the private config returns a usage_policy with token_issuance_allowed: false or kill_switch: true, these fields are admitted here but never evaluated; _sanitize_firestore_usage_policy still returns firestore_usage_policy_accepted: True, and the Firestore poll path only checks that accepted flag plus firestore_sync_enabled before signing in and reading metadata. In that misconfigured or emergency-off state the CLI can still attach to Firestore instead of failing closed, defeating the cost guard.

Useful? React with 👍 / 👎.

@YoneRai12 YoneRai12 added the intake-reviewed Maintainer confirmed PR/review/comment intake for current head. label Jun 24, 2026
@YoneRai12
YoneRai12 merged commit c6fbadd into main Jun 24, 2026
16 of 19 checks passed
@YoneRai12
YoneRai12 deleted the codex/firestore-cost-guard-sync-smoke branch June 24, 2026 16:27
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