feat(talkbot): move TalkBot bookkeeping to dedicated ex_apps_talk_bots table

[oleksandr-nc]

Summary

• New dedicated table oc_ex_apps_talk_bots (id, appid, route, secret, created_time) replaces the two-row-per-bot K/V pattern in appconfig_ex. Unique index on (appid, route) is the natural key for register/unregister/proxy lookups; appid index supports per-ExApp fan-out.
• TalkBotsService rewritten on top of TalkBotMapper / TalkBot entity. Bot secrets are encrypted at rest via ICrypto. Re-register is a no-op on AppAPI's side - Talk owns the human-readable metadata via BotInstallEvent and reconciles its own row on every register.
• One-shot data migration backfills existing bots from appconfig_ex. Handles both plaintext (the legacy default, since the old service never passed $sensitive=true) and sensitive=1 encrypted secret rows. Re-encrypts on insert into the new table. Each bot runs in its own transaction with per-bot rollback, and the migration is safe to re-run (skips bots already present in the new table).
• OCS contract for ExApps is unchanged. POST/DELETE /ex-app/talk_bot keep the same params and the same {id, secret} response shape. bot_id = sha1(\$appid . '_' . \$route) is preserved, so existing community bots (including nc_py_api callers) continue to work post-upgrade without re-registration. The proxy URL and HMAC verification path are byte-identical.
• Fan-out cleanup on ExApp uninstall. ExAppService::unregisterExApp calls TalkBotsService::unregisterExAppTalkBots, which dispatches BotUninstallEvent per bot so Talk's talk_bots_server stays aligned with our table.
• Compensating uninstall on insert failure. If mapper->insert fails after Talk has already persisted its row from the install event, the service fires BotUninstallEvent to avoid leaving Talk with an orphan pointing at a secret AppAPI never stored.
• PHPUnit coverage - controller register/unregister/re-register/corrupted-secret/fan-out, plus migration coverage (plaintext secret, sensitive-encrypted secret, orphan route row, malformed key suffix, multi-bot multi-app, idempotent re-run, re-run after pre-existing).

Test plan

• Full PHPUnit suite passes (68 tests, 216 assertions)
• psalm clean (stub added for spreed Bot{Install,Uninstall}Event)
• End-to-end: register bot via nc_py_api -> simulate pre-upgrade legacy appconfig_ex state with the same secret bytes -> run migration -> real HMAC-SHA256-signed POST through proxyTalkMessage is accepted (HMAC verification succeeds against the migrated secret)
• Fan-out via occ app_api:app:unregister: both oc_ex_apps_talk_bots and oc_talk_bots_server rows for the ExApp are cleaned
• Reviewer: smoke-test with a real Talk bot ExApp (e.g. summary_bot) on a vanilla instance - install on main, upgrade to this branch, confirm the bot still responds to messages without re-registering

Summary by CodeRabbit

• Infrastructure

  • Talk bot configurations now use dedicated database storage for improved reliability and a new migration consolidates legacy configs into the new table.

• Bug Fixes

  • Improved secret handling: encrypted storage, decryption safeguards, corruption detection, and safer rollback on failures.

• Behavior

  • Bot registration/unregistration now handles duplicates, per-bot errors, and compensating cleanup reliably.

• Tests

  • Expanded test coverage for registration, cleanup, migration idempotency, and secret handling.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR moves Talk bot registration data from legacy appconfig_ex to a new ex_apps_talk_bots table. It adds a TalkBot entity and TalkBotMapper, refactors TalkBotsService to use encrypted secrets via ICrypto and mapper CRUD, implements a migration that validates, decrypts when needed, re-encrypts, and removes legacy rows with per-bot transactions, and adds tests and Psalm stubs for Talk bot events.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 9.30% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat(talkbot): move TalkBot bookkeeping to dedicated ex_apps_talk_bots table' clearly and specifically describes the main architectural change: migrating TalkBot storage from appconfig_ex to a new dedicated database table.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 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 `@lib/Service/TalkBotsService.php`:
- Around line 122-124: The loop that calls unregisterExAppBot($exApp,
$bot->getRoute()) should isolate failures so one failing bot doesn't stop
cleanup; wrap the call inside the foreach in its own try/catch (catch
Throwable/Exception), log the error including the $bot->getRoute() and exception
message/trace, and continue to the next bot so remaining bots still get
unregistered.

In `@tests/php/Migration/Version034000Date20260428144801Test.php`:
- Around line 47-51: The test constructs Version034000Date20260428144801 with
three arguments but the migration's constructor only accepts two; update the
test setup in Version034000Date20260428144801Test so the new
Version034000Date20260428144801(...) call matches the migration constructor
signature (pass only the expected parameters, e.g., $this->db and $this->crypto)
or, if the migration was intentionally changed, adjust the migration constructor
to accept the third dependency (IURLGenerator) and update usages accordingly;
locate the creation in the test and ensure the parameter list aligns with the
class constructor for Version034000Date20260428144801.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 11e2eed2-dfdc-4424-ab83-f033383f15fa

📥 Commits

Reviewing files that changed from the base of the PR and between 2f76f5e and 5fface8.

📒 Files selected for processing (8)

• lib/Db/TalkBot.php
• lib/Db/TalkBotMapper.php
• lib/Migration/Version034000Date20260428144801.php
• lib/Service/TalkBotsService.php
• psalm.xml
• tests/php/Controller/TalkBotControllerTest.php
• tests/php/Migration/Version034000Date20260428144801Test.php
• tests/stubs/oca_talk_events.php

[edward-ly]

Nice! Works with talk_bot_ai