fix: Slack permission sync test waits for the private doc

Merged
EV
1d ago
+56-25

Description

Evidence. test_slack_permission_sync fails in CI in two ways:

  • assert "Sara's favorite number ..." not in [...]: user_1 still sees the private message after the kick and perm sync.
  • TimeoutError: Permission sync was not completed within 300 seconds: about 9 stack PRs ran the Slack job in the same minute on one Slack app. Every Slack call got a 429 with retry-after=30.

Also, a failed first attempt can never get a full second attempt. The test step retries 3 times with 20 minutes each, but the job stops after 25 minutes.

Cause.

  • CCPairManager.wait_for_sync waited until any N docs had last_synced >= last_modified >= after. The public doc could fill that count while the private doc's index update was still waiting for the next doc-sync beat. The search then saw the old ACL.
  • The Slack channel pool allowed 20 slots, so it never limited concurrency.
  • The job timeout-minutes did not fit the retry budget.

Fix.

  • wait_for_sync takes an optional doc_ids. The index wait now also waits until each listed doc is synced. The timeout error names the missing docs. Other callers do not pass it, so their behavior does not change.
  • SlackManager.add_message_to_channel returns the Onyx doc id of the posted message (built with the connector's _build_doc_id). The test passes the private message's doc id to the post-kick wait. The earlier "user_1 can see it" check does not have this problem: indexing writes the ACL, and the test waits for indexing to complete.
  • Slack channel pool: _MAX_POOL_SLOTS 20 → 3, so extra runs queue for a slot. A slot is held for one test (usually 1-3 minutes), so the 10-minute claim timeout queues a run behind several full turns of the pool. The busy-slot poll goes from 15 s to 30 s, because each poll reads the lock channel with conversations.history, which the running tests also use. Slots 3-19 stay unused. The sweep does not touch pool channels.
  • Workflow: the integration-tests matrix job timeout goes from 25 to 70 minutes (3 × 20 min attempts, plus setup of about 3-6 min and log upload). This keeps the 20-minute attempt: the slowest suites take about 12 minutes, and the Slack suite can now wait for a pool slot. One job definition serves the full matrix, so this also covers reindex_port and the other connector suites. The onyx-lite-tests job had the same mismatch (3 × 20 min in a 10-minute job, for a 2-minute suite). It now uses 3 × 8 min attempts in a 30-minute job.

The workflow file and the Slack test dir are in the slack paths filter, so this PR runs the Slack job.

How Has This Been Tested?

  • A local check with mocked API calls ran wait_for_sync. With doc_ids, it waited until the private doc was synced. Without doc_ids, it returned as soon as the public doc was synced, which is the old flaw. On timeout, the error lists the missing doc id.
  • pre-commit passed on the changed files (ruff, ty, actionlint, zizmor).
  • I did not run the live Slack test locally: it resets the shared local stack. CI runs the Slack job on this PR.

Additional Options

  • [Optional] Please cherry-pick this PR to the latest release version.
  • [Optional] Override Linear Check

🤖 Generated with Claude Code

Summary by cubic

Fixes the flaky Slack permission sync test that failed when the public doc synced before the private doc's ACL change, and when concurrent Slack runs hit API rate limits.

  • wait_for_sync now accepts doc_ids to wait until each specified doc is synced; the timeout error lists missing docs.
  • add_message_to_channel returns the Onyx doc id of the posted message via the now-public build_slack_doc_id, so the test can wait on the private doc.
  • Slack channel pool slots dropped from 20 to 3 to queue concurrent runs, and claims are released on any failure so slots free before the TTL.
  • Integration job timeouts now fit all retry attempts (70 min for the main matrix, 30 min for onyx-lite); the lite suite's retry timeout drops from 20 to 8 min.

Summary by CodeRabbit

  • Tests

    • Improved integration test validation to confirm specific Slack documents are indexed before completion.
    • Enhanced coverage for Slack permission synchronization in private channels.
    • Updated test timing and concurrency settings to better accommodate longer-running integrations and service rate limits.

  • Chores

    • Adjusted integration test timeouts and retry behavior to reduce premature failures during longer-running test suites.
    • Improved cleanup handling when Slack synchronization attempts are interrupted or encounter errors.