feat: add Matrix appservice bot, platform-agnostic core, and Kubernetes deployment #1

Open

Opencode wants to merge 9 commits from feat/matrix-appservice into main

pull from: feat/matrix-appservice

merge into: armistace:main

armistace:main

Conversation 5 Commits 9 Files Changed 18 +1049 -366

Opencode commented 2026-08-18 21:50:06 +10:00

Collaborator

Copy Link

Summary

Adds native Matrix appservice integration to Steward, refactors the conversation pipeline to be platform-agnostic, and adds Kubernetes deployment scaffolding mirroring the pr_reviewer pattern.

Changes

• Matrix appservice bot (steward/bot/matrix.py): mautrix-python appservice that receives Synapse transactions and replies via the client-server API.
• Platform-agnostic core (steward/bot/core.py): ConversationService + ThreadKey owning the LLM call, history, knowledge-base search, and thread-memory keying, shared by Telegram and Matrix.
• Telegram refactor: handlers become thin wrappers over ConversationService.
• ThreadMemoryStore: generalized to platform-scoped keys with legacy chat_id:thread_id migration.
• Config: new matrix section (homeserver, tokens, room/user allowlists).
• main.py: async, starts Telegram and/or Matrix on one event loop.
• Deployment: kube/ manifests + Gitea Actions workflow (.gitea/workflows/build_push.yml) mirroring pr_reviewer.
• Docs: README + CONFIGURATION.md updated; Synapse appservice registration template at matrix/steward_appservice.yaml.

Verification

• 86/86 tests pass
• ruff clean
• Docker image builds and imports all modules
• kube manifests pass kubectl apply --dry-run=client

Notes

• Uses mautrix>=0.21.0 (matrix-nio is unmaintained and has no appservice framework).
• Synapse appservice registration is prepared but not yet applied to production (deferred).

## Summary Adds native Matrix appservice integration to Steward, refactors the conversation pipeline to be platform-agnostic, and adds Kubernetes deployment scaffolding mirroring the pr_reviewer pattern. ## Changes - **Matrix appservice bot** (`steward/bot/matrix.py`): mautrix-python appservice that receives Synapse transactions and replies via the client-server API. - **Platform-agnostic core** (`steward/bot/core.py`): `ConversationService` + `ThreadKey` owning the LLM call, history, knowledge-base search, and thread-memory keying, shared by Telegram and Matrix. - **Telegram refactor**: handlers become thin wrappers over `ConversationService`. - **ThreadMemoryStore**: generalized to platform-scoped keys with legacy `chat_id:thread_id` migration. - **Config**: new `matrix` section (homeserver, tokens, room/user allowlists). - **main.py**: async, starts Telegram and/or Matrix on one event loop. - **Deployment**: `kube/` manifests + Gitea Actions workflow (`.gitea/workflows/build_push.yml`) mirroring pr_reviewer. - **Docs**: README + CONFIGURATION.md updated; Synapse appservice registration template at `matrix/steward_appservice.yaml`. ## Verification - 86/86 tests pass - ruff clean - Docker image builds and imports all modules - kube manifests pass `kubectl apply --dry-run=client` ## Notes - Uses `mautrix>=0.21.0` (matrix-nio is unmaintained and has no appservice framework). - Synapse appservice registration is prepared but not yet applied to production (deferred).

Opencode added 4 commits 2026-08-18 21:50:07 +10:00

chore: add Kubernetes deployment manifests and Gitea Actions build workflow ... 260720dd10

Mirror the pr_reviewer deployment pattern: Gitea Actions builds a multi-arch
image in the gitea-runner namespace, pushes to git.aridgwayweb.com, recreates
the steward namespace with regcred + env secret, and applies kube manifests.
Add .omo/ to .gitignore.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

feat: add Matrix appservice bot and platform-agnostic conversation core ... 76777c98eb

Introduce a shared ConversationService (steward/bot/core.py) that owns the
LLM call, history, knowledge-base search, and thread-memory keying behind a
normalized ThreadKey, so both Telegram and Matrix drive the same pipeline.

- Add steward/bot/matrix.py: a mautrix-python appservice bot that receives
  Synapse transactions and replies via the client-server API.
- Refactor telegram.py handlers into thin wrappers over ConversationService.
- Generalize ThreadMemoryStore/ThreadSummary to platform-scoped keys with
  legacy chat_id:thread_id migration.
- Add a matrix config section (homeserver, tokens, room/user allowlists).
- Rewrite main.py as async, starting Telegram and/or Matrix on one event loop.
- Add mautrix>=0.21.0 dependency.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

docs: document Matrix appservice integration and gitea deployment ... c8f3600b32

- Add Matrix bot to the feature list and prerequisites.
- Add a Matrix Appservice section covering config, Synapse registration,
  and the shared conversation pipeline.
- Document the matrix config section in CONFIGURATION.md.
- Add a ready-to-use Synapse appservice registration template
  (matrix/steward_appservice.yaml).
- Document the Gitea Actions build/deploy workflow and kube manifests.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

ci: wire Matrix appservice env vars into the steward-env secret ... 4838b9c388

Add STEWARD__MATRIX__* literals to the deployment secret so the Matrix
appservice bot is configured when the cluster secret is created. Values
come from gitea repo vars/secrets (MATRIX_HOMESERVER_URL, MATRIX_AS_TOKEN,
etc.) and are only populated when those are set.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

armistace commented 2026-08-18 21:50:09 +10:00

Owner

Copy Link

PR received — starting review, sit tight 🫡

PR received — starting review, sit tight :saluting_face:

armistace commented 2026-08-18 21:51:12 +10:00

Owner

Copy Link

PR Review Results

Consolidated Review Report: Matrix Integration & Core Refactor

1. Executive Summary

This Pull Request represents a major architectural pivot, transitioning the bot from a Telegram-specific implementation to a hexagonal, platform-agnostic architecture. The introduction of a centralized ConversationService is a significant engineering win, decoupling the core LLM logic from the transport layers (Telegram/Matrix).

However, while the software design is excellent, the operational and security scaffolding is currently insufficient for a production deployment. The review identifies critical gaps in Kubernetes security context, secret management, and resource boundary definitions. Furthermore, because the review was conducted on architectural patterns and metadata, specific verification of the ThreadKey implementation is required to prevent cross-platform data leakage.

---

2. Prioritized Issue List

🔴 Critical

• Secret Management: Potential for hardcoded secrets or plain-text environment variables in kube/steward_deployment.yaml and CI pipelines.
• Container Privilege Escalation: Lack of securityContext in K8s manifests; the bot may be running as root, increasing the blast radius of a potential RCE.
• Memory Key Collision (IDOR): Risk of users on different platforms accessing the same conversation history if ThreadKey generation lacks strict delimitation.

🟠 High

• Event Loop Blockage: Risk of a single synchronous I/O call in the shared ConversationService freezing both Telegram and Matrix bots simultaneously.
• Resource Constraints: Absence of CPU/Memory requests and limits in Kubernetes, risking Node-level OOM events.
• Input Validation: Potential for spoofing within the Matrix Appservice if transaction origin is not strictly verified.

🟡 Medium

• Observability Gaps: Lack of platform-specific tagging in logs, making cross-platform debugging difficult.
• Health Monitoring: Absence of livenessProbe and readinessProbe; K8s cannot detect if the internal async event loop has hung.
• Configuration Bloat: The flat configuration structure will become unmaintainable as more platforms are added.
• Image Tagging: Use of latest tags in Gitea Actions instead of Git SHAs or SemVer.

🔵 Low

• Network Policy: Lack of egress restrictions for the pod.
• Graceful Shutdown: Need for explicit SIGTERM handling to close connections cleanly.

---

3. Domain-Specific Recommendations

💻 Code & Architecture

• Async Integrity: Audit core.py and ConversationService to ensure zero blocking calls. Use asyncio.to_thread for any CPU-bound parsing.
• ThreadKey Validation: Implement and test a strict delimiter for keys (e.g., platform::user_id::thread_id) to ensure absolute collision resistance.
• Error Isolation: Wrap each bot's main handler in a top-level exception boundary so a failure in the Matrix transport does not crash the Telegram bot.
• Config Refactor: Transition from a flat config.py to a provider-based object structure (e.g., Config.matrix, Config.telegram).

🛡️ Security

• Hardening: Update steward_deployment.yaml with:

  securityContext:
    runAsNonRoot: true
    runAsUser: 1000
    allowPrivilegeEscalation: false
  

• Secret Injection: Mandatory move to secretKeyRef pointing to K8s Secrets or an external Vault.
• Rate Limiting: Implement rate limiting at the ConversationService level to prevent "Denial of Wallet" attacks on the LLM API.
• Audit Trail: Ensure every LLM request log includes platform and user_id tags for forensic analysis.

🏗️ Infrastructure

• Resource Boundaries: Define explicit resources.limits and resources.requests in the deployment manifest to ensure cluster stability.
• Health Checks: Implement a lightweight /healthz endpoint (via aiohttp or FastAPI) and link it to K8s liveness/readiness probes.
• CI/CD Pipeline: Update the Gitea workflow to tag Docker images with the Git commit hash for idempotent deployments and easy rollbacks.
• Network Security: Implement a NetworkPolicy restricting egress traffic only to known API endpoints (Matrix, Telegram, LLM provider).

---

4. Positive Aspects of the PR

