Skip to content

[codex] Fix Reborn WebUI v2 Slack connect canary #5485

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
serrrfirat merged 1 commit into from
Jul 1, 2026
Merged

[codex] Fix Reborn WebUI v2 Slack connect canary #5485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 47 additions & 11 deletions scripts/reborn_webui_v2_live_qa/run_live_qa.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1653,22 +1653,58 @@ async def _slack_connect_case(ctx: LiveQaContext, *, case_name: str) -> ProbeRes
from playwright.async_api import expect

started = time.monotonic()
prompt = _qa_sheet_prompt(case_name)
observed: dict[str, object] = {"chat_connect_prompt": prompt}
observed: dict[str, object] = {
"qa_sheet_prompt": _qa_sheet_prompt(case_name),
"slack_connect_surface": "/v2/extensions/channels",
}

async def action(page: object) -> None:
await page.goto(
f"{ctx.base_url}/v2/?token={AUTH_TOKEN}",
f"{ctx.base_url}/v2/extensions/channels?token={AUTH_TOKEN}",
wait_until="domcontentloaded",
) # type: ignore[attr-defined]
composer = page.locator("[data-testid='chat-composer']") # type: ignore[attr-defined]
await expect(composer).to_be_visible(timeout=15000)
await composer.fill(prompt)
await composer.press("Enter")
body = page.locator("body") # type: ignore[attr-defined]
await expect(body).to_contain_text("Connect Slack", timeout=15000)
await expect(body).to_contain_text("Message the Slack app", timeout=15000)
observed["slack_connect_card_visible"] = True
await expect(page.locator("body")).to_contain_text("Channels", timeout=15000) # type: ignore[attr-defined]
body = await _fetch_webui_json(page, "/api/webchat/v2/channels/connectable")
channels = body.get("channels")
if not isinstance(channels, list):
raise AssertionError(f"connectable channels body did not include a list: {body!r}")
slack_channels = [
channel
for channel in channels
if isinstance(channel, dict) and channel.get("channel") == "slack"
]
observed["connectable_channel_count"] = len(channels)
observed["slack_strategy_count"] = len(slack_channels)
observed["slack_strategies"] = [
channel.get("strategy")
for channel in slack_channels
if isinstance(channel, dict)
]
personal = next(
(
channel
for channel in slack_channels
if isinstance(channel, dict)
and channel.get("strategy") == "inbound_proof_code"
),
None,
)
if not isinstance(personal, dict):
raise AssertionError(f"Slack inbound_proof_code connect strategy missing: {channels!r}")
action_body = personal.get("action")
if not isinstance(action_body, dict):
raise AssertionError(f"Slack connect action missing: {personal!r}")
title = str(action_body.get("title") or "")
if not title:
raise AssertionError(f"Slack connect action title missing: {personal!r}")
instructions = str(action_body.get("instructions") or "")
if "Message the Slack app" not in instructions:
raise AssertionError(f"unexpected Slack connect instructions: {instructions!r}")
await expect(page.locator("body")).to_contain_text(title, timeout=15000) # type: ignore[attr-defined]
await expect(page.locator("body")).to_contain_text("Message the Slack app", timeout=15000) # type: ignore[attr-defined]
observed["slack_display_name"] = personal.get("display_name")
observed["slack_connect_title"] = title
observed["slack_connect_instructions"] = instructions

try:
slack = _slack_preflight(ctx)
Expand Down
139 changes: 139 additions & 0 deletions scripts/reborn_webui_v2_live_qa/test_run_live_qa.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,145 @@ def locator(self, selector: str):
self.assertFalse(absent_result)
self.assertFalse(absent.clicked)

def test_slack_connect_case_uses_extensions_channels_surface(self):
class FakePage:
def __init__(self) -> None:
self.gotos: list[tuple[str, str | None]] = []

async def goto(self, url: str, wait_until: str | None = None) -> None:
self.gotos.append((url, wait_until))

def locator(self, selector: str) -> str:
return selector

class FakeExpectation:
def __init__(self, selector: str) -> None:
self.selector = selector

async def to_contain_text(
self,
text: str,
timeout: int | None = None,
) -> None:
expected_texts.append((self.selector, text, timeout))

def fake_expect(selector: str) -> FakeExpectation:
return FakeExpectation(selector)

async def fake_with_page(
_output_dir: Path,
_case_name: str,
action,
) -> None:
await action(fake_page)

