Skip to content

docs(adr): separate binaries by default, opt-in unified build#1116

Merged
thepagent merged 5 commits into
mainfrom
docs/adr-unified-binary
Jun 19, 2026
Merged

docs(adr): separate binaries by default, opt-in unified build#1116
thepagent merged 5 commits into
mainfrom
docs/adr-unified-binary

Conversation

@chaodu-agent

@chaodu-agent chaodu-agent commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Proposes restructuring OpenAB as a Cargo workspace that keeps two separate binaries by default (core + gateway), while enabling a single unified binary via --features unified or a Dockerfile BUILD_MODE arg — zero code changes required.

Key Design

  • Default: cargo build → core binary (Discord + Slack). Gateway ships separately.
  • Unified opt-in: cargo build --features unified → all adapters in one binary.
  • Dockerfile: docker build --build-arg BUILD_MODE=unified flips to single binary.
  • Runtime activation: compiled-in adapters only start if their config section is present.
  • Feature flags: granular — pick just telegram,line if you don't need all adapters.

Published Artifacts

Image Contents
openab:latest Core only (Discord + Slack)
openab-gateway:latest Standalone gateway
openab:unified All adapters in single binary

Changes

  • docs/adr/unified-binary.md — new ADR document

Add ADR for restructuring OpenAB as a Cargo workspace that ships all
platform adapters in a single binary, activated at runtime via config.
This eliminates the two-process (core + gateway sidecar) deployment
model for most users while keeping the standalone gateway available
for advanced use cases.
@chaodu-agent chaodu-agent requested a review from thepagent as a code owner June 15, 2026 03:20
@chaodu-agent

This comment has been minimized.

- Default: two separate binaries (core + gateway), unchanged from today
- Opt-in: --features unified compiles all adapters into single binary
- Zero code changes needed — just a build flag or image swap
- Published: openab:latest (core), openab:unified (all-in-one)
@chaodu-agent

This comment has been minimized.

@chaodu-agent chaodu-agent changed the title docs(adr): unified single-binary architecture docs(adr): separate binaries by default, opt-in unified build Jun 15, 2026
@canyugs

canyugs commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

From Fellowship of the Code

## 🧙 Gandalf — Aggregated Review Synthesis for PR #1116

### Verdict: **Request Changes** (4 request-changes, 2 approves, 1 request-changes — 7/7 reviewers reported)

---

### 🔴 Critical / Must-Fix

| # | Reviewer | Finding |
|---|----------|---------|
| F1 | Aragorn | **Silent feature-default regression**`secrets-aws`/`agentcore` defaults dropped. Plain `cargo build` would lose AWS Secrets Manager support — a deployment regression hiding inside a docs-only PR. **Top-priority fix.** |
| F1 | Merry | **Config contract contradiction** — "Default behavior unchanged" (L213) conflicts with `[telegram]`/`[line]` sections working in both modes (L215). Today's two-binary path uses `[gateway]` config + env/Helm on the gateway side. Split the config contract: `[gateway]` stays as two-binary core contract; `[telegram]`/`[line]` are unified-mode only unless a compatibility/translation layer is specified. |
| F2 | Sam | **"Supersedes" overstates the relationship** — The custom-gateway ADR's outbound-only contract still holds (gateway is default). Use "Related" / "Amends" instead. Suggested reword (Frodo): `- **Amends:** Deployment model from [ADR: Custom Gateway](./custom-gateway.md) — outbound-only + gateway sidecar remains the default and is only waived under the --features unified (or BUILD_MODE=unified) opt-in path.` |
| F4 | Aragorn | **Superseding a Proposed ADR** — Cannot supersede a document still in Proposed status. Fix the lifecycle reference. |

---

### 🟡 Suggestions / Nits

| # | Reviewer | Finding |
|---|----------|---------|
| N1 | Boromir | Dockerfile `BUILD_MODE` RUN block is fragile for `FEATURES` quoting/multi-arg. Consider a dedicated target or `.cargo/config.toml` note. (Frodo notes this belongs in the implementation PR, not the ADR.) |
| N1 | Sam | `src/telegram.rs` example (L145) doesn't match workspace layout (`crates/openab-gateway/`). Use crate-level path or mark as illustrative. |



| N5 | Frodo | Workspace layout shows both `gateway/` and `crates/openab-gateway/` during transition — duplication risk and intended relationship (thin binary re-exporting the library crate, not a second copy) should be called out in the ADR body. |

---

### 🟢 Positive Observations

- **Correct conservative direction**: Flipping to separate-by-default with unified as opt-in respects "zero disruption" and existing users (Boromir, Sam, Frodo, Merry, Aragorn).
- **Clean runtime activation model**: compiled-in but unconfigured adapters have zero overhead; start only if config section is present (Frodo). Worth a "Design Principle" callout.
- **ADR authorship quality**: Scope correction between commits is model process (Boromir).
- **Opt-in via feature flag** is well-calibrated for an active fleet — existing two-binary deployments need no migration (Aragorn).

---

### 🔧 Recommended Actions (priority order)

1. **[Blocking]** Restore `secrets-aws`/`agentcore` as default features so plain `cargo build` doesn't regress.
2. **[Blocking]** Fix ADR lifecycle: cannot supersede a Proposed ADR — correct the status/relationship.
3. Fix config contract: clearly separate `[gateway]` (two-binary) from `[telegram]`/`[line]` (unified-only).
4. Change "Supersedes" → "Amends" with explicit outbound-only default statement.
5. Address workspace layout duplication narrative in the ADR body.
6. Minor: fix example path to match crate structure.

---

*7/7 reviewers reported. 4 request-changes, 2 approves.*

@chaodu-agent

This comment has been minimized.

- Status: Proposed → Accepted
- Workspace layout: core stays in root crate, openab-core deferred to Phase 2
- Feature flags: discord = ["dep:serenity"] (not openab-core/discord)
- Dockerfile: document root (--no-default-features) vs agent (additive) semantics
- Add cross-reference to docs/image-tags.md for tagging convention
- Add implementation link to PR #1146

Consensus: team unanimously chose option 2 (structural alignment only,
operational details stay in dedicated docs).
@chaodu-agent

chaodu-agent commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator Author

ADR Updated — Aligned with Implementation (#1146) ✅

Changes (2 commits: 9b391b7 + 1c7b13f)

# Section Update
1 Status ProposedAccepted + implementation link to #1146
2 Workspace Layout Corrected: core stays in root crate, openab-core extraction deferred to Phase 2
3 Feature Flags Updated to match implementation: discord = ["dep:serenity"], added secrets-aws/agentcore in defaults, added optional = true dependency example
4 Dockerfile Added build semantics table: root (--no-default-features) vs agent (additive --features)
5 Image Tagging Not added to ADR — cross-reference link to docs/image-tags.md instead
6 Supersedes → Amends Custom-gateway ADR is still Proposed — cannot be superseded. Changed to "Amends" with scoped language preserving outbound-only default
7 Config contract Clarified: [telegram]/[line] are unified-mode additions; [gateway] remains the two-binary contract

External Review Findings Addressed

Responding to @canyugs Fellowship of the Code review:

Finding Severity Status
F1: secrets-aws/agentcore dropped from defaults 🔴 ✅ Fixed — restored in proposed default features
F2: "Supersedes" invalid lifecycle 🔴 ✅ Fixed — changed to "Amends" with scoped language
F3: Config contract contradiction 🟡 ✅ Fixed — clarified [telegram]/[line] are unified-mode only
F4: Cannot supersede Proposed ADR 🔴 ✅ Fixed — merged with F2
F5: Missing optional = true declaration 🟡 ✅ Fixed — added dependency example
F6: gateway/ vs crates/openab-gateway/ unclear 🟡 ✅ Fixed — new layout makes relationship clear
F7: src/telegram.rs path contradicts layout 🟡 ℹ️ Acceptable — §4 is a conceptual flow diagram, not a file path reference
N1: Dockerfile quoting fragility 🟡 ℹ️ Implementation concern — addressed in #1146, not ADR scope

Team Consensus

All 5 reviewers chose Option 2 — update structural decisions only, keep operational details in dedicated docs.

Principle Applied

ADR records decisions and rationale. Implementation details live in code and dedicated docs. The two stay aligned on structural boundaries but don't duplicate each other.


This PR is now ready for merge — all critical and important findings from both internal team and external reviewers have been addressed.

- F2: Supersedes → Amends (custom-gateway ADR is still Proposed)
- F3: Clarify config contract — [telegram]/[line] are unified-mode additions,
  [gateway] remains the two-binary contract
- F5: Add optional dependency declaration example for dep:openab-gateway
@thepagent thepagent enabled auto-merge (squash) June 19, 2026 15:16
@thepagent thepagent merged commit 3d296c9 into main Jun 19, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants