Skip to content

feat(backend): split X connector sync into its own Cloud Run Job (#9298) - #11183

Open
aryanorastar wants to merge 18 commits into
BasedHardware:mainfrom
aryanorastar:fix/9298-x-connector-sync-job
Open

feat(backend): split X connector sync into its own Cloud Run Job (#9298)#11183
aryanorastar wants to merge 18 commits into
BasedHardware:mainfrom
aryanorastar:fix/9298-x-connector-sync-job

Conversation

@aryanorastar

@aryanorastar aryanorastar commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Move X connector incremental sync off notifications-job onto a dedicated x-connector-sync-job (entrypoint, Dockerfile, runtime-env contract, deploy workflow).
  • Make Scheduler the single cadence owner (x-connector-sync-6h); the entrypoint no longer uses hour-modulo gating.
  • Fail the Cloud Run execution on registry/listing failures or total per-user failure instead of reporting a healthy no-op.
  • Preserve the existing scheduled-memory contract: construct one PromotionFlexRunRouter per job run, reuse it for every connected user, and leave intentional Flex capacity deferrals pending without counting them as sync failures.
  • Add the runbook and Cloud Run Jobs checklist/scaffold so future cron work does not return to the notifications process.

Fixes #9298

Why the Flex follow-up matters

While this PR was open, main added the shared Flex router to scheduled X memory extraction. The job split retained the Flex-aware per-user implementation but its rewritten scheduler boundary stopped passing the router, which made scheduled extraction silently use the legacy standard lane. The latest commit restores the current production routing contract without weakening this PR's new Cloud Run failure surface.

Verification

  • Focused X scheduler and failure-surface suites — 16 passed.
  • X + runtime-env + deployment-contract suites — 162 passed.
  • New behavioral regression drives run_x_sync_job() through two registry users and proves:
    • exactly one router is built with the job start time;
    • the same router reaches every sync_x_for_user() call;
    • a PromotionFlexDeferred user remains retryable and does not increment failed.
  • Full backend/test.sh sweep exercised all 992 test files; both X suites passed. Five unrelated current-main files remain red and are byte-identical to origin/main in this branch (test_legacy_memory_surface_inventory.py, test_modulate_stt.py, test_streaming_deepgram_backoff.py, test_async_app_integrations.py, test_async_realtime_integrations_offload.py).
  • Runtime image source-closure/import-smoke coverage includes x-connector-sync-job.
  • Maintainer operational follow-up after merge: create/verify the Scheduler job and perform the first deploy via gcp_x_connector_sync_job.yml.

Failure class

Failure-Class: none

The change is an ownership split plus a review-time regression repair; it is not a shipped recurring failure-class instance.

Product invariants affected

  • INV-DATA-1 — preserves the existing production data-plane authority; the new job workflow does not change client or Firebase routing.
  • INV-MEM-4 — preserves canonical-memory maintenance and scheduled Flex-routing ownership while removing unrelated X-sync bindings from notifications-job.

Test plan

  • Behavioral X scheduler/Flex routing and deferral coverage.
  • Cloud Run nonzero failure surface coverage.
  • Runtime env, Scheduler validator, notifications isolation, image contract, and render tests.
  • Current-main refresh and diff hygiene.
  • Post-merge live Scheduler/deploy verification by a maintainer with GCP authority.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@cubic-dev-ai cubic-dev-ai 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.

4 issues found across 24 files

Confidence score: 2/5

  • In backend/utils/other/jobs.py, deploying the notifications job before x-connector-sync-job and its Scheduler trigger exist can halt X incremental sync, leaving connected users with stale data; gate rollout on verified job/scheduler creation or enforce dependency ordering in deployment automation.
  • In .github/workflows/gcp_x_connector_sync_job.yml, the unconditional gcloud scheduler jobs describe "$SCHEDULER_JOB" check makes the first workflow run fail when x-connector-sync-6h has not been created yet, which can block initial provisioning—add a create-if-missing/bootstrap path or conditional validation.
  • In backend/scripts/scaffold_cloud_run_job.sh, name validation currently allows trailing hyphens and overlong names, so scaffolding appears successful but later gcloud run jobs deployment fails; validate the full Cloud Run naming regex and 1–63 length constraints up front.
  • In backend/scripts/validate_x_connector_sync_scheduler.py (and duplicated tests), near-copying validate_memory_maintenance_scheduler increases drift risk when one validator evolves and the other does not; extract shared scheduler-validation logic to a common helper and keep only job-specific constants in each script.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/utils/other/jobs.py">

<violation number="1" location="backend/utils/other/jobs.py:6">
P1: Deploying the updated notifications job before `x-connector-sync-job` and its Scheduler trigger are provisioned stops all X incremental sync, leaving connected users' data stale. Please make creation/verification of `x-connector-sync-6h` a prerequisite for this handoff, or retain the old invocation until the new job is live.</violation>
</file>

<file name="backend/scripts/scaffold_cloud_run_job.sh">

<violation number="1" location="backend/scripts/scaffold_cloud_run_job.sh:52">
P2: The input check accepts trailing-hyphen and overlong Cloud Run job names, causing scaffolding to succeed before the eventual `gcloud run jobs` deployment rejects the resource name. Validate the full 1–63 character name grammar before writing the stubs.</violation>
</file>

<file name="backend/scripts/validate_x_connector_sync_scheduler.py">

<violation number="1" location="backend/scripts/validate_x_connector_sync_scheduler.py:45">
P2: This new scheduler validator is a near-verbatim copy of `backend/scripts/validate_memory_maintenance_scheduler.py` (only the schedule constant and docstring differ), and the test file duplicates `test_validate_memory_maintenance_scheduler.py`. Because both are deploy-critical gates, any future contract change (e.g., adding retry config, a second target field, or a different OAuth setting) has to be applied to two independent copies and can silently drift. Per the repo's own guidance on shared primitives, consider extracting a common scheduler-contract validator (e.g., a shared `SchedulerContract` + `validate_scheduler_state(state, contract, *, schedule, display_name)`) and having the memory and X connector entrypoints reuse it, instead of maintaining two parallel implementations.</violation>
</file>

<file name=".github/workflows/gcp_x_connector_sync_job.yml">

<violation number="1" location=".github/workflows/gcp_x_connector_sync_job.yml:140">
P2: The first run of this workflow will fail at 'Validate 6h Scheduler trigger': the step runs `gcloud scheduler jobs describe "$SCHEDULER_JOB"` unconditionally, but this PR does not create the `x-connector-sync-6h` scheduler (per the PR description, maintainers wire it via runbook). On the initial deploy the scheduler does not exist, so `describe` returns NOT_FOUND and the whole deploy job reports failed right after the Cloud Run job was successfully deployed. The operations team ends up with a red first deploy and a manual pre-step ordering that isn't encoded anywhere in the workflow. Consider making the scheduler validation resilient to a missing scheduler on first bootstrap (e.g., skip with a warning when `describe` returns NOT_FOUND), or add an explicit scheduler-create/bootstrap step so the first deploy isn't guaranteed to fail.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread backend/scripts/validate_x_connector_sync_scheduler.py Outdated

async def start_job():
# Notification
# Notification / daily summary only. X connector sync lives on x-connector-sync-job.

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: Deploying the updated notifications job before x-connector-sync-job and its Scheduler trigger are provisioned stops all X incremental sync, leaving connected users' data stale. Please make creation/verification of x-connector-sync-6h a prerequisite for this handoff, or retain the old invocation until the new job is live.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/utils/other/jobs.py, line 6:

<comment>Deploying the updated notifications job before `x-connector-sync-job` and its Scheduler trigger are provisioned stops all X incremental sync, leaving connected users' data stale. Please make creation/verification of `x-connector-sync-6h` a prerequisite for this handoff, or retain the old invocation until the new job is live.</comment>

<file context>
@@ -1,13 +1,8 @@
 
 async def start_job():
-    # Notification
+    # Notification / daily summary only. X connector sync lives on x-connector-sync-job.
     if should_run_daily_notification_job():
         await start_cron_notification_job()
</file context>

Comment thread backend/scripts/scaffold_cloud_run_job.sh Outdated
Comment thread backend/deploy/runtime_env/_base.yaml
Comment thread backend/scripts/scaffold_cloud_run_job.sh Outdated
Comment thread backend/tests/unit/test_x_connector_sync_job_import.py
Comment thread backend/tests/unit/test_validate_x_connector_sync_scheduler.py Outdated
Comment thread backend/tests/unit/test_validate_x_connector_sync_scheduler.py
Comment thread backend/scripts/scaffold_cloud_run_job.sh Outdated
Comment thread backend/utils/x_connector.py Outdated

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for pulling the X connector sync out into its own Cloud Run Job. The direction is good: the new entrypoint keeps the job single-purpose, the workflow includes a read-only Scheduler contract check, and the tests cover the notifications-job isolation and selected-job runtime-env rendering.

I do think this needs changes before merge because the new job does not appear to receive the full runtime contract needed by run_x_sync_job():

  • backend/utils/x_connector.py refreshes OAuth access tokens via X_OAUTH_CLIENT_ID, X_OAUTH_CLIENT_SECRET, and X_OAUTH_REDIRECT_URI, then falls back to utils.social.get_twitter_timeline(), which reads RAPID_API_HOST / RAPID_API_KEY at import time.
  • The new cloud_run.jobs.x-connector-sync-job bindings in backend/deploy/runtime_env*.yaml move PINECONE_* over, but do not bind the X OAuth or RapidAPI env/secrets to the new job.
  • That means the dedicated job can refresh neither expired OAuth tokens nor the RapidAPI fallback path; connected users whose access tokens expire would silently degrade to failed/stale X syncs even though the Scheduler and Cloud Run Job are healthy.

Please add the X OAuth and RapidAPI bindings needed by the code path to x-connector-sync-job (and keep any secrets off notifications-job if it no longer needs them), then extend the runtime-env/render tests to assert the new job carries those values and the notifications job does not retain X-sync-only bindings.

Agent-instruction impact: this PR also updates backend/AGENTS.md and adds docs/doc/developer/backend/cloud_run_jobs_checklist.md, both of which guide coding/review agents working in this repo. The guidance is directionally safe and useful (one periodic domain per Cloud Run Job; do not hitchhike unrelated work onto notifications-job), but because it changes future agent behavior and deploy scaffolding expectations, I’d still want maintainer review on the final wording once the runtime contract issue above is fixed.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added needs-maintainer-review Needs a human maintainer to sign off before merge workflow-review Needs maintainer review for workflow, automation, hooks, or CI behavior backend Backend Task (python) labels Aug 6, 2026
aryanorastar added a commit to aryanorastar/omi that referenced this pull request Aug 7, 2026
Address David CR on BasedHardware#11183: the dedicated sync job must refresh OAuth
tokens and use RapidAPI fallback. Also tighten scaffold/scheduler gates.

Co-authored-by: Cursor <cursoragent@cursor.com>
@aryanorastar

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level addressed your runtime-contract CR in c0aab2816b.

Runtime contract (your blocker)

cloud_run.jobs.x-connector-sync-job now binds everything run_x_sync_job() needs:

  • Secrets: X_OAUTH_CLIENT_SECRET, RAPID_API_KEY (plus existing OpenAI / SA / encryption / Pinecone)
  • Env: X_OAUTH_CLIENT_ID, X_OAUTH_REDIRECT_URI, RAPID_API_HOST (workflow passes the GitHub env vars into render)
  • Notifications-job stays free of X-sync-only bindings; render/validator tests assert both sides

Also fixed while here (cubic P1s aligned with the same surface)

  • Scaffold no longer strips Build/Verify/Push; gateway URL env removal keeps valid YAML
  • Scheduler validator requires the project-scoped x-connector-sync-scheduler@… SA (+ UnicodeError on state files)
  • Runbook documents deploy order so notifications without X sync cannot land before this job + x-connector-sync-6h are live
  • Removed dead should_run_x_sync_job() hour-modulo gate

Verification

Focused unit suite green in backend/.venv (render + validator + orchestrator + import smoke + scheduler).

Ready for re-pass when you have a moment. Agent-doc wording (AGENTS.md / checklist) unchanged aside from the runbook contract/rollout note — still happy to adjust that if you want different guidance.

@aryanorastar

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level gentle bump for re-pass.

Head b04ac48ea5 is fully CI green (Backend unit suite + hermetic gauntlets + Hygiene/Formatting/Static Contracts). Runtime-contract fix from your CR is in (X_OAUTH_* + RapidAPI on x-connector-sync-job; notifications stays clean). Author-side clear — ready when you can re-review.

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the follow-up. The original runtime-contract blocker is addressed now: x-connector-sync-job receives the X OAuth/RapidAPI bindings needed by run_x_sync_job(), and the tests now assert those bindings stay off notifications-job.

I still need to request one deploy-readiness change before this can merge:

  • backend/modal/x_connector_sync_job.py calls asyncio.run(run_x_sync_job()) and then exits 0 regardless of the returned summary. In backend/utils/x_connector.py, the job catches a global user-listing failure and returns {'users': 0, 'synced': 0, 'new_posts': 0}, and per-user failures are logged/ignored. As a standalone Cloud Run Job triggered every 6h, that means credential/firestore/network breakage can look like a successful Scheduler execution, so the new dedicated sync job can silently stop syncing users while Cloud Run/Scheduler appear healthy.

Please make the job surface failure to Cloud Run for at least global/listing failures, and preferably include enough failed-user accounting in the runner/entrypoint to raise when the scheduled run could not complete its sync contract. The nearby memory-maintenance-job pattern of inspecting the summary and raising on errors is a good reference.

Agent-instruction impact: this PR updates backend/AGENTS.md and adds docs/doc/developer/backend/cloud_run_jobs_checklist.md, which will guide future coding/review agents toward one periodic domain per Cloud Run Job and away from adding unrelated work to notifications-job. That guidance looks safe and useful, but because it changes future agent behavior and deploy scaffolding expectations, it should still get human maintainer sign-off with the workflow/deploy changes.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@aryanorastar

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level addressed the deploy-readiness CR on c4833cf528.

Failure surface (your blocker) — mirrors memory-maintenance-job:

  • run_x_sync_job() now returns failed + errors; listing/Firestore breakage populates errors instead of a healthy {users:0,…} summary
  • Per-user failures increment failed (including success: false returns, not only exceptions)
  • Entrypoint does summary = asyncio.run(...) then raise_if_x_sync_job_failed(summary) so Cloud Run / Scheduler see a non-zero exit on:
    • any listing/global errors
    • total failure (users > 0 and synced == 0 and every user failed)
  • Partial success (some users synced) still exits 0 — one flaky account doesn’t red-page the whole job

Tests: test_x_connector_sync_job_failure_surface.py + orchestrator entrypoint wiring — 13 focused tests green locally.

Ready for re-pass when you have a moment. Agent-doc wording unchanged from prior head.

aryanorastar added a commit to aryanorastar/omi that referenced this pull request Aug 8, 2026
Split mangled x-connector render assertions into their own test (NameError
on jobs), and black-format the failure-surface files at line-length 120.

Failure-Class: none
Co-authored-by: Cursor <cursoragent@cursor.com>
@aryanorastar

aryanorastar commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

CI was red on c4833cf528 after your failure-surface re-pass tip — fixed on 12fdcc1999:

  1. Static Contracts / Backend unit — GitHub’s merge-with-main put the x-connector render asserts into test_dev_runtime_manifest_contains_no_removed_first_user_or_capture_admission without defining jobs (NameError). Merged main locally and moved those asserts into test_render_dev_emits_x_connector_sync_job_outputs.
  2. Formatting — black 26.5.1 / line-length 120 on test_x_connector_sync_job_failure_surface.py + utils/x_connector.py.

Focused local: test_render_backend_runtime_env + failure-surface + notifications orchestrator → 37 passed.

Author-side clear again once CI goes green — ready for re-pass on the failure-surface behavior when you can.

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the follow-up — the two earlier blockers I raised look addressed on this head:

  • backend/deploy/runtime_env.yaml / _base.yaml now give x-connector-sync-job the X OAuth, RapidAPI, Pinecone, Firebase, encryption, and stage/project bindings it needs, while notifications-job no longer carries the X-sync-only env/secrets.
  • backend/modal/x_connector_sync_job.py now calls raise_if_x_sync_job_failed(summary) after asyncio.run(run_x_sync_job()), and backend/utils/x_connector.py now reports listing errors and all-user failures through the summary instead of letting Cloud Run/Scheduler see a healthy no-op.
  • .github/workflows/gcp_x_connector_sync_job.yml uses the checked-out SHA for image tags, deploys the dedicated job, and gates success on a read-only Scheduler describe + validator.
  • backend/utils/other/jobs.py is now limited to notifications/daily summary work, which matches the split.
  • backend/AGENTS.md and docs/doc/developer/backend/cloud_run_jobs_checklist.md do change future agent/coding guidance: they steer agents toward one periodic domain per Cloud Run Job and away from hitchhiking new domains onto notifications-job. That guidance looks safe and aligned with this fix, but it is agent-behavior/deploy guidance and should still get maintainer sign-off with the workflow change.

I found one remaining deploy-contract issue that should be fixed before merge:

  • backend/testing/workflow_contracts.json adds the invariant that x-connector-sync-job deploys fail unless the exact enabled 6h OAuth Scheduler trigger targets the deployed job, and it registers tests/unit/test_validate_x_connector_sync_scheduler.py. However, the backend_cloud_run_deploy.sources list still only includes the memory-maintenance workflow/validator (.github/workflows/gcp_memory_maintenance_job.yml, .github/workflows/gcp_memory_maintenance_job_auto_dev.yml, and backend/scripts/validate_memory_maintenance_scheduler.py). It does not include .github/workflows/gcp_x_connector_sync_job.yml or backend/scripts/validate_x_connector_sync_scheduler.py, which are the actual enforcement files for the new invariant. That leaves the source-closure/diff-scoped contract mapping blind to future edits of this deploy-critical gate. Please add those two files to the backend_cloud_run_deploy.sources list alongside the existing memory-maintenance entries.

Given this touches workflow/deploy behavior and agent-facing docs, I’m leaving the existing workflow-review / needs-maintainer-review labels in place for maintainer sign-off after the source-closure fix.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level
Git-on-my-level dismissed stale reviews from themself August 8, 2026 21:35

Resolved on current head: the X connector job now has the required runtime bindings and surfaces sync failures to Cloud Run; current review tracks a separate workflow-contract source-closure issue.

@aryanorastar

Copy link
Copy Markdown
Contributor Author

Addressed review @936f3ede4056

@Git-on-my-level Source-closure blocker fixed.

backend/testing/workflow_contracts.jsonbackend_cloud_run_deploy.sources now includes:

  • .github/workflows/gcp_x_connector_sync_job.yml
  • backend/scripts/validate_x_connector_sync_scheduler.py

alongside the existing memory-maintenance entries, so diff-scoped/source-closure mapping tracks the 6h Scheduler invariant enforcement files (not only the registered test).

Also merged latest main. Local: test_every_external_workflow_contract_source_triggers_backend_unit_workflow + test_validate_x_connector_sync_scheduler → 15 passed.

Ready for re-pass / dismiss CR when CI is green.

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the updates — I re-reviewed this head and the prior blocker looks resolved.

What I checked:

  • backend/testing/workflow_contracts.json now includes both .github/workflows/gcp_x_connector_sync_job.yml and backend/scripts/validate_x_connector_sync_scheduler.py in the backend Cloud Run deploy source closure, so the new Scheduler/deploy invariant is no longer blind to the actual enforcement files.
  • .github/workflows/gcp_x_connector_sync_job.yml defines a dedicated manual deploy for x-connector-sync-job, tags from the checked-out commit, renders only the selected job env, and fails the deploy if the existing x-connector-sync-6h Scheduler contract does not match the job.
  • backend/deploy/runtime_env.yaml plus _base.yaml/overlays separate the X OAuth/RapidAPI/Pinecone bindings onto x-connector-sync-job while keeping notifications-job scoped to notification/daily-summary runtime bindings.
  • backend/modal/x_connector_sync_job.py is a thin Firebase-initializing entrypoint that always runs run_x_sync_job() and calls raise_if_x_sync_job_failed(summary), so Cloud Run/Scheduler see a non-zero failure when the sync contract is broken.
  • backend/utils/x_connector.py removes the old hour-modulo gate, reports listing failures through errors, counts per-user failures, and fails only on global/listing or all-user failure while still allowing partial progress.
  • backend/utils/other/jobs.py no longer imports or invokes X sync, which matches the ownership split away from notifications-job.
  • backend/scripts/validate_x_connector_sync_scheduler.py is read-only and validates the exact Scheduler name, 6h cadence, target URI, OAuth service account, and enabled state without creating or mutating cloud resources.
  • backend/AGENTS.md and docs/doc/developer/backend/cloud_run_jobs_checklist.md do change agent-facing repo guidance: future coding/review agents will be steered toward one periodic domain per Cloud Run Job and away from adding unrelated jobs to notifications-job. That guidance looks safe and consistent with this PR, but it is still maintainer-facing agent/deploy guidance.

Given the workflow/deploy surface and the agent-instruction docs, I’m not formally approving; this should still get human maintainer sign-off for the rollout plan and Scheduler resource creation. Technically, I do not see a remaining code blocker on this head, and I’m dismissing my older change request because the source-closure concern is now addressed.


by AI on behalf of David — leaving this for human maintainer review because it changes deploy workflow behavior and agent-facing backend guidance.

@Git-on-my-level
Git-on-my-level dismissed their stale review August 9, 2026 21:34

Resolved on 936f3ed: backend_cloud_run_deploy now includes gcp_x_connector_sync_job.yml and validate_x_connector_sync_scheduler.py in source closure.

@aryanorastar

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level @kodjima33 need human response

Author-clear on 936f3ede40:

  • Your source-closure CR dismissed; latest pass found no remaining code blocker
  • MERGEABLE + all checks green (backend unit, Static Contracts, Hygiene, hermetic, source-closure)
  • Prior blockers still on tip: X OAuth/RapidAPI on x-connector-sync-job, fail-closed sync-contract exit, backend_cloud_run_deploy.sources includes the new workflow + scheduler validator

Branch protection still waits on code-owner review from @Git-on-my-level (workflow/deploy) plus write-access APPROVE. Please APPROVE when the rollout plan is acceptable; Scheduler job creation + first deploy via gcp_x_connector_sync_job.yml remain maintainer post-merge (runbook), as called out in the PR body.

@undivisible undivisible added human Human-authored pull request docs-tooling Layer: Documentation, examples, dev tools labels Aug 10, 2026
@Git-on-my-level Git-on-my-level removed the docs-tooling Layer: Documentation, examples, dev tools label Aug 10, 2026
@aryanorastar

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level @kodjima33 workflow-review ping: current head is MERGEABLE with all checks green; the only remaining gate is write-access/code-owner approval for the X connector Cloud Run Job workflow. Please approve or state an explicit blocker.

aryanorastar added a commit to aryanorastar/omi that referenced this pull request Aug 13, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@aryanorastar

Copy link
Copy Markdown
Contributor Author

Refreshed onto current main (1d4ff45b02). Kept the X-connector-sync job split in backend/AGENTS.md and the job render tests; took main's retired canonical-memory env-var contract. Runtime-env tests 103 passed locally.

@Git-on-my-level workflow CO when ready.

aryanorastar and others added 9 commits August 16, 2026 20:42
Address David CR on BasedHardware#11183: the dedicated sync job must refresh OAuth
tokens and use RapidAPI fallback. Also tighten scaffold/scheduler gates.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the Cloud Run Job service map concise and format the scheduler test
with the pinned Black version.

Failure-Class: none

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Keep generated scaffold guidance descriptive without creating untracked
TODO markers that fail repository metadata checks.

Failure-Class: none

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Surface listing failures and total per-user failure to Cloud Run via a
non-zero exit (memory-maintenance pattern) so Scheduler cannot look healthy
while X sync is broken.

Failure-Class: none
Co-authored-by: Cursor <cursoragent@cursor.com>
Split mangled x-connector render assertions into their own test (NameError
on jobs), and black-format the failure-surface files at line-length 120.

Failure-Class: none
Co-authored-by: Cursor <cursoragent@cursor.com>
Include gcp_x_connector_sync_job.yml and validate_x_connector_sync_scheduler.py
in backend_cloud_run_deploy.sources so source-closure tracks the 6h Scheduler invariant.

Co-authored-by: Cursor <cursoragent@cursor.com>
@aryanorastar
aryanorastar force-pushed the fix/9298-x-connector-sync-job branch from a51ce21 to d4787f5 Compare August 16, 2026 15:13
@aryanorastar

aryanorastar commented Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

tip refreshed onto main (d4787f5d) — MERGEABLE. Workflow CO path for X connector Cloud Run Job split (#9298). Please APPROVE when tip CI is green.

Scheduled X extraction Flex bindings and the gateway verify/probe now live on
the dedicated sync job; notifications-job no longer carries the leftover
gateway/Flex surface. Recomposed runtime_env.yaml for Hygiene.

Co-authored-by: Cursor <cursoragent@cursor.com>
@aryanorastar

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level tip after d4787f5 CI:

  • Hygieneruntime_env.yaml was out of date vs overlays; recomposed.
  • Static Contracts — validator still required X Flex (OMI_BACKGROUND_FLEX_CAPABLE / gateway URL / service token) on notifications-job after the ownership split. Moved that contract onto x-connector-sync-job, moved gateway verify + omi:auto:x-memory-extraction-flex probe into gcp_x_connector_sync_job.yml, and stripped the leftovers from notifications.

Local: focused runtime-env/x-job suite green; pre-deploy-check.sh passed. Please workflow CO when tip CI is green.

@aryanorastar

aryanorastar commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level @kodjima33

Conflicts with current main resolved on tip 5750272efc (clean merge of backend/AGENTS.md). Branch is MERGEABLE. Ready for review/merge.


Still current and still author-side clear. Verified against main on 2026-08-23: not superseded, still MERGEABLE, all checks green, no conflicts. Blocker is the CODEOWNERS workflow-review routing. Grouped with six sibling CI PRs in one consolidated ping at #10959 rather than bumping each thread separately.

…sync-job

# Conflicts:
#	backend/AGENTS.md
#	backend/tests/unit/test_notifications_job_orchestrator.py
#	backend/utils/other/jobs.py
@aryanorastar

Copy link
Copy Markdown
Contributor Author

Author-side work is complete. I refreshed the branch onto current main and fixed the new metadata failure on 49548234a1.

The red check came from backend/AGENTS.md exceeding its 39,000-byte merge-result budget. The X-job ownership guidance is now 38,900 bytes without dropping the contract, and the PR body cites the newly matched INV-DATA-1 and INV-MEM-4 paths with preservation statements.

Current-main verification: 32/32 PR preflight checks passed. The branch is mergeable and CI is rerunning. The remaining items are maintainer-owned: CODEOWNERS approval, then the documented first job/Scheduler deployment after merge.

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the continued updates — the split itself remains in good shape. This pass surfaced one real blocker on head b16d822a and one piece of good news about CI.

Blocker: scheduled X extraction silently dropped the gateway Flex routing

While you were iterating, main gained PromotionFlexRunRouter in scheduled X sync (a052a90f, "Expand shared Flex switch to scheduled memory work", Aug 16 06:08 UTC) — main's run_x_sync_job() constructs the router and passes background_flex= to sync_x_for_user(), so extraction runs through the x_memory_extraction_flex gateway lane when the shared switch is on.

Your rewritten backend/utils/x_connector.py::run_x_sync_job() (kept through the later main merges) never constructs the router, so extraction_llm is always None and scheduled extraction always takes the legacy get_llm('memories') direct path. That path only routes through the gateway with OMI_LLM_GATEWAY_FEATURE_MODE=gateway, which is not part of this job's contract.

The problem is that the PR's own deploy contract still requires Flex on this job, so the code and the contract now disagree:

  • backend/deploy/runtime_env/_base.yaml + composed runtime_env.yaml set OMI_BACKGROUND_FLEX_CAPABLE: 'true' and bind OMI_LLM_GATEWAY_URL / OMI_LLM_GATEWAY_SERVICE_TOKEN on x-connector-sync-job
  • backend/scripts/runtime_env_validation/manifest.py enforces both, with the rationale "so the shared live flag covers scheduled X extraction"
  • .github/workflows/gcp_x_connector_sync_job.yml runs the gateway serving gate and probes the omi:auto:x-memory-extraction-flex VPC lane before deploy

For comparison, the job this PR clones — memory-maintenance-job — constructs PromotionFlexRunRouter(force_enabled=...) in its cron, so the capability flag there matches code that actually reads it.

Could you either restore router construction in run_x_sync_job() (preferred, preserves current prod behavior), or, if dropping Flex routing from X extraction is intended, strip the capability flag, gateway token/URL bindings, the manifest.py rule, and the Flex-lane probe in the same change so the contract describes what the code does? As-is, the deploy guarantees something the runtime never uses, and the flag OMI_BACKGROUND_FLEX_CAPABLE (which promotion_flex_capable() reads) becomes dead env on this job.

CI: the red "Backend unit suite" is not yours

The failure is pyright reportUnusedImport ×2 at backend/utils/stt/streaming.py:37 (SafeSonioxSocket, process_audio_soniox) — a file this PR doesn't touch. Main introduced it in 7c900a93 (Aug 27 14:25 UTC), your branch merged main at 17:13, and main already fixed it in 2af0832 (#12084, 18:18 UTC). Main's own unit runs failed on the same window and pass from 2af0832 onward. A main refresh should clear it without any change here. (I checked the pytest portion of the run — it passed.)

What still looks good on this head

  • backend/modal/x_connector_sync_job.py — thin entrypoint, always runs run_x_sync_job(), exits non-zero via raise_if_x_sync_job_failed(summary); backend/utils/x_connector.py now surfaces listing failures and all-user failure instead of a silent healthy no-op.
  • backend/utils/other/jobs.py — clean removal of the X import/gate; no dangling should_run_x_sync_job references anywhere.
  • backend/deploy/runtime_env.yaml + _base.yaml/overlays — the ownership split is correct end-to-end: notifications-job keeps only the secrets its remaining code reads (I verified utils/other/notifications.py and the materialization-health check use no Pinecone/gateway/X env), and all X OAuth/RapidAPI/Pinecone/Flex bindings move to the new job.
  • backend/scripts/validate_x_connector_sync_scheduler.py — read-only, validates the exact 6h/OAuth-SA/target contract; the workflow only validates (never creates) the Scheduler job.
  • backend/scripts/scaffold_cloud_run_job.sh — dry-run by default, creates no GCP/Secret/Scheduler resources; template references all resolve.
  • .github/workflows/gcp_x_connector_sync_job.yml — dispatch-only, environment-gated, image smoke-verified before push, deploy concurrency lock registered.

Agent-facing docs (flagged for maintainers)

backend/AGENTS.md and docs/doc/developer/backend/cloud_run_jobs_checklist.md change agent guidance: future coding/review agents will be steered toward one periodic domain per Cloud Run Job, cloning the landed templates, and away from adding unrelated work to notifications-job. I checked the wording against the tree — it's accurate and matches issue #9298's intent — but it is durable agent-facing policy from an outside contributor, so a maintainer should consciously accept it.

Leaving for human maintainer review: the Flex routing decision above (restore vs. deliberately strip the contract), and post-merge rollout — the Scheduler SA + x-connector-sync-6h creation and the ordered deploy in the runbook can't be verified from code.


by AI on behalf of David — requesting changes for the Flex routing/contract mismatch on the scheduled X sync path; needs a maintainer decision on restore-vs-strip before the new job owns prod sync.

…sync-job

# Conflicts:
#	backend/runtime_images.json
#	backend/tests/unit/test_render_backend_runtime_env.py
Restore the job-level PromotionFlexRunRouter that the Cloud Run split dropped, pass one shared run router to every connected user, and preserve deliberate PromotionFlexDeferred outcomes without counting them as user failures.\n\nThe behavioral regression test drives run_x_sync_job through an injected registry, asserts the router is constructed with the job start time and reused for every user, and proves a capacity deferral leaves failed=0.\n\nVerification:\n- focused X scheduler suites: 16 passed\n- X/runtime/deployment contract suites: 162 passed\n- full backend suite exercised; X suites passed, while five files identical to origin/main remain red on unrelated current-main STT/test-isolation/inventory failures\n\nFailure-Class: none
@aryanorastar

aryanorastar commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level Resolved the Flex-routing blocker in 12a538f73d and refreshed the branch onto current main (8f9a67658a).

What changed

  • run_x_sync_job(*, job_started_at=...) now constructs one PromotionFlexRunRouter for the Cloud Run execution.
  • The same router is passed to every sync_x_for_user(..., background_flex=...) call, restoring the x_memory_extraction_flex gateway lane and its stale-result fence.
  • PromotionFlexDeferred is handled separately: the raw source remains pending for retry and the intentional capacity deferral is not counted as a failed user. Actual user failures still feed this PR's nonzero Cloud Run failure surface.

Regression proof

Added a behavioral test (not a source-string tripwire) that runs the real run_x_sync_job() coordinator with two registry users and asserts:

  1. the router is constructed exactly once with the injected job start time;
  2. both users receive the identical router instance;
  3. a capacity-deferring second user produces failed == 0, while the successful user and post count are retained.

Verification on the updated head:

  • focused X scheduler/failure suites: 16 passed;
  • X + runtime-env + deployment contract suites: 162 passed;
  • bounded pre-push acceptance gate: passed, including pyright (0 errors), runtime-image source closure for all 13 images, and the selected eight backend suites;
  • full backend/test.sh sweep exercised all 992 files; both X suites passed. Its remaining failures reproduce in files byte-identical to current origin/main (legacy-memory ratchet, retired STT expectations, and two test module-stub isolation imports), so none are introduced by this PR.

No physical-device test applies: this is the backend scheduled Cloud Run path. Live Scheduler/deploy verification remains the explicit post-merge maintainer step already listed in the PR.

@aryanorastar

aryanorastar commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@Git-on-my-level @undivisible CI follow-up: the three remaining red checks are reproducible on the current main and are not caused by this X connector diff.

  • Repo Checks / Hygiene and PR Metadata Preflight: a documentation-only InsightAssistant mention increments the legacy-memory token ratchet from 4 to 5.
  • Backend Unit Tests: the two hermetic app-integration harnesses do not stub the newly imported utils.llm.temporal, so collection fails before their tests run.

I isolated the main-wide repair in #12339. Proof on that branch:

  • exact backend/test.sh reproduction for all three CI-reported files: 35 passed
  • legacy-memory ratchet: PASS (892 findings across 82 path counters)
  • deterministic preflight: 26/26 selected checks passed
  • bounded pre-push gate: passed, including pyright, runtime-image closure, focused backend tests, and formatting

The X-specific suites on this PR remain green, including the shared per-run Flex router regression added in 12a538f73d. Once #12339 lands and this branch updates from main, these unrelated red checks should clear.

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

Labels

backend Backend Task (python) human Human-authored pull request needs-maintainer-review Needs a human maintainer to sign off before merge workflow-review Needs maintainer review for workflow, automation, hooks, or CI behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Split X connector sync out of notifications-job; document Cloud Run Job pattern + scaffold

3 participants