chore: add Copilot code review instructions#262
Closed
Reese-max wants to merge 80 commits intoopenabdev:mainfrom
Closed
chore: add Copilot code review instructions#262Reese-max wants to merge 80 commits intoopenabdev:mainfrom
Reese-max wants to merge 80 commits intoopenabdev:mainfrom
Conversation
* perf: cache dependency build layer in Dockerfile * perf: native multi-arch build with matrix runners --------- Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
* fix: use app bot identity for chart bump commits * fix: pull --rebase before push in bump-chart --------- Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: openclaw-helm-bot[bot] <3185992+openclaw-helm-bot[bot]@users.noreply.github.com>
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: openclaw-helm-bot[bot] <3185992+openclaw-helm-bot[bot]@users.noreply.github.com>
* feat: support AGENTS.md injection via helm values Add agentsMd value to chart so AGENTS.md can be configured at install time via values.yaml instead of manually exec-ing into the pod. * chore: bump chart to 0.1.8
* feat: add Codex support via codex-acp adapter - Add Dockerfile.codex (node:22 + codex-acp + codex CLI for OAuth) - Simplify PVC mount: persist entire /home/agent instead of subPaths This ensures auth tokens for any CLI backend (kiro, codex, etc.) survive restarts - Bump chart to 0.1.9 Closes openabdev#14 * ci: build codex image alongside kiro in CI - Add variant matrix to build both Dockerfile and Dockerfile.codex - Produces ghcr.io/thepagent/agent-broker (kiro) and ghcr.io/thepagent/agent-broker-codex - Trigger on Dockerfile.* changes - Separate cache scopes per variant+platform * feat: add Claude Code support via claude-agent-acp adapter - Add Dockerfile.claude (node:22 + @agentclientprotocol/claude-agent-acp@0.25.0 + claude CLI) - Add claude variant to CI build matrix - Produces ghcr.io/thepagent/agent-broker-claude Note: @zed-industries/claude-agent-acp is deprecated, using @agentclientprotocol/claude-agent-acp Closes openabdev#16 * docs: dynamic NOTES.txt auth instructions per backend Show kiro-cli, codex, or claude auth steps based on agent.command value. * chore: remove unused matrix field, add token expiry note --------- Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
…ollision (openabdev#23) The default variant (suffix='') produced artifact names like 'digests-ubuntu-latest' which matched the download pattern 'digests-codex-*' prefix. Use explicit artifact keys (default, codex, claude) instead of suffix-based naming. Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: openclaw-helm-bot[bot] <3185992+openclaw-helm-bot[bot]@users.noreply.github.com>
Users can now install with just:
helm install agent-broker agent-broker/agent-broker \
--set discord.botToken=$DISCORD_BOT_TOKEN \
--set discord.allowedChannels[0]=CHANNEL_ID \
--set agent.preset=codex
Presets: kiro (default), codex, claude — auto-configures image + command.
Closes openabdev#25
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
…v#30) * docs: update README with preset install instructions and tested backends * docs: add per-backend CLI install prompts * docs: simplify prompts, defer auth to NOTES.txt --------- Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: openclaw-helm-bot[bot] <3185992+openclaw-helm-bot[bot]@users.noreply.github.com>
Co-authored-by: openclaw-helm-bot[bot] <3185992+openclaw-helm-bot[bot]@users.noreply.github.com>
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
…nabdev#33) * ci: default to beta chart versions, stable via workflow_dispatch - Push-triggered builds produce beta versions (e.g. 0.2.1-beta.12345) - workflow_dispatch adds 'release' toggle for stable versions - Beta charts are invisible to `helm install` (requires --devel or --version) - Bump PRs labeled as beta or stable * docs: add RELEASING.md for maintainers * docs: add ASCII flow diagrams to RELEASING.md --------- Co-authored-by: thepagent <thepagent@users.noreply.github.com>
* feat: add Gemini CLI support (native ACP) - Add Dockerfile.gemini (node:22 + @google/gemini-cli) - Add gemini preset to _helpers.tpl (command: gemini, args: [--acp]) - Add gemini to CI build matrix - Add gemini auth hint to NOTES.txt Closes openabdev#15 * docs: add Gemini to README presets, install examples, and auth instructions --------- Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: openclaw-helm-bot[bot] <3185992+openclaw-helm-bot[bot]@users.noreply.github.com>
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Inject Discord sender identity as a structured <sender_context> JSON block into the prompt, enabling downstream CLIs to identify message senders. Implements Phase 1+2 of openabdev#61. Fields: schema, sender_id, sender_name, display_name, channel, channel_id, is_bot. Thread names use the original prompt text (no sender prefix pollution). Closes openabdev#61
…penabdev#64) Add fail-fast validation in configmap template to detect scientific notation in channel IDs, plus --set-string warnings in NOTES.txt and values.yaml. Closes openabdev#43
Includes openabdev#62 (sender identity injection) and openabdev#64 (Helm float64 validation).
) 1. bump-chart: push directly to main with [skip ci] instead of creating a PR. This ensures appVersion in Chart.yaml always matches the actual image SHA from the build that triggered it. Previously the bump PR merge created a new commit SHA that had no corresponding image. 2. release.yml: push Helm charts to oci://ghcr.io/<owner>/charts instead of oci://ghcr.io/<owner> to avoid namespace collision with Docker images in the same GHCR repo. Fixes openabdev#66 Co-authored-by: thepagent <thepagent@users.noreply.github.com>
…v#68) Direct push to main is blocked by branch protection rules. Revert to PR approach but with auto-merge enabled, so the appVersion still matches the correct image SHA. Follows up on openabdev#66 Co-authored-by: thepagent <thepagent@users.noreply.github.com>
image: f0a2d0e Co-authored-by: openclaw-helm-bot[bot] <3185992+openclaw-helm-bot[bot]@users.noreply.github.com>
Discord Snowflake IDs exceed float64 precision. Using --set causes silent message ignoring due to scientific notation mangling. Ref: openabdev#43 Co-authored-by: thepagent <thepagent@users.noreply.github.com>
image: 3865d98 Co-authored-by: openclaw-helm-bot[bot] <3185992+openclaw-helm-bot[bot]@users.noreply.github.com>
Define @thepagent as code owner for all files on main branch. Co-authored-by: thepagent <thepagent@users.noreply.github.com>
image: 7947315 Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
* ci: switch Build & Release to git tag driven flow - trigger: push tags v* (instead of push to main with paths filter) - version: parsed from tag (v0.7.0-beta.1 → 0.7.0-beta.1) - docker tags: sha + semver + major.minor + latest (stable only) - bump-chart: version comes directly from tag, no more GITHUB_RUN_NUMBER - workflow_dispatch: kept for manual trigger with explicit tag input * ci: mark beta chart releases as pre-release * ci: split chart release — beta pushes OCI only, stable opens PR Beta (tag contains '-'): → release-chart-beta: helm package + push to OCI registry → no PR, main branch untouched Stable (tag without '-'): → bump-chart-stable: update Chart.yaml + values.yaml → PR → auto merge → release.yml picks up Chart.yaml change → publish to GitHub Pages + OCI * ci: add resolve-tag job, fix workflow_dispatch bug, deduplicate - Add resolve-tag job: validates tag format, parses chart_version, image_sha, is_beta — single source of truth for all downstream jobs - Fix: workflow_dispatch now uses inputs.tag instead of github.ref_name for beta/stable branching (github.ref_name is branch name, not tag) - Remove 3× duplicated 'Resolve version tag' steps - IMAGE_SHA computed once in resolve-tag, not repeated per job * ci: stable release promotes beta image instead of rebuilding - build-image + merge-manifests: only run for beta tags - promote-stable: retag existing beta image with stable tags (version, major.minor, latest) using imagetools create - verify beta image exists before promoting — fail fast if not - bump-chart-stable now depends on promote-stable This ensures "what you tested is what you ship" — the stable release uses the exact same image artifact validated during beta. * ci: add tagpr workflow and config for automated release PR * ci: pin tagpr action to commit hash and bump checkout to v6 * ci: add tagpr workflow and config for automated release PR - Add tagpr.yml with GitHub App token (so tags trigger build.yml) - Simplify build.yml: remove beta/stable two-stage, all tags do full build - Add pre-release support: manual tags like v0.7.0-rc.1 won't overwrite latest - Configure .tagpr to sync Cargo.toml + Chart.yaml version/appVersion - Simplify release.yml: keep chart-releaser + install instructions - Align Cargo.toml and Chart.yaml versions to 0.6.0 - Rewrite RELEASING.md with new tag-driven flow * ci: add promote-stable to guarantee pre-release = stable image - Pre-release tag (v0.7.0-rc.1): full build, image tags = sha + version - Stable tag (v0.7.0): promote (re-tag) pre-release image, no rebuild - Verify pre-release image exists before promote, fail if not found - Update RELEASING.md with two-path flow diagram * docs: rewrite RELEASING.md with complete release flow and constraints * ci: promote-stable finds pre-release image by version tag, not commit SHA - promote-stable uses git tag -l to find latest pre-release tag (e.g. v0.7.0-rc.*) - re-tags pre-release image to stable tags (no rebuild, same artifact) - removes commit SHA dependency — pre-release and stable can be on different commits - natural flow: pre-release first → test → merge Release PR → auto promote * chore: update openab version to 0.6.0 in Cargo.lock * ci: upgrade tagpr to v1.18.1 and use vPrefix config * ci: replace deprecated app-id with client-id in create-github-app-token * ci: add CI workflow for PR validation on source and Dockerfile changes * docs: add GitHub App permissions to RELEASING.md * ci: replace tagpr with native GitHub workflows - Remove tagpr.yml and .tagpr (third-party dependency) - Add release-pr.yml: workflow_dispatch with auto/manual version bump - Add tag-on-merge.yml: auto-tag on release PR merge - Update RELEASING.md: document new flow, simplify App permissions * ci: address review feedback - release-chart: add resolve-tag.result == 'success' to if-condition - release-pr: add rust-toolchain + cargo generate-lockfile for Cargo.lock sync - tag-on-merge: validate version format before pushing tag - build: add default placeholder for workflow_dispatch tag input * fix: replace map_or with is_some_and to satisfy clippy * chore: use beta instead of rc for pre-release naming --------- Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
…penabdev#206) - Cargo.toml stays at stable version on main (e.g. 0.6.4) - Chart.yaml gets the full version (beta or stable) - Version source changed from Cargo.toml to Chart.yaml - Removed rust-toolchain/cargo generate-lockfile (not needed) Co-authored-by: thepagent <thepagent@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
The helm install examples used a stale commit SHA (b500bf6) from PR openabdev#145. Now that tag-driven releases produce :latest on stable promote, use that instead. Co-authored-by: thepagent <thepagent@users.noreply.github.com>
* feat: resize and compress images before base64 encoding Follow OpenClaw's approach to prevent large image payloads from exceeding JSON-RPC transport limits (Internal Error -32603). Changes: - Add image crate dependency (jpeg, png, gif, webp) - Resize images so longest side <= 1200px (Lanczos3) - Re-encode as JPEG at quality 75 (~200-400KB after base64) - GIFs pass through unchanged to preserve animation - Fallback to original bytes if resize fails Fixes openabdev#209 * test: add unit tests for image resize and compression Tests cover: - Large image resized to max 1200px - Small image keeps original dimensions - Landscape/portrait aspect ratio preserved - Compressed output smaller than original - GIF passes through unchanged - Invalid data returns error * fix: preserve aspect ratio on resize + add fallback size check Address review feedback from @the3mi: - 🔴 Fix resize() to calculate proportional dimensions instead of forcing 1200x1200 (was distorting images) - 🟡 Add 1MB size check on fallback path when resize fails - Fix portrait/landscape test assertions to match correct aspect ratios * fix: restore post-download size check + use structured logging Address minor review feedback: - Restore defense-in-depth bytes.len() check after download - Use tracing structured fields (url = %url, error = %e) for consistency with codebase style --------- Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
…abdev#138) fix: dedupe tool call display by toolCallId and sanitize titles
…enabdev#81) (openabdev#135) fix: prevent Discord message fragmentation during streaming (fixes openabdev#81)
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
…ev#225) * feat: support voice message STT (Speech-to-Text) for Discord Add optional STT support that transcribes Discord voice message attachments (audio/ogg) via any OpenAI-compatible /audio/transcriptions endpoint and injects the transcript into the ACP prompt as text. - New src/stt.rs: ~50-line module calling POST /audio/transcriptions - New SttConfig in config.rs: enabled, api_key, model, base_url - discord.rs: detect audio/* attachments, download, transcribe, inject - Defaults to Groq free tier (whisper-large-v3-turbo) - Supports any OpenAI-compatible endpoint via base_url (Groq, OpenAI, local whisper server, etc.) - Feature is opt-in: disabled by default, zero impact when unconfigured Closes openabdev#224 * fix: add json feature to reqwest for resp.json() in stt module * docs: add STT configuration and deployment guide * fix: address PR review feedback - Reuse shared HTTP_CLIENT in stt.rs instead of creating per-call client - Pass actual MIME type from attachment (not hardcoded audio/ogg) - Fix attachment routing: check audio first, avoid wasted image download - Add api_key validation at startup (fail fast on empty key) - Add response_format=json to multipart form (fixes local servers) - Update docs: clarify api_key requirement, add Technical Notes section * feat: auto-detect GROQ_API_KEY from env when stt.enabled=true If stt.enabled = true and api_key is not set in config, openab automatically checks for GROQ_API_KEY in the environment. This allows minimal config: [stt] enabled = true No api_key line needed if the env var exists. * fix: only auto-detect GROQ_API_KEY when base_url points to Groq Prevents leaking Groq API key to unrelated endpoints when user sets a custom base_url without explicitly setting api_key. * docs: clarify GROQ_API_KEY auto-detect scope in stt.md * fix: move STT auto-detect before handler construction The handler clones stt_config at construction time. Auto-detect was running after the clone, so the handler never received the detected api_key. Now auto-detect runs first. --------- Co-authored-by: openab-bot <openab-bot@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
* helm: add first-class STT config to chart
Add stt as a first-class config block in the Helm chart so users
can enable STT with a single helm upgrade command:
helm upgrade openab openab/openab \
--set agents.kiro.stt.enabled=true \
--set agents.kiro.stt.apiKey=gsk_xxx
- values.yaml: add stt defaults (enabled, apiKey, model, baseUrl)
- configmap.yaml: render [stt] section when enabled, using ${STT_API_KEY}
- secret.yaml: store apiKey in K8s Secret (same pattern as botToken)
- deployment.yaml: inject STT_API_KEY env var from Secret
API key stays out of the configmap — follows the existing
DISCORD_BOT_TOKEN pattern.
Closes openabdev#227
* docs: add Helm chart deployment section to stt.md
* docs: mention STT support in README with link to docs/stt.md
* fix(helm): fail fast when stt.enabled=true but apiKey is empty
---------
Co-authored-by: openab-bot <openab-bot@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Set image.tag to empty string so the Helm template falls back to .Chart.AppVersion. Closes openabdev#235
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Co-authored-by: openab-app[bot] <274185012+openab-app[bot]@users.noreply.github.com>
Add `.github/copilot-instructions.md` to guide GitHub Copilot's automated PR reviews with project-specific context: - ACP protocol correctness (JSON-RPC routing, notification handling) - Concurrency safety (atomic fields, Mutex across await, pool locks) - Discord API constraints (2000 char limit, 3s autocomplete deadline) - Skip CI-covered checks (rustfmt, clippy, tests) - >80% confidence threshold to reduce noise Modeled after block/goose's production-grade instructions. 3440 chars — within the 4000 char limit.
Author
|
Merging into #260 as a single PR. |
There was a problem hiding this comment.
Pull request overview
This PR significantly expands the project beyond adding Copilot review instructions: it renames the crate/service to openab, adds Discord attachment handling (voice STT + images) and ACP prompt content blocks, and replaces/updates deployment + release automation (Helm chart + GitHub workflows) alongside documentation updates.
Changes:
- Rename/rebrand
agent-broker→openabacross Rust crate metadata, logs, docs, Dockerfiles, and k8s manifests. - Add Discord attachment ingestion: voice message STT via OpenAI-compatible
/audio/transcriptions+ image resize/compress/base64 → ACPContentBlockprompt payloads. - Introduce a new multi-agent Helm chart and tag-driven build/release workflows, with new release/triage documentation.
Reviewed changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/stt.rs | New STT client for /audio/transcriptions (OpenAI-compatible). |
| src/main.rs | Adds modules, allowlist parsing, STT env auto-detect, and handler wiring. |
| src/format.rs | Unicode-safe message splitting + new truncate helper for streaming edits. |
| src/error_display.rs | New Discord-facing error formatting helpers with unit tests. |
| src/discord.rs | Major handler changes: user allowlist, sender context injection, STT + image content blocks, improved tool rendering, and streaming behavior updates with tests. |
| src/config.rs | Adds [stt] config + allowed_users to Discord config. |
| src/acp/protocol.rs | Adds tool call IDs to events; updates notification classification. |
| src/acp/mod.rs | Re-exports ContentBlock. |
| src/acp/connection.rs | Adds ContentBlock and sends structured prompt blocks to ACP. |
| RELEASING.md | New release process documentation. |
| README.md | Rebrand to OpenAB, update features/install guidance, and add STT mention. |
| k8s/secret.yaml | Rename secret to openab-secret. |
| k8s/pvc.yaml | Rename PVC to openab-data. |
| k8s/deployment.yaml | Rename deployment/app labels; add pod/container security contexts; update mount paths and secret references. |
| k8s/configmap.yaml | Rename ConfigMap; update working dir path. |
| docs/stt.md | New STT configuration and usage documentation. |
| docs/steering/triage.md | New issue triage guide. |
| docs/helm-publishing.md | New Helm publishing documentation. |
| docs/gh-auth-device-flow.md | New guide for gh device-flow auth in headless agent envs. |
| docs/discord-bot-howto.md | Update bot setup docs for openab; document allowed_users. |
| Dockerfile.gemini | New image variant for Gemini CLI agent. |
| Dockerfile.codex | New image variant for Codex agent. |
| Dockerfile.claude | New image variant for Claude agent. |
| Dockerfile | Rebrand binary, add gh CLI, add non-root user, add healthcheck, improve build caching. |
| config.toml.example | Update working_dir and document allowed_users. |
| charts/openab/values.yaml | New multi-agent chart values (including STT + allowlists). |
| charts/openab/templates/secret.yaml | New per-agent Secret template (bot token + optional STT key). |
| charts/openab/templates/pvc.yaml | New per-agent PVC template. |
| charts/openab/templates/NOTES.txt | New Helm post-install notes with per-agent auth guidance. |
| charts/openab/templates/deployment.yaml | New per-agent Deployment template with security contexts and mounts. |
| charts/openab/templates/configmap.yaml | New per-agent ConfigMap template emitting config.toml (+ STT gate). |
| charts/openab/templates/_helpers.tpl | New Helm helper functions (naming, enablement, image resolution). |
| charts/openab/Chart.yaml | New chart identity/versioning for openab. |
| charts/agent-broker/values.yaml | Removes legacy chart values. |
| charts/agent-broker/templates/secret.yaml | Removes legacy chart template. |
| charts/agent-broker/templates/pvc.yaml | Removes legacy chart template. |
| charts/agent-broker/templates/NOTES.txt | Removes legacy chart template. |
| charts/agent-broker/templates/deployment.yaml | Removes legacy chart template. |
| charts/agent-broker/templates/configmap.yaml | Removes legacy chart template. |
| charts/agent-broker/templates/_helpers.tpl | Removes legacy chart helpers. |
| Cargo.toml | Renames crate to openab, bumps version, adds reqwest/base64/image deps. |
| Cargo.lock | Updates lockfile for new deps (but root package version mismatch noted in comments). |
| .github/workflows/tag-on-merge.yml | New workflow to tag releases on merge of release/* PRs. |
| .github/workflows/release.yml | Updates chart release workflow to target charts/openab. |
| .github/workflows/release-pr.yml | New workflow to generate version bump release PRs. |
| .github/workflows/issue-triage.yml | New workflow to auto-label issues with needs-triage. |
| .github/workflows/ci.yml | New Rust CI workflow (check/clippy/test) scoped to relevant paths. |
| .github/workflows/build.yml | Replaces push-to-main build with tag-driven build/promotion + OCI chart push. |
| .github/ISSUE_TEMPLATE/guidance.yml | New guidance issue template. |
| .github/ISSUE_TEMPLATE/feature.yml | New feature request template. |
| .github/ISSUE_TEMPLATE/config.yml | Disables blank issues. |
| .github/ISSUE_TEMPLATE/bug.yml | New bug report template. |
| .github/copilot-instructions.md | Adds Copilot PR review instructions. |
| .github/CODEOWNERS | Adds default code owner configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+10
to
+19
| let msg_lower = message.to_lowercase(); | ||
|
|
||
| // Startup / connection errors (code == 0 from anyhow) | ||
| if msg_lower.contains("timeout waiting for") { | ||
| // Use msg_lower for extraction to stay case-insistent with the match above. | ||
| // msg_lower and message are the same length, so byte offsets are valid. | ||
| if let Some(start) = msg_lower.find("timeout waiting for ") { | ||
| let rest = &message[start + "timeout waiting for ".len()..]; | ||
| let method = rest.split_whitespace().next().unwrap_or("request"); | ||
| return format!("**Request Timeout**\nTimeout waiting for {}, please try again.", method); |
There was a problem hiding this comment.
format_user_error computes start on message.to_lowercase() and then uses that byte offset to slice the original message, but to_lowercase() is not guaranteed to preserve UTF-8 byte length (some Unicode chars expand), so this can slice at the wrong boundary or panic. Consider doing extraction on the original string using a case-insensitive matcher (e.g., regex with case_insensitive(true)) or avoid cross-string byte offsets entirely.
|
|
||
| // Startup / connection errors (code == 0 from anyhow) | ||
| if msg_lower.contains("timeout waiting for") { | ||
| // Use msg_lower for extraction to stay case-insistent with the match above. |
There was a problem hiding this comment.
Typo in comment: "case-insistent" should be "case-insensitive".
Comment on lines
+1
to
+19
| {{- range $name, $cfg := .Values.agents }} | ||
| {{- if ne (include "openab.agentEnabled" $cfg) "false" }} | ||
| {{- if $cfg.discord.botToken }} | ||
| {{- $d := dict "ctx" $ "agent" $name "cfg" $cfg }} | ||
| --- | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: {{ include "openab.agentFullname" $d }} | ||
| labels: | ||
| {{- include "openab.labels" $d | nindent 4 }} | ||
| annotations: | ||
| "helm.sh/resource-policy": keep | ||
| type: Opaque | ||
| data: | ||
| discord-bot-token: {{ $cfg.discord.botToken | b64enc | quote }} | ||
| {{- if and ($cfg.stt).enabled ($cfg.stt).apiKey }} | ||
| stt-api-key: {{ $cfg.stt.apiKey | b64enc | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
This Secret is only rendered when discord.botToken is set, but the Deployment can still reference stt-api-key when stt.enabled=true/stt.apiKey is provided; if users provide the bot token via an externally-managed Secret (or omit it here), STT will break because the Secret won't exist. Render the Secret when either discord.botToken or stt.apiKey is set, and include whichever keys are present (and update NOTES accordingly).
Comment on lines
+3
to
+28
| OpenAB publishes the Helm chart to two channels automatically via the `Release Charts` workflow (`.github/workflows/release.yml`). | ||
|
|
||
| ## Channels | ||
|
|
||
| | Channel | URL | Install command | | ||
| |---------|-----|-----------------| | ||
| | GitHub Pages | `https://openabdev.github.io/openab` | `helm repo add openab https://openabdev.github.io/openab && helm install openab openab/openab` | | ||
| | OCI (GHCR) | `oci://ghcr.io/openabdev/charts/openab` | `helm install openab oci://ghcr.io/openabdev/charts/openab` | | ||
|
|
||
| ## How it works | ||
|
|
||
| ``` | ||
| charts/openab/Chart.yaml changed on main | ||
| │ | ||
| ▼ | ||
| ┌─────────────────────────────┐ | ||
| │ Release Charts workflow │ | ||
| │ .github/workflows/ │ | ||
| │ release.yml │ | ||
| │ │ | ||
| │ 1. helm package │ | ||
| │ 2. helm push → OCI (GHCR) │ | ||
| │ 3. cr upload → GH Release │ | ||
| │ 4. cr index → gh-pages │ | ||
| │ 5. Update release notes │ | ||
| └─────────────────────────────┘ |
There was a problem hiding this comment.
This doc says .github/workflows/release.yml both packages and helm pushes to OCI, but the current release.yml no longer pushes to OCI; OCI publishing appears to happen in build.yml (tag-driven) instead. Update this document’s flow/trigger/inputs sections to reflect the actual pipeline, otherwise users will follow incorrect release instructions.
Comment on lines
31
to
73
| @@ -78,14 +62,14 @@ jobs: | |||
|
|
|||
| ### Helm Repository (GitHub Pages) | |||
| \`\`\`bash | |||
| helm repo add agent-broker https://${OWNER}.github.io/${REPO} | |||
| helm repo add openab https://${OWNER}.github.io/${REPO} | |||
| helm repo update | |||
| helm install agent-broker agent-broker/agent-broker --version ${VERSION} | |||
| helm install openab openab/openab --version ${VERSION} | |||
| \`\`\` | |||
|
|
|||
| ### OCI Registry | |||
| \`\`\`bash | |||
| helm install agent-broker oci://ghcr.io/${OWNER}/agent-broker --version ${VERSION} | |||
| helm install openab oci://ghcr.io/${OWNER}/charts/openab --version ${VERSION} | |||
| \`\`\` | |||
There was a problem hiding this comment.
release.yml appends OCI install instructions to the GitHub release notes, but this workflow does not publish the chart to OCI anymore; users may try to install a version that hasn’t been pushed. Either restore the OCI publish step here, or only include OCI instructions when the tag-driven build.yml has published the chart.
Comment on lines
+119
to
+122
| let prompt_with_sender = format!( | ||
| "<sender_context>\n{}\n</sender_context>\n\n{}", | ||
| serde_json::to_string(&sender_ctx).unwrap(), | ||
| prompt |
There was a problem hiding this comment.
serde_json::to_string(&sender_ctx).unwrap() can panic in the Discord message handler; even if it’s unlikely to fail for this payload, a panic here would drop the event task and can take down the bot depending on runtime settings. Prefer handling the Result (fallback to a minimal context string) instead of unwrapping in a production path.
Comment on lines
+255
to
+268
| const MAX_SIZE: u64 = 25 * 1024 * 1024; // 25 MB (Whisper API limit) | ||
|
|
||
| if u64::from(attachment.size) > MAX_SIZE { | ||
| error!(filename = %attachment.filename, size = attachment.size, "audio exceeds 25MB limit"); | ||
| return None; | ||
| } | ||
|
|
||
| let resp = HTTP_CLIENT.get(&attachment.url).send().await.ok()?; | ||
| if !resp.status().is_success() { | ||
| error!(url = %attachment.url, status = %resp.status(), "audio download failed"); | ||
| return None; | ||
| } | ||
| let bytes = resp.bytes().await.ok()?.to_vec(); | ||
|
|
There was a problem hiding this comment.
download_and_transcribe trusts attachment.size but doesn’t re-check the actual downloaded size; if the response is larger than expected (or attachment.size is inaccurate), this will buffer the entire body into memory and can lead to OOM. Add a post-download size check (like the image path does) and/or stream with a hard cap.
Comment on lines
+1
to
+16
| # GitHub Copilot Code Review Instructions | ||
|
|
||
| ## Review Philosophy | ||
| - Only comment when you have HIGH CONFIDENCE (>80%) that an issue exists | ||
| - Be concise: one sentence per comment when possible | ||
| - Focus on actionable feedback, not observations | ||
| - Silence is preferred over noisy false positives | ||
|
|
||
| ## Project Context | ||
| - **OpenAB**: A lightweight ACP (Agent Client Protocol) harness bridging Discord ↔ any ACP-compatible coding CLI over stdio JSON-RPC | ||
| - **Language**: Rust 2021 edition, single binary | ||
| - **Async runtime**: tokio (full features) | ||
| - **Discord**: serenity 0.12 (gateway + cache) | ||
| - **Error handling**: `anyhow::Result` everywhere, no `unwrap()` in production paths | ||
| - **Serialization**: serde + serde_json for ACP JSON-RPC, toml for config | ||
| - **Key modules**: `acp/connection.rs` (ACP stdio bridge), `acp/pool.rs` (session pool), `discord.rs` (Discord event handler), `config.rs` (TOML config), `usage.rs` (pluggable quota runners), `reactions.rs` (emoji reactions), `stt.rs` (speech-to-text) |
There was a problem hiding this comment.
The PR title/description say this change only adds .github/copilot-instructions.md, but this PR also includes a project rename (agent-broker → openab), new STT/image handling, Helm chart rewrites, and CI/release pipeline changes. Please update the PR description (or split into focused PRs) so reviewers understand the full scope and risk.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
.github/copilot-instructions.mdto guide Copilot's automated PR reviews with OpenAB-specific context.Reopened from #261 (closed due to history rewrite for secret cleanup).
Motivation
Copilot code review currently uses generic analysis. With project-specific instructions, it can focus on what matters for OpenAB:
Modeled after block/goose's production-grade instructions. 3440 / 4000 chars — within Copilot's read limit.
Changes
.github/copilot-instructions.md