• Forward-Thinking Design: The move to a platform-agnostic core is a high-quality architectural decision that drastically reduces future development friction.
• Professional Tooling: Integration of Ruff for linting and a comprehensive test suite (86/86 passing) shows a strong commitment to code quality.
• Deployment Automation: The addition of K8s manifests and CI/CD pipelines moves the project toward a modern DevOps maturity model.
• Clear Migration Path: The recognition of the need for a chat_id \rightarrow thread_id migration demonstrates attention to data persistence.

---

5. Overall Assessment & Recommendation

Final Rating: ⚠️ SIGNIFICANT CHANGES NEEDED

Justification:
While the code refactor is "Approved" from a design perspective, the infrastructure and security implementation is "Needs Changes." The current state of the K8s manifests and the lack of visible secret management constitute a high risk for production deployment. The transition to a centralized service also creates a high-value target that requires stricter validation and isolation than what is currently documented.

Required Action Items for Merge:

1. Security: Implement securityContext (non-root) and secretKeyRef in K8s manifests.
2. Stability: Add resources limits and liveness/readiness probes.
3. Verification: Provide the source code for ThreadKey and matrix.py to verify collision resistance and input validation.
4. DevOps: Update CI pipeline to use unique image tags (Git SHA).

## PR Review Results # Consolidated Review Report: Matrix Integration & Core Refactor ## 1. Executive Summary This Pull Request represents a major architectural pivot, transitioning the bot from a Telegram-specific implementation to a **hexagonal, platform-agnostic architecture**. The introduction of a centralized `ConversationService` is a significant engineering win, decoupling the core LLM logic from the transport layers (Telegram/Matrix). However, while the **software design is excellent**, the **operational and security scaffolding is currently insufficient** for a production deployment. The review identifies critical gaps in Kubernetes security context, secret management, and resource boundary definitions. Furthermore, because the review was conducted on architectural patterns and metadata, specific verification of the `ThreadKey` implementation is required to prevent cross-platform data leakage. --- ## 2. Prioritized Issue List ### 🔴 Critical * **Secret Management:** Potential for hardcoded secrets or plain-text environment variables in `kube/steward_deployment.yaml` and CI pipelines. * **Container Privilege Escalation:** Lack of `securityContext` in K8s manifests; the bot may be running as `root`, increasing the blast radius of a potential RCE. * **Memory Key Collision (IDOR):** Risk of users on different platforms accessing the same conversation history if `ThreadKey` generation lacks strict delimitation. ### 🟠 High * **Event Loop Blockage:** Risk of a single synchronous I/O call in the shared `ConversationService` freezing both Telegram and Matrix bots simultaneously. * **Resource Constraints:** Absence of CPU/Memory `requests` and `limits` in Kubernetes, risking Node-level OOM events. * **Input Validation:** Potential for spoofing within the Matrix Appservice if transaction origin is not strictly verified. ### 🟡 Medium * **Observability Gaps:** Lack of platform-specific tagging in logs, making cross-platform debugging difficult. * **Health Monitoring:** Absence of `livenessProbe` and `readinessProbe`; K8s cannot detect if the internal async event loop has hung. * **Configuration Bloat:** The flat configuration structure will become unmaintainable as more platforms are added. * **Image Tagging:** Use of `latest` tags in Gitea Actions instead of Git SHAs or SemVer. ### 🔵 Low * **Network Policy:** Lack of egress restrictions for the pod. * **Graceful Shutdown:** Need for explicit `SIGTERM` handling to close connections cleanly. --- ## 3. Domain-Specific Recommendations ### 💻 Code & Architecture * **Async Integrity:** Audit `core.py` and `ConversationService` to ensure zero blocking calls. Use `asyncio.to_thread` for any CPU-bound parsing. * **ThreadKey Validation:** Implement and test a strict delimiter for keys (e.g., `platform::user_id::thread_id`) to ensure absolute collision resistance. * **Error Isolation:** Wrap each bot's main handler in a top-level exception boundary so a failure in the Matrix transport does not crash the Telegram bot. * **Config Refactor:** Transition from a flat `config.py` to a provider-based object structure (e.g., `Config.matrix`, `Config.telegram`). ### 🛡️ Security * **Hardening:** Update `steward_deployment.yaml` with: ```yaml securityContext: runAsNonRoot: true runAsUser: 1000 allowPrivilegeEscalation: false ``` * **Secret Injection:** Mandatory move to `secretKeyRef` pointing to K8s Secrets or an external Vault. * **Rate Limiting:** Implement rate limiting at the `ConversationService` level to prevent "Denial of Wallet" attacks on the LLM API. * **Audit Trail:** Ensure every LLM request log includes `platform` and `user_id` tags for forensic analysis. ### 🏗️ Infrastructure * **Resource Boundaries:** Define explicit `resources.limits` and `resources.requests` in the deployment manifest to ensure cluster stability. * **Health Checks:** Implement a lightweight `/healthz` endpoint (via `aiohttp` or `FastAPI`) and link it to K8s liveness/readiness probes. * **CI/CD Pipeline:** Update the Gitea workflow to tag Docker images with the Git commit hash for idempotent deployments and easy rollbacks. * **Network Security:** Implement a `NetworkPolicy` restricting egress traffic only to known API endpoints (Matrix, Telegram, LLM provider). --- ## 4. Positive Aspects of the PR * **Forward-Thinking Design:** The move to a platform-agnostic core is a high-quality architectural decision that drastically reduces future development friction. * **Professional Tooling:** Integration of Ruff for linting and a comprehensive test suite (86/86 passing) shows a strong commitment to code quality. * **Deployment Automation:** The addition of K8s manifests and CI/CD pipelines moves the project toward a modern DevOps maturity model. * **Clear Migration Path:** The recognition of the need for a `chat_id` $\rightarrow$ `thread_id` migration demonstrates attention to data persistence. --- ## 5. Overall Assessment & Recommendation **Final Rating: ⚠️ SIGNIFICANT CHANGES NEEDED** **Justification:** While the **code refactor is "Approved"** from a design perspective, the **infrastructure and security implementation is "Needs Changes."** The current state of the K8s manifests and the lack of visible secret management constitute a high risk for production deployment. The transition to a centralized service also creates a high-value target that requires stricter validation and isolation than what is currently documented. **Required Action Items for Merge:** 1. [ ] **Security:** Implement `securityContext` (non-root) and `secretKeyRef` in K8s manifests. 2. [ ] **Stability:** Add `resources` limits and `liveness/readiness` probes. 3. [ ] **Verification:** Provide the source code for `ThreadKey` and `matrix.py` to verify collision resistance and input validation. 4. [ ] **DevOps:** Update CI pipeline to use unique image tags (Git SHA).

Opencode added 1 commit 2026-08-18 21:51:17 +10:00

docs: document Gitea Actions pipeline variables in README ... 0a7273b321

Add the full table of secrets and variables the build/deploy workflow reads
from the gitea repo, including the Matrix and Ollama Cloud options.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Opencode added 1 commit 2026-08-18 21:53:19 +10:00

fix: harden k8s security context and use SHA-tagged images ... 9c36b373d2

Address pr_reviewer findings:
- Enforce non-root (UID/GID 1000), no privilege escalation, drop all caps
  in the deployment securityContext.
- Tag images with the git SHA in addition to latest, and pin the deployed
  image to the SHA for idempotent rollbacks.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Opencode commented 2026-08-18 21:53:47 +10:00

Author

Collaborator

Copy Link

Response to review + fixes applied

Thanks for the thorough review. I have addressed the actionable items and pushed the fixes to the branch. Here is the disposition of each finding:

✅ Fixed

• Container privilege escalation — added securityContext to kube/steward_deployment.yaml: runAsNonRoot: true, runAsUser/Group: 1000, allowPrivilegeEscalation: false, drop all capabilities.
• Image tagging — the Gitea workflow now tags images with the git SHA (${{ gitea.sha }}) in addition to latest, and pins the deployed image to the SHA via kubectl set image for idempotent rollbacks.

🔍 Already handled (no change needed)

• Secret management — no hardcoded secrets. The deployment uses envFrom: secretRef: steward-env and the workflow uses ${{ secrets.* }} / ${{ vars.* }} placeholders only.
• Matrix transaction spoofing — mautrix AppService verifies the hs_token on every incoming transaction automatically; unauthorized requests return M_FORBIDDEN.
• Graceful shutdown — main.py installs SIGINT/SIGTERM handlers and shuts down both the Telegram updater and Matrix appservice cleanly.
• Config bloat — config is already provider-based (Settings.matrix, Settings.telegram, etc.), not flat.
• Resource constraints — resources.requests/limits were already present in the manifest.
• ThreadKey collision (IDOR) — the platform field already delimits keys (telegram:100:55 vs matrix:room:...), so cross-platform collision is impossible. The __str__ uses : separators and platform is always the first component.

📝 Noted for follow-up (out of scope for this PR)

• Event-loop blockage — ThreadMemoryStore does synchronous file I/O in async handlers. This is a pre-existing pattern; a follow-up could move it to asyncio.to_thread.
• Health probes — a /healthz endpoint + liveness/readiness probes would be valuable; deferred to a follow-up.
• NetworkPolicy — egress restriction is a good hardening; deferred.
• Rate limiting — LLM rate limiting at the service level is a good idea; deferred.

Please re-review the updated branch. Thanks!

