Repository navigation
Fix Slack OAuth live canary setup - #5773
Conversation
⏳ IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted review state before this projection. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughSlack live QA now materializes Slack setup and personal OAuth inputs, applies Slack setup after server start, strips legacy Slack config fields, gates select cases on personal-auth readiness, and updates the runner, docs, tests, and SQLite connection handling. ChangesSlack live QA migration
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
scope: ci
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Slack integration setup for the Reborn WebUI v2 live QA runner by removing legacy Slack setup fields from config.toml and shifting to headlessly applying the bot installation setup via a PUT API call after the server boots. It also wraps SQLite connection usages with contextlib.closing across helper files to ensure proper resource cleanup. Feedback on the changes highlights two issues: in _slack_setup_payload, oauth_client_id should be retrieved from the preflight dictionary rather than directly from the environment to maintain consistency, and in test_run_live_qa.py, a second database insert is executed after db.commit() but is never committed, causing SQLite to roll it back and rendering the test assertion vacuous.
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.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Verdict: ❌ Changes requested
Findings: 1 blocking / 0 notes
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Head: 64c773e749af1c265ea870b3ad01f03b736ffad2
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Found one blocking issue: the updated Slack connect live QA can pass without Slack personal OAuth being configured or exercised.
Findings
1. ❌ [MEDIUM] Slack connect QA passes without OAuth readiness
Location: scripts/reborn_webui_v2_live_qa/run_live_qa.py:2051-2063
The Slack connect case now treats the always-advertised oauth connectable-channel descriptor as success, but it never checks preflight['setup']['personal_oauth_ready'] or starts the Slack OAuth flow. The Reborn descriptor is returned whenever Slack connectable channels are mounted, even if the setup API did not receive REBORN_WEBUI_V2_LIVE_QA_SLACK_OAUTH_CLIENT_ID/secret or the runtime lacks the Slack personal OAuth slot, so qa_3a/qa_5a/qa_8a can go green while real Slack OAuth connect is broken. Gate this case on personal OAuth readiness and drive the actual OAuth start path (or fail/skip explicitly when the OAuth inputs are absent).
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
✅ Action performedFull review finished. |
|
@coderabbitai full review |
|
@ironloopai review --agent reviewer |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Verdict: ❌ Changes requested
Findings: 1 blocking / 0 notes
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Head: 7fac454357b12aa77d9c3ede6a8ea03ea04594fe
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Found one blocking issue: the new Slack personal OAuth live QA path saves client credentials but never starts the Reborn server with the Slack OAuth redirect URI required to mount the OAuth provider, so the Slack connect cases will fail at oauth/start in CI.
Findings
1. ❌ [MEDIUM] Set Slack personal OAuth redirect before starting Reborn
Location: scripts/reborn_webui_v2_live_qa/run_live_qa.py:717-724
ironclaw-reborn serve only wires the Slack personal OAuth slot when IRONCLAW_REBORN_SLACK_PERSONAL_OAUTH_REDIRECT_URI is present, but this start-up path only synthesizes the Google redirect URI. The workflow added Slack OAuth client id/secret env vars, and _slack_connect_case now posts to /api/webchat/v2/extensions/slack/setup/oauth/start; without the Slack redirect env, the server starts without the Slack personal OAuth provider and that endpoint fails closed even after _apply_slack_setup_api_after_start saves the credentials. Synthesize IRONCLAW_REBORN_SLACK_PERSONAL_OAUTH_REDIRECT_URI from base_url when Slack OAuth credentials are present, similar to the Google redirect handling.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
Inline review fallback
Inline comment projection fell back to a body-only PR Review because GitHub rejected the inline payload.
Reason: Unprocessable Entity: "Line could not be resolved" - https://docs.github.com/rest/pulls/reviews#create-a-review-for-a-pull-request
IronLoop preserved the inline review comment payloads below instead of dropping them.
Inline fallback 1: scripts/reborn_webui_v2_live_qa/run_live_qa.py:717
This startup path needs to also populate IRONCLAW_REBORN_SLACK_PERSONAL_OAUTH_REDIRECT_URI when Slack personal OAuth credentials are configured. The Reborn runtime only creates the Slack personal OAuth slot if that env var is set, and the new connect case calls /api/webchat/v2/extensions/slack/setup/oauth/start; saving client credentials through the setup API after startup is not enough to mount the provider.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 789-794: The Slack setup failure path in run_live_qa.py is logging
the API response body, which can leak sensitive bot/signing/OAuth data into
live-QA logs. Update the LiveQaError handling around the Slack setup request so
it does not include response.text or any echoed payload; keep only the HTTP
status and a generic failure message, and preserve the response-body handling
only in local variables if needed for debugging without logging.
In `@scripts/reborn_webui_v2_live_qa/slack_helpers.py`:
- Around line 531-536: The setup payload is omitting the stored Slack OAuth
client ID when only the durable setup record has it, even though
_slack_setup_preflight can mark personal OAuth ready from that saved state.
Update the payload-building logic in slack_helpers.py so the code around
oauth_client_id/oauth_client_secret pulls the client ID from the persisted setup
record (or equivalent stored source) when the env var is absent, and then always
includes oauth_client_id in the request when a valid stored value exists. Use
the existing _env_value and payload assembly path to keep behavior consistent
for oauth_client_secret.
- Around line 158-162: The `_discover_slack_dm_route_channel` helper has a stale
unused `config_text` parameter, which triggers Ruff ARG001. Remove `config_text`
from the function signature in `slack_helpers.py` and update the call site in
`run_live_qa.py` so it passes only the remaining arguments; keep the token
lookup via `SLACK_BOT_TOKEN_ENV` intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 01f7a7db-0542-401e-ba4b-4538f1d787aa
📒 Files selected for processing (8)
.github/workflows/live-canary.ymldocs/internal/live-canary.mdscripts/reborn_webui_v2_live_qa/external_auth_helpers.pyscripts/reborn_webui_v2_live_qa/google_auth_helpers.pyscripts/reborn_webui_v2_live_qa/root_filesystem.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/slack_helpers.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
|
@coderabbitai full review |
|
@ironloopai review --agent reviewer |
✅ Action performedFull review finished. |
|
@ironloopai review --agent reviewer |
✅ Action performedFull review finished. |
|
🚅 Deployed to the ironclaw-pr-5773 environment in ironclaw-ci-preview
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
@ironloopai review --agent reviewer |
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Verdict: ✅ Approved
Findings: 0 blocking / 0 notes
Next: No reviewer action needed.
Head: 844b987390b71a16c5914260c8ef52eeb1dd8599
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the PR changes. The Slack live-QA updates are scoped to the canary workflow, harness helpers, docs, and unit coverage.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/stale/stalled state while reviewers run.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 4503-4513: The Slack setup API step in the per-case loop is not
isolated, so a LiveQaError from _apply_slack_setup_api_after_start can abort the
entire shard instead of failing just the current case. Wrap the setup-api call
in the same local failure handling used for the other preconditions in
run_live_qa.py, and on exception record a failed per-case _result(..., False,
...) and continue to the next selected case. Keep the fix scoped around
_apply_slack_setup_api_after_start, slack_preflight, and the surrounding for
name in selected_cases loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 00e952df-6b83-4891-a677-656033051364
📒 Files selected for processing (9)
.github/workflows/live-canary.ymldocs/internal/live-canary.mdscripts/reborn_webui_v2_live_qa/case_matrix.pyscripts/reborn_webui_v2_live_qa/external_auth_helpers.pyscripts/reborn_webui_v2_live_qa/google_auth_helpers.pyscripts/reborn_webui_v2_live_qa/root_filesystem.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/slack_helpers.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/reborn_webui_v2_live_qa/run_live_qa.py (1)
4488-4517: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSlack setup-API failure still takes down the whole shard — prior fix not applied.
Every other precondition in this loop (Telegram/GitHub/Google/Slack-bot/personal-auth/target) converts failure to a per-case
_result(..., False, ...)+continue._apply_slack_setup_api_after_starthas no localexcept; aLiveQaErrorhere propagates pastfor name in selected_cases:and is only caught bymain()'s top-level handler, killing every remaining case in the shard with one opaque aggregate failure. It also carries Slack bot/signing/OAuth secrets in its payload — an unhandled failure here is exactly the "external-call hazard" class that should be case-scoped, not shard-fatal.🔧 Proposed fix: isolate the failure to this case
if case_spec.requires_slack and isinstance(slack_preflight, dict): - setup_api = await _apply_slack_setup_api_after_start( - base_url=base_url, - prepared_home=prepared_home, - ) + try: + setup_api = await _apply_slack_setup_api_after_start( + base_url=base_url, + prepared_home=prepared_home, + ) + except LiveQaError as exc: + result = _result( + name, + False, + started, + {"blocked": True, "error": f"Slack setup API failed: {exc}"}, + ) + results.append(result) + print( + f"[reborn-webui-v2-live-qa] case={name} success=False " + f"blocked=slack_setup_api_failed", + flush=True, + ) + continue slack_preflight["setup_api"] = setup_api🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py` around lines 4488 - 4517, The Slack setup API call in the per-case loop is still shard-fatal because `_apply_slack_setup_api_after_start` can raise past `for name in selected_cases` and bypass the case-scoped failure handling used elsewhere. Add a local `try/except` around the Slack setup block in `run_live_qa.py` (the `LiveQaContext` / `slack_preflight` section), catch `LiveQaError` or related setup exceptions, record a per-case `_result(..., False, ...)` with the error, and `continue` so the shard keeps processing later cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/reborn_webui_v2_live_qa/slack_helpers.py`:
- Around line 897-1028: The _slack_personal_auth_preflight function is doing too
much inside the per-account decrypt-and-auth.test section, which triggers the
Ruff complexity warnings. Extract that inner account-processing block into a
dedicated helper near _slack_personal_auth_preflight (for example, one that
builds the account_preflight dict and runs _root_filesystem_secret_by_handle,
_decrypt_filesystem_secret, and _slack_user_token_auth_test). Keep
_slack_personal_auth_preflight focused on iterating rows, collecting results,
and setting the final ready/reason fields.
---
Duplicate comments:
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 4488-4517: The Slack setup API call in the per-case loop is still
shard-fatal because `_apply_slack_setup_api_after_start` can raise past `for
name in selected_cases` and bypass the case-scoped failure handling used
elsewhere. Add a local `try/except` around the Slack setup block in
`run_live_qa.py` (the `LiveQaContext` / `slack_preflight` section), catch
`LiveQaError` or related setup exceptions, record a per-case `_result(...,
False, ...)` with the error, and `continue` so the shard keeps processing later
cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 64d72a44-5ae8-4781-a379-234c89d08ae4
📒 Files selected for processing (9)
.github/workflows/live-canary.ymldocs/internal/live-canary.mdscripts/reborn_webui_v2_live_qa/case_matrix.pyscripts/reborn_webui_v2_live_qa/external_auth_helpers.pyscripts/reborn_webui_v2_live_qa/google_auth_helpers.pyscripts/reborn_webui_v2_live_qa/root_filesystem.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/slack_helpers.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
There was a problem hiding this comment.
♻️ Duplicate comments (2)
scripts/reborn_webui_v2_live_qa/run_live_qa.py (1)
4488-4531: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSlack setup-API failure still isolates nothing — one case can abort the whole run.
Same gap flagged previously: every other precondition check in this loop (Telegram/GitHub/Google/Slack-bot/personal-auth/target) converts failure into a per-case
_result(..., False, ...)+continue. The_apply_slack_setup_api_after_startcall at Line 4504 still has no localtry/except, so aLiveQaErrorraised inside it (HTTP failure, non-2xx status, bad JSON) propagates past thefor name in selected_cases:loop and is only caught bymain()'s top-level handler, failing every remaining case in this invocation instead of just the current one.🔧 Proposed fix: isolate the failure to this case
if case_spec.requires_slack and isinstance(slack_preflight, dict): - setup_api = await _apply_slack_setup_api_after_start( - base_url=base_url, - prepared_home=prepared_home, - ) + try: + setup_api = await _apply_slack_setup_api_after_start( + base_url=base_url, + prepared_home=prepared_home, + ) + except LiveQaError as exc: + result = _result( + name, + False, + started, + {"blocked": True, "error": f"Slack setup API failed: {exc}"}, + ) + results.append(result) + print( + f"[reborn-webui-v2-live-qa] case={name} success=False " + f"blocked=slack_setup_api_failed", + flush=True, + ) + continue slack_preflight["setup_api"] = setup_api🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py` around lines 4488 - 4531, The Slack setup-API step in the per-case loop still isn’t isolated, so a LiveQaError from _apply_slack_setup_api_after_start can abort the entire run. Add a local try/except around the Slack setup path in the selected-cases loop, similar to the other preflight checks, and on failure append a per-case _result(..., False, ...) for the current case and continue. Keep the handling inside run_live_qa.py near LiveQaContext, _apply_slack_setup_api_after_start, and CASES[name].fn so only the current case is marked failed.scripts/reborn_webui_v2_live_qa/slack_helpers.py (1)
453-524: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant SQLite opens per preflight call (still unaddressed).
_slack_setup_preflightopens/parses_slack_setup_from_reborn_homedirectly at Line 478, then calls_slack_setup_fieldthree more times (Lines 479, 485, 491), each of which independently re-opens and re-parses the same JSON via its own_slack_setup_from_reborn_home(reborn_home)call at Line 460 — 4 DB round-trips for one preflight.♻️ Proposed fix
-def _slack_setup_field( - reborn_home: Path, - config_text: str, - field: str, - env_name: str, - default: str = "", -) -> str | None: - setup = _slack_setup_from_reborn_home(reborn_home) or {} +def _slack_setup_field( + setup: dict[str, object], + config_text: str, + field: str, + env_name: str, + default: str = "", +) -> str | None: value = str(setup.get(field) or "").strip()Then load
setup = _slack_setup_from_reborn_home(reborn_home) or {}once per caller and pass it through to each_slack_setup_fieldcall.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/reborn_webui_v2_live_qa/slack_helpers.py` around lines 453 - 524, The Slack preflight path is re-opening and re-parsing the same setup data multiple times. Update `_slack_setup_preflight` and `_slack_setup_field` in `slack_helpers.py` so `_slack_setup_from_reborn_home(reborn_home)` is loaded once in the caller and reused for `installation_id`, `team_id`, and `api_app_id` instead of each `_slack_setup_field` call fetching it again. Adjust `_slack_setup_field` to accept the cached setup object and use it directly, preserving the existing fallback order from setup data, env secrets, config text, then default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 4488-4531: The Slack setup-API step in the per-case loop still
isn’t isolated, so a LiveQaError from _apply_slack_setup_api_after_start can
abort the entire run. Add a local try/except around the Slack setup path in the
selected-cases loop, similar to the other preflight checks, and on failure
append a per-case _result(..., False, ...) for the current case and continue.
Keep the handling inside run_live_qa.py near LiveQaContext,
_apply_slack_setup_api_after_start, and CASES[name].fn so only the current case
is marked failed.
In `@scripts/reborn_webui_v2_live_qa/slack_helpers.py`:
- Around line 453-524: The Slack preflight path is re-opening and re-parsing the
same setup data multiple times. Update `_slack_setup_preflight` and
`_slack_setup_field` in `slack_helpers.py` so
`_slack_setup_from_reborn_home(reborn_home)` is loaded once in the caller and
reused for `installation_id`, `team_id`, and `api_app_id` instead of each
`_slack_setup_field` call fetching it again. Adjust `_slack_setup_field` to
accept the cached setup object and use it directly, preserving the existing
fallback order from setup data, env secrets, config text, then default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 64d72a44-5ae8-4781-a379-234c89d08ae4
📒 Files selected for processing (9)
.github/workflows/live-canary.ymldocs/internal/live-canary.mdscripts/reborn_webui_v2_live_qa/case_matrix.pyscripts/reborn_webui_v2_live_qa/external_auth_helpers.pyscripts/reborn_webui_v2_live_qa/google_auth_helpers.pyscripts/reborn_webui_v2_live_qa/root_filesystem.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/slack_helpers.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/reborn_webui_v2_live_qa/run_live_qa.py`:
- Around line 4507-4517: The Slack preflight flow in the live QA runner records
the setup result even when `_apply_slack_setup_api_after_start` returns a
non-applied response, which allows Slack cases to continue after a setup
precondition failure. Update the Slack setup handling in `run_live_qa.py` so the
case is explicitly blocked or failed when the returned setup API payload
indicates `applied` is false, and only persist `slack_preflight["setup_api"]`,
`slack_preflight["setup"]`, and copy the preflight artifacts after a successful
applied setup. Use `_apply_slack_setup_api_after_start`, `slack_preflight`, and
`write_preflight` as the key symbols to locate and adjust this branch.
- Around line 751-761: Update the Slack redirect URI setup in
`_slack_setup_preflight` so it also runs when OAuth is considered ready from the
stored client ID copied by setup, not only when `SLACK_OAUTH_CLIENT_ID_ENV` is
present in `process_extra_env`. Adjust the conditional that sets
`IRONCLAW_REBORN_SLACK_PERSONAL_OAUTH_REDIRECT_URI` to account for the
copied/stored `oauth_client_id` path, ensuring homes with copied setup plus env
client secret still receive the callback URI before OAuth start.
In `@scripts/reborn_webui_v2_live_qa/slack_helpers.py`:
- Around line 497-500: The OAuth client ID precedence in the Slack helpers is
wrong: the copied-home value from setup is overriding the env-provided client ID
even though the client secret comes from the env, which can create a mismatched
OAuth pair. Update the fallback logic in the Slack OAuth config path (around the
oauth_client_id / oauth_client_secret handling) so the env value from
SLACK_OAUTH_CLIENT_ID_ENV is preferred before any copied-home setup value,
keeping the pair consistent with the env secret. Use the existing helper symbols
in slack_helpers.py for the OAuth config flow to make the change in the same
code path.
- Around line 923-955: Scope the Slack personal-auth lookup in slack_helpers.py
so it only considers rows for the current user, not just rows with a matching
scope.user_id when present. Update the account filtering in the callback-account
scan to explicitly reject rows missing scope.resource.user_id or otherwise
unable to prove ownership, using the existing rows/account_user_id logic in the
same block. Keep the check tied to the current user_id so malformed or legacy
records cannot make another user’s Slack token appear ready.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d11433c-59e0-459b-9ad1-f5f5ab474224
📒 Files selected for processing (9)
.github/workflows/live-canary.ymldocs/internal/live-canary.mdscripts/reborn_webui_v2_live_qa/case_matrix.pyscripts/reborn_webui_v2_live_qa/external_auth_helpers.pyscripts/reborn_webui_v2_live_qa/google_auth_helpers.pyscripts/reborn_webui_v2_live_qa/root_filesystem.pyscripts/reborn_webui_v2_live_qa/run_live_qa.pyscripts/reborn_webui_v2_live_qa/slack_helpers.pyscripts/reborn_webui_v2_live_qa/test_run_live_qa.py
This branch was successfully deployed
1 participant
Summary
[slack]setup fields thatironclaw-reborn servenow rejectsslack_personalOAuth flow headlesslyChange Type
Linked Issue
None.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo build -p ironclaw_reborn_cli --features webui-v2-beta,slack-v2-host-beta --bin ironclaw-rebornpython3 scripts/reborn_webui_v2_live_qa/test_run_live_qa.pycargo test --features integrationif database-backed or integration behavior changedPUT /api/webchat/v2/channels/slack/setupreturns configured + personal OAuth readyreview-prorpr-shepherd --fixwas run before requesting review: CodeRabbit full review and IronLoop reviewer requested from PR commentsSecurity Impact
Touches Slack bot/signing/OAuth secret handling in the live QA harness. Secrets remain materialized via the existing
*_PATHconvention, the setup API call uses the local WebUI bearer token, and Slack setup API failure messages now omit response bodies so echoed payloads cannot leak to CI logs.Reborn Trust-Boundary Checklist
serde(default)fields fail closed or have migration tests. N/A, no schema/default change.Transient,Permanent,Misconfigured,PolicyDeniedor equivalent). Harness setup failures are explicit misconfiguration/HTTP setup failures; secret-bearing response bodies are omitted.slack_personalproduct-auth provider naming.Database Impact
No schema or migration changes. The live QA harness writes/reads existing local-dev libSQL root-filesystem records for Slack setup, product-auth callback accounts, and Slack personal identity/DM binding state.
Blast Radius
Reborn WebUI v2 live QA only: workflow env materialization, live QA Slack helper logic, and internal live-canary docs. The main runtime code is not changed.
Rollback Plan
Revert this PR to restore the previous live QA harness behavior. If this lands and causes canary failures, disable or narrow the Reborn WebUI v2 Slack cases while reverting the harness update; no production data migration is involved.
Review Follow-Through
Addressed Gemini, IronLoop, and CodeRabbit findings with follow-up commits. Remaining live validation is the secret-backed Reborn WebUI v2 canary run on this PR head.
Review track: C (security/runtime/DB/CI)