async def fake_fetch_webui_json(_page: object, path: str) -> dict[str, object]:
fetched_paths.append(path)
return {
"channels": [
{
"channel": "slack",
"display_name": "Slack",
"strategy": "admin_managed_channels",
"action": {"title": "Choose Slack channel"},
},
{
"channel": "slack",
"display_name": "Slack",
"strategy": "inbound_proof_code",
"action": {
"title": "Slack account connection",
"instructions": "Message the Slack app, then enter the code here.",
},
},
]
}

with tempfile.TemporaryDirectory() as tmpdir:
output_dir = Path(tmpdir)
(output_dir / "preflight.json").write_text(
json.dumps(
{
"checks": {
"slack": {
"enabled_in_config": True,
"env_present": True,
"auth_test": {
"ok": True,
"team_id": "T123",
"user_id": "U123",
},
}
}
}
),
encoding="utf-8",
)
fake_page = FakePage()
fetched_paths: list[str] = []
expected_texts: list[tuple[str, str, int | None]] = []
playwright_module = types.ModuleType("playwright")
playwright_async_api = types.ModuleType("playwright.async_api")
playwright_async_api.expect = fake_expect
ctx = run_live_qa.LiveQaContext(
base_url="http://127.0.0.1:3000",
output_dir=output_dir,
reborn_home=output_dir / "reborn-home",
env={},
)

with (
patch.dict(
sys.modules,
{
"playwright": playwright_module,
"playwright.async_api": playwright_async_api,
},
),
patch.object(run_live_qa, "_with_page", new=fake_with_page),
patch.object(
run_live_qa,
"_fetch_webui_json",
new=fake_fetch_webui_json,
),
):
result = asyncio.run(
run_live_qa._slack_connect_case(
ctx,
case_name="qa_3a_slack_connect",
)
)

self.assertTrue(result.success, result.details)
self.assertEqual(
fake_page.gotos,
[
(
"http://127.0.0.1:3000/v2/extensions/channels?"
f"token={run_live_qa.AUTH_TOKEN}",
"domcontentloaded",
)
],
)
self.assertEqual(
fetched_paths,
["/api/webchat/v2/channels/connectable"],
)
observed_expectations = [text for _selector, text, _timeout in expected_texts]
self.assertIn("Channels", observed_expectations)
self.assertIn("Slack account connection", observed_expectations)
self.assertIn("Message the Slack app", observed_expectations)
self.assertNotIn("Connect Slack", observed_expectations)
self.assertFalse(any("/v2/chat" in url for url, _wait in fake_page.gotos))
self.assertEqual(
result.details["slack_connect_surface"],
"/v2/extensions/channels",
)
self.assertEqual(
result.details["slack_connect_title"],
"Slack account connection",
)

Comment on lines +89 to +227

@coderabbitai coderabbitai Bot Jul 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

LGTM! Solid regression coverage for the extensions-channels rewrite; the assertNotIn("Connect Slack", ...) check directly guards against reverting to the removed chat flow.

Optional: a companion case where the fake API omits the inbound_proof_code strategy (verifying _slack_connect_case returns success=False with the "missing" AssertionError) would round out coverage for the new failure branch, but not blocking.

🧰 Tools
🪛 ast-grep (0.44.0)

[info] 145-159: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"checks": {
"slack": {
"enabled_in_config": True,
"env_present": True,
"auth_test": {
"ok": True,
"team_id": "T123",
"user_id": "U123",
},
}
}
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

[warning] 169-169: Configuring an LLM/agent client endpoint over http:// sends prompts and responses (and often API keys) in cleartext, exposing them to interception. Use https for the base_url.
Context: base_url="http://127.0.0.1:3000"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.

(llm-client-insecure-http-python)

🤖 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/test_run_live_qa.py` around lines 89 - 227,
Add a negative-path test for `_slack_connect_case` to cover the new failure
branch when the connectable channels API does not return an `inbound_proof_code`
Slack entry. Reuse the existing test scaffolding in
`test_slack_connect_case_uses_extensions_channels_surface` with `FakePage`,
`fake_fetch_webui_json`, and `LiveQaContext`, but have the fake API response
omit that strategy and assert the case returns `success=False` with the expected
missing-entry AssertionError in `result.details`.

def test_product_connect_cases_start_from_chat_then_verify_registry(self):
captured_chat: dict[str, dict[str, object]] = {}
captured_registry: dict[str, dict[str, object]] = {}
Expand Down
Loading