## Response to review + fixes applied Thanks for the thorough review. I have addressed the actionable items and pushed the fixes to the branch. Here is the disposition of each finding: ### ✅ Fixed - **Container privilege escalation** — added `securityContext` to `kube/steward_deployment.yaml`: `runAsNonRoot: true`, `runAsUser/Group: 1000`, `allowPrivilegeEscalation: false`, drop all capabilities. - **Image tagging** — the Gitea workflow now tags images with the git SHA (`${{ gitea.sha }}`) in addition to `latest`, and pins the deployed image to the SHA via `kubectl set image` for idempotent rollbacks. ### 🔍 Already handled (no change needed) - **Secret management** — no hardcoded secrets. The deployment uses `envFrom: secretRef: steward-env` and the workflow uses `${{ secrets.* }}` / `${{ vars.* }}` placeholders only. - **Matrix transaction spoofing** — mautrix `AppService` verifies the `hs_token` on every incoming transaction automatically; unauthorized requests return `M_FORBIDDEN`. - **Graceful shutdown** — `main.py` installs SIGINT/SIGTERM handlers and shuts down both the Telegram updater and Matrix appservice cleanly. - **Config bloat** — config is already provider-based (`Settings.matrix`, `Settings.telegram`, etc.), not flat. - **Resource constraints** — `resources.requests`/`limits` were already present in the manifest. - **ThreadKey collision (IDOR)** — the `platform` field already delimits keys (`telegram:100:55` vs `matrix:room:...`), so cross-platform collision is impossible. The `__str__` uses `:` separators and platform is always the first component. ### 📝 Noted for follow-up (out of scope for this PR) - **Event-loop blockage** — `ThreadMemoryStore` does synchronous file I/O in async handlers. This is a pre-existing pattern; a follow-up could move it to `asyncio.to_thread`. - **Health probes** — a `/healthz` endpoint + liveness/readiness probes would be valuable; deferred to a follow-up. - **NetworkPolicy** — egress restriction is a good hardening; deferred. - **Rate limiting** — LLM rate limiting at the service level is a good idea; deferred. Please re-review the updated branch. Thanks!

Opencode added 1 commit 2026-08-18 22:02:08 +10:00

fix: address pr_reviewer findings (data loss, memory, prompt injection) ... f79214bb22

- CI/CD: stop deleting the steward namespace on every deploy; use
  kubectl apply (dry-run -> apply) so the PVC and conversation memory
  survive deployments.
- core: bound _histories with an LRU eviction (max 1000 active threads)
  to prevent unbounded memory growth.
- core: wrap knowledge-base context in KB START/END delimiters and
  instruct the LLM to treat it as data, mitigating indirect prompt
  injection.
- matrix: wrap message processing in try/except so failures are logged
  instead of silently dropped.
- telegram: remove now-dead flush/tags prompt constants (centralized in
  core).

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Opencode commented 2026-08-18 22:02:20 +10:00

Author

Collaborator

Copy Link

pr_reviewer: Consolidated Review (manual trigger)

Triggered manually via the pr_reviewer API with the changed file contents, since the webhook does not re-fire on branch updates.

Unified Code Review Synthesis Report: Steward Bot Matrix Integration

1. Executive Summary

This PR represents a major architectural evolution for the Steward bot, transitioning it to a platform-agnostic core to support both Telegram and Matrix. While the structural refactor is well-conceived and significantly improves the maintainability of the business logic, the current implementation contains critical failures in the deployment pipeline and severe memory management issues that make it unsuitable for production in its current state.

The most alarming finding is a "nuke and pave" CI/CD strategy that deletes the entire Kubernetes namespace on every push, effectively nullifying the purpose of the Persistent Volume Claim (PVC) and causing total data loss upon every deployment. Additionally, a high-severity memory leak in the core conversation logic poses a significant stability risk.

---

2. Prioritized Issue List

🔴 Critical (Must be fixed before merge)

• CI/CD Destructive Deployment: The Gitea workflow deletes the entire steward namespace on every push (kubectl delete namespace steward). This wipes all secrets and the PVC containing the knowledge base.
• Unbounded Memory Growth: The _histories dictionary in ConversationService grows indefinitely for every unique ThreadKey, leading to inevitable OOM (Out-of-Memory) crashes.

🟠 High (Should be fixed before merge)

• Blocking Async I/O: Synchronous file operations (Path.write_text) in ThreadMemoryStore will block the event loop, causing latency spikes for all users during KB flushes.
• Indirect Prompt Injection: Knowledge base summaries are injected directly into system prompts without delimiters or sanitization, allowing stored malicious strings to hijack the LLM.
• Insecure Secret Handling: Secrets are created via --from-literal in CI, risking leakage in logs and process memory.

🟡 Medium (Address shortly after merge)

• Inconsistent History Capping: Memory policies (caps) are fragmented between the core and platform adapters.
• Plaintext Persistence: Conversation summaries (PII/Corporate data) are stored in unencrypted JSON on the PVC.
• Over-exposed Network Surface: Use of NodePort and 0.0.0.0 binding exposes the Matrix endpoint to the broader network without strict NetworkPolicies.

🔵 Low (General improvements)

• Configuration "God Object": The Settings class is bloated with legacy properties.
• Image Tagging: Use of :latest in manifests complicates rollbacks and stability.
• Hardcoded Values: Registry URLs and namespaces are hardcoded in YAML/Workflows.
• Input Validation: Lack of validation for LLM-generated tags and Matrix MXID regex.

---

3. Domain-Specific Recommendations

💻 Code Quality

• Memory Management: Replace the _histories dict with a TTL (Time-To-Live) cache or an LRU cache. For production scale, migrate active session history to Redis.
• Asynchronous I/O: Replace json and Path calls with aiofiles or wrap blocking calls in asyncio.to_thread().
• Logic Centralization: Move _FLUSH_SYSTEM_PROMPT and _TAGS_SYSTEM_PROMPT exclusively to core.py to ensure consistency across Telegram and Matrix.
• Robustness: Implement a try...except block in StewardMatrixBot._on_event to prevent silent failures and provide user feedback on errors.

🛡️ Security

• Prompt Hardening: Wrap KB context in clear delimiters (e.g., ### KB START ###) and explicitly instruct the LLM to treat the content as data, not instructions.
• Data Protection: Implement encryption-at-rest for the thread_memory.json file or migrate to an encrypted database.
• Network Hardening: Implement a Kubernetes NetworkPolicy to restrict traffic to the bot's port (8000) so only the Matrix Homeserver can communicate with it.
• Image Pinning: Use specific image digests (SHAs) instead of the :latest tag.

⚙️ Infrastructure

• Declarative CI/CD: Replace kubectl delete namespace with kubectl apply -f. Move toward a declarative model using Helm or Kustomize.
• Secret Management: Transition away from --from-literal. Use SealedSecrets or an external secret provider (Vault/AWS SM).
• Service Architecture: Replace NodePort with an Ingress controller to enable proper TLS termination and DNS management.
• Reliability: Add livenessProbe and readinessProbe to the Deployment to ensure the Matrix HTTP server is healthy before routing traffic.

---

4. Positive Aspects of the PR

• Architectural Vision: The move to a platform-agnostic ConversationService is a major win for the project's scalability and maintainability.
• Identity Normalization: The introduction of ThreadKey elegantly solves the problem of identifying conversations across disparate platforms.
• Modern Tooling: Transitioning to Pydantic and OmegaConf for configuration is a significant improvement in type safety and environment management.
• Deployment Readiness: The inclusion of multi-arch Docker builds and a SecurityContext (non-root) shows good baseline awareness of container best practices.

---

5. Overall Assessment & Recommendation

Overall Rating: 🔴 SIGNIFICANT CHANGES NEEDED

Justification:
While the code's structural design is excellent, the operational implementation is dangerous. The combination of a destructive deployment pipeline (which deletes user data on every update) and a critical memory leak creates a high risk of data loss and production instability. These two issues, combined with the potential for indirect prompt injection, must be resolved before this PR can be approved.

Action Plan for Developer:

1. Fix the CI/CD pipeline to use kubectl apply instead of delete namespace.
2. Implement an LRU/TTL cache for _histories.
3. Convert blocking I/O to asynchronous.
4. Add delimiters to the KB prompt injection logic.
5. Address secret management and image tagging.

## pr_reviewer: Consolidated Review (manual trigger) > Triggered manually via the pr_reviewer API with the changed file contents, since the webhook does not re-fire on branch updates. # Unified Code Review Synthesis Report: Steward Bot Matrix Integration ## 1. Executive Summary This PR represents a major architectural evolution for the Steward bot, transitioning it to a platform-agnostic core to support both Telegram and Matrix. While the structural refactor is well-conceived and significantly improves the maintainability of the business logic, the current implementation contains **critical failures in the deployment pipeline and severe memory management issues** that make it unsuitable for production in its current state. The most alarming finding is a "nuke and pave" CI/CD strategy that deletes the entire Kubernetes namespace on every push, effectively nullifying the purpose of the Persistent Volume Claim (PVC) and causing total data loss upon every deployment. Additionally, a high-severity memory leak in the core conversation logic poses a significant stability risk. --- ## 2. Prioritized Issue List ### 🔴 Critical (Must be fixed before merge) * **CI/CD Destructive Deployment:** The Gitea workflow deletes the entire `steward` namespace on every push (`kubectl delete namespace steward`). This wipes all secrets and the PVC containing the knowledge base. * **Unbounded Memory Growth:** The `_histories` dictionary in `ConversationService` grows indefinitely for every unique `ThreadKey`, leading to inevitable OOM (Out-of-Memory) crashes. ### 🟠 High (Should be fixed before merge) * **Blocking Async I/O:** Synchronous file operations (`Path.write_text`) in `ThreadMemoryStore` will block the event loop, causing latency spikes for all users during KB flushes. * **Indirect Prompt Injection:** Knowledge base summaries are injected directly into system prompts without delimiters or sanitization, allowing stored malicious strings to hijack the LLM. * **Insecure Secret Handling:** Secrets are created via `--from-literal` in CI, risking leakage in logs and process memory. ### 🟡 Medium (Address shortly after merge) * **Inconsistent History Capping:** Memory policies (caps) are fragmented between the core and platform adapters. * **Plaintext Persistence:** Conversation summaries (PII/Corporate data) are stored in unencrypted JSON on the PVC. * **Over-exposed Network Surface:** Use of `NodePort` and `0.0.0.0` binding exposes the Matrix endpoint to the broader network without strict NetworkPolicies. ### 🔵 Low (General improvements) * **Configuration "God Object":** The `Settings` class is bloated with legacy properties. * **Image Tagging:** Use of `:latest` in manifests complicates rollbacks and stability. * **Hardcoded Values:** Registry URLs and namespaces are hardcoded in YAML/Workflows. * **Input Validation:** Lack of validation for LLM-generated tags and Matrix MXID regex. --- ## 3. Domain-Specific Recommendations ### 💻 Code Quality * **Memory Management:** Replace the `_histories` dict with a TTL (Time-To-Live) cache or an LRU cache. For production scale, migrate active session history to Redis. * **Asynchronous I/O:** Replace `json` and `Path` calls with `aiofiles` or wrap blocking calls in `asyncio.to_thread()`. * **Logic Centralization:** Move `_FLUSH_SYSTEM_PROMPT` and `_TAGS_SYSTEM_PROMPT` exclusively to `core.py` to ensure consistency across Telegram and Matrix. * **Robustness:** Implement a `try...except` block in `StewardMatrixBot._on_event` to prevent silent failures and provide user feedback on errors. ### 🛡️ Security * **Prompt Hardening:** Wrap KB context in clear delimiters (e.g., `### KB START ###`) and explicitly instruct the LLM to treat the content as data, not instructions. * **Data Protection:** Implement encryption-at-rest for the `thread_memory.json` file or migrate to an encrypted database. * **Network Hardening:** Implement a Kubernetes `NetworkPolicy` to restrict traffic to the bot's port (8000) so only the Matrix Homeserver can communicate with it. * **Image Pinning:** Use specific image digests (SHAs) instead of the `:latest` tag. ### ⚙️ Infrastructure * **Declarative CI/CD:** Replace `kubectl delete namespace` with `kubectl apply -f`. Move toward a declarative model using Helm or Kustomize. * **Secret Management:** Transition away from `--from-literal`. Use **SealedSecrets** or an external secret provider (Vault/AWS SM). * **Service Architecture:** Replace `NodePort` with an `Ingress` controller to enable proper TLS termination and DNS management. * **Reliability:** Add `livenessProbe` and `readinessProbe` to the Deployment to ensure the Matrix HTTP server is healthy before routing traffic. --- ## 4. Positive Aspects of the PR * **Architectural Vision:** The move to a platform-agnostic `ConversationService` is a major win for the project's scalability and maintainability. * **Identity Normalization:** The introduction of `ThreadKey` elegantly solves the problem of identifying conversations across disparate platforms. * **Modern Tooling:** Transitioning to Pydantic and OmegaConf for configuration is a significant improvement in type safety and environment management. * **Deployment Readiness:** The inclusion of multi-arch Docker builds and a `SecurityContext` (non-root) shows good baseline awareness of container best practices. --- ## 5. Overall Assessment & Recommendation **Overall Rating: 🔴 SIGNIFICANT CHANGES NEEDED** **Justification:** While the code's structural design is excellent, the operational implementation is dangerous. The combination of a **destructive deployment pipeline** (which deletes user data on every update) and a **critical memory leak** creates a high risk of data loss and production instability. These two issues, combined with the potential for indirect prompt injection, must be resolved before this PR can be approved. **Action Plan for Developer:** 1. Fix the CI/CD pipeline to use `kubectl apply` instead of `delete namespace`. 2. Implement an LRU/TTL cache for `_histories`. 3. Convert blocking I/O to asynchronous. 4. Add delimiters to the KB prompt injection logic. 5. Address secret management and image tagging.

Opencode commented 2026-08-18 22:02:31 +10:00

Author

Collaborator

Copy Link

Response to pr_reviewer findings

Thanks for the thorough review. I have addressed the critical and high-severity items and pushed the fixes to the branch (commit f79214b).

✅ Fixed

• CI/CD destructive deployment (Critical) — removed kubectl delete namespace steward. The workflow now uses kubectl apply (via --dry-run=client -o yaml | kubectl apply -f -) for the namespace and secrets, so the PVC and conversation memory survive every deployment.
• Unbounded memory growth (Critical) — ConversationService._histories is now an LRU cache bounded to 1000 active threads (_MAX_ACTIVE_THREADS), evicting least-recently-used scopes.
• Indirect prompt injection (High) — knowledge-base context is now wrapped in ### KB START ### / ### KB END ### delimiters with an explicit instruction to treat the content as data, not instructions.
• Silent Matrix failures (Medium) — StewardMatrixBot._on_event now wraps processing in try/except and logs failures instead of dropping them.
• Dead code (Low) — removed the duplicated _FLUSH_SYSTEM_PROMPT/_TAGS_SYSTEM_PROMPT from telegram.py; they now live only in core.py.

🔍 Already handled / by design

• Blocking I/O in async loop — ThreadMemoryStore does synchronous file I/O. This is a pre-existing pattern; a follow-up can move it to asyncio.to_thread. Noted.
• Plaintext JSON persistence — conversation summaries are stored in plaintext JSON on the PVC. Encryption-at-rest is a follow-up (Longhorn supports it).
• NodePort exposure — the Matrix appservice endpoint is exposed via NodePort 30002. A NetworkPolicy restricting it to the Synapse homeserver is a good follow-up.
• latest image tag — the workflow now tags with the git SHA and pins the deployed image via kubectl set image. The manifest keeps latest as a fallback for the initial apply.
• Inconsistent history capping — the cap is intentionally per-platform (Telegram threads unbounded, non-thread and Matrix capped at 40). This is a deliberate policy, not an oversight.

📝 Deferred (follow-up PRs)

• Health probes (livenessProbe/readinessProbe)
• NetworkPolicy for egress restriction
• SealedSecrets/ExternalSecrets for secret management
• Helm chart / Kustomize for environment-specific config
• asyncio.to_thread for blocking I/O

Please re-review. Thanks!

## Response to pr_reviewer findings Thanks for the thorough review. I have addressed the critical and high-severity items and pushed the fixes to the branch (commit `f79214b`). ### ✅ Fixed - **CI/CD destructive deployment (Critical)** — removed `kubectl delete namespace steward`. The workflow now uses `kubectl apply` (via `--dry-run=client -o yaml | kubectl apply -f -`) for the namespace and secrets, so the PVC and conversation memory survive every deployment. - **Unbounded memory growth (Critical)** — `ConversationService._histories` is now an LRU cache bounded to 1000 active threads (`_MAX_ACTIVE_THREADS`), evicting least-recently-used scopes. - **Indirect prompt injection (High)** — knowledge-base context is now wrapped in `### KB START ###` / `### KB END ###` delimiters with an explicit instruction to treat the content as data, not instructions. - **Silent Matrix failures (Medium)** — `StewardMatrixBot._on_event` now wraps processing in `try/except` and logs failures instead of dropping them. - **Dead code (Low)** — removed the duplicated `_FLUSH_SYSTEM_PROMPT`/`_TAGS_SYSTEM_PROMPT` from `telegram.py`; they now live only in `core.py`. ### 🔍 Already handled / by design - **Blocking I/O in async loop** — `ThreadMemoryStore` does synchronous file I/O. This is a pre-existing pattern; a follow-up can move it to `asyncio.to_thread`. Noted. - **Plaintext JSON persistence** — conversation summaries are stored in plaintext JSON on the PVC. Encryption-at-rest is a follow-up (Longhorn supports it). - **NodePort exposure** — the Matrix appservice endpoint is exposed via NodePort 30002. A NetworkPolicy restricting it to the Synapse homeserver is a good follow-up. - **`latest` image tag** — the workflow now tags with the git SHA and pins the deployed image via `kubectl set image`. The manifest keeps `latest` as a fallback for the initial apply. - **Inconsistent history capping** — the cap is intentionally per-platform (Telegram threads unbounded, non-thread and Matrix capped at 40). This is a deliberate policy, not an oversight. ### 📝 Deferred (follow-up PRs) - Health probes (`livenessProbe`/`readinessProbe`) - NetworkPolicy for egress restriction - SealedSecrets/ExternalSecrets for secret management - Helm chart / Kustomize for environment-specific config - `asyncio.to_thread` for blocking I/O Please re-review. Thanks!

Opencode added 1 commit 2026-08-18 22:03:45 +10:00

docs: document pr_reviewer manual-trigger workflow in AGENTS.md ... e2ba3dd099

The pr_reviewer webhook only fires on PR open and does not re-trigger on
branch updates or recall. Document the manual API trigger and the loop of
posting the review + response to the PR as comments, so future sessions
can iterate on reviews.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Opencode added 1 commit 2026-08-18 22:09:03 +10:00

Revert "docs: document pr_reviewer manual-trigger workflow in AGENTS.md" ... 0667adbf72

This reverts commit e2ba3dd0990d31167f48bfd0f33f273f0e426514.

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin feat/matrix-appservice:feat/matrix-appservice

git checkout feat/matrix-appservice

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

hermes Opencode armistace

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: armistace/steward_mirror#1

No description provided.