Skip to content

[codex] Add architecture review#32

Merged
bglusman merged 1 commit intomainfrom
codex-architecture-review
Apr 25, 2026
Merged

[codex] Add architecture review#32
bglusman merged 1 commit intomainfrom
codex-architecture-review

Conversation

@bglusman
Copy link
Copy Markdown
Owner

Summary

  • Adds a repository-level architecture review covering crate boundaries, trust boundaries, duplicated policy/proxy/credential responsibilities, and model gateway direction.
  • Recommends a shared decision envelope, clearer zeroclawed internal domains, OneCLI-only secret materialization, wrapper-first host-agent architecture, and RFC-aligned model gateway work.

Why

The project has strong sidecar separation already, but several components now overlap on policy, proxy, credential, and gateway concepts. This review captures small consolidation moves before those concepts drift further.

Validation

  • pre-push hook: fmt, clippy, workspace unit tests, loom tests, workspace checks

Draft until the recommendations are triaged into concrete implementation issues or PRs.

Copilot AI review requested due to automatic review settings April 25, 2026 03:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a repository-level architecture review document to capture current crate/trust boundaries, highlight overlapping policy/proxy/credential responsibilities, and recommend small consolidation steps before further drift.

Changes:

  • Introduces a dated architecture review doc covering policy planes, zeroclawed responsibility boundaries, credential materialization direction, wrapper-first host-agent hardening, and model gateway RFC sequencing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +9 to +12
The repository has six Rust crates:

| Crate | Current role |
|---|---|
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex integration sweep: acknowledged. I am leaving this PR branch untouched per the parallel-agent boundary; this remains actionable for the PR owner or a follow-up unless it is superseded by #38.

Comment on lines +126 to +127
This also supports the fnox direction: a fnox writer or UI can create secret
material, but runtime callers still get only injection, not readback.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex integration sweep: acknowledged. I am leaving this PR branch untouched per the parallel-agent boundary; this remains actionable for the PR owner or a follow-up unless it is superseded by #38.

@bglusman
Copy link
Copy Markdown
Owner Author

Acknowledged. The architecture review document is helpful as a reference — no merge conflicts expected since it's docs-only. Will keep visible during follow-up work on the gateway / MCP integration.

@bglusman bglusman marked this pull request as ready for review April 25, 2026 18:40
@bglusman bglusman merged commit 18defe0 into main Apr 25, 2026
18 checks passed
bglusman added a commit that referenced this pull request Apr 25, 2026
Codex agent's strategic architecture review. Five findings: shared decision envelope, zeroclawed crate ownership boundaries, credential injection consolidation onto onecli, wrapper-first host-agent default, model-gateway-RFC implementation sequencing. Merging as the doc reference for the architecture work to come.
@bglusman bglusman deleted the codex-architecture-review branch May 1, 2026 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants