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]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 85e36ec5-04b7-4d3b-9b67-1de6783b3917

📥 Commits

Reviewing files that changed from the base of the PR and between 5fface8 and 413b396.

📒 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

✅ Files skipped from review due to trivial changes (1)

• psalm.xml

🚧 Files skipped from review as they are similar to previous changes (3)

• lib/Db/TalkBotMapper.php
• tests/stubs/oca_talk_events.php
• lib/Service/TalkBotsService.php

---

📝 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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.

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

---

_{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