chore: add .gitignore and AGENTS.md; fix implementation-specialist conventions
The scaffold was missing basic repo hygiene, which led to drift: the implementation-specialist agent's Project Conventions section was a verbatim copy from alkcall (referencing OperationEnv, EventEnvelope, Capabilities, alktype as a dep, call.aborted, vendored core types — none of which exist in alktty), so spawned agents got alkcall conventions injected. - .gitignore: target/, node_modules/, .worktrees/ (matches alkcall/alktype) - AGENTS.md: alktty-specific operating instructions — Git Workflow, 14 Project Conventions (5-byte chunk header wire format, TtyBackend trait one-way door, local feature isolation / WASM invariant, BAST-as-doc-not-dep, 3-arg/4-arg ownership shapes, alknet ADR→alktty ADR renumbering plan), Verification Commands (incl. wasm checks), Architecture Context (phase status, ADR index, port origin) - .opencode/agents/implementation-specialist.md: replace stale alkcall Project Conventions section with alktty-appropriate rules that mirror AGENTS.md Verification: cargo test (80 passed), cargo fmt --check, cargo build --target wasm32-unknown-unknown all clean. No source changes.
This commit is contained in:
1 parent
e1610c2825
commit
712e7ae071
3 files changed
+390
-49
No files matched your search
@@ -210,55 +210,101 @@ This is especially important for complex tasks that span many file operations.
|
||||
Read `AGENTS.md` at project root for full details. Key rules:
|
||||
|
||||
1. **No comments in code** — Per project convention. Doc comments (`///`, `//!`)
|
||||
are fine and expected on public API.
|
||||
2. **Error handling** — `thiserror` for library error types. No panics in
|
||||
library code. No `unwrap()` or `expect()` outside tests. For poisoned
|
||||
`RwLock`/`Mutex`, use `unwrap_or_else(|e| e.into_inner())` so a panic in one
|
||||
operation does not cascade to other operations.
|
||||
3. **`tokio` is the async runtime** — all I/O is async. Use `tokio::sync`
|
||||
primitives (`oneshot`, `mpsc`) for request correlation and subscription
|
||||
channels; `parking_lot` for short-held internal locks (`PendingRequestMap`).
|
||||
4. **No secret material on the wire** — `call.requested`/`call.responded`
|
||||
payloads and `OperationContext.metadata` carry no private keys, API keys, or
|
||||
decrypted credentials. Outbound credentials flow through `Capabilities`
|
||||
injected at the assembly layer → `HandlerRegistration.capabilities` →
|
||||
`OperationContext.capabilities` → handler.
|
||||
5. **No-env-vars invariant** — no handler reads outbound credentials from any
|
||||
source other than `OperationContext.capabilities`. This is a spec-level
|
||||
invariant, not a runtime convention.
|
||||
6. **`OperationEnv` must remain a trait** — the trait-based design enables
|
||||
registry layering (session overlays, connection overlays, peer-keyed
|
||||
composition). Do not make it concrete or hardcode the global registry.
|
||||
7. **Wire formats are stable** — `EventEnvelope` (`{ type, id, payload }` +
|
||||
length-prefixed JSON framing) and the channels 8-byte chunk header
|
||||
(`[channel_id:u32 BE][length:u32 BE][payload]`) are one-way doors. New event
|
||||
types may be added; existing shapes must not change.
|
||||
8. **Producer/consumer, not server/client** — both sides of a call or channels
|
||||
connection can initiate. Use "producer"/"consumer" or "accept side"/"connect
|
||||
side," not "server"/"client."
|
||||
9. **Vendored core types** — `Connection`, `ProtocolHandler`, `BiStream`,
|
||||
`BidiStreamSource`, `AuthContext`, `IdentityProvider`, `Identity`,
|
||||
`AuthToken`, `Capabilities`, `OwnershipProvider`, `HandlerError`,
|
||||
`StreamError` live in this crate. Do not add a separate `alkcore` dependency.
|
||||
Keep them lean (no TLS, no transport coupling, no endpoint/accept-loop).
|
||||
10. **`alktype` dependency** — use `alktype` for binary layout (channels chunk
|
||||
header, future binary payload schemas) and JSON payload schema validation
|
||||
(`OperationSpec`'s `input_schema`/`output_schema`). Do not roll your own.
|
||||
11. **Feature flags** — transports may be feature-gated if the need arises. The
|
||||
base crate should compile lean (no `quinn`, no `iroh` unless the feature is
|
||||
on). Verify both `cargo test` (default) and `cargo test --all-features` pass
|
||||
if features are added.
|
||||
12. **Abort cascades to descendants** — `call.aborted` for a parent cascades to
|
||||
all non-terminal descendants. Default `abort-dependents`;
|
||||
`continue-running` opt-in. The composing handler decides the child's policy,
|
||||
not the wire caller.
|
||||
13. **Peer authorization via `AccessControl`** — a remote peer's call is
|
||||
authorized by `AccessControl::check(peer_identity)`. No `remote_safe` flag,
|
||||
no `trusted_peer` bypass. `Visibility::Internal` ops are never wire-callable.
|
||||
14. **Naming conventions** — Rust standard: `snake_case` for functions/variables/
|
||||
modules, `PascalCase` for types/traits, `SCREAMING_SNAKE_CASE` for constants.
|
||||
15. **Module structure** — one module per file under `src/`, re-exported from
|
||||
`src/lib.rs`. Public API surface is `lib.rs` re-exports.
|
||||
are fine and expected on public API. Inline `//` comments only when the user
|
||||
asks or when a non-obvious safety/correctness constraint would otherwise be
|
||||
missed (e.g., "zero-length stdin is the EOF sentinel — the codec does not
|
||||
special-case it; the adapter interprets `length == 0` chunks").
|
||||
2. **Error handling** — `thiserror` for library error types (`TtyError`,
|
||||
`NegotiateError`, `WireError`; `HandlerError`/`StreamError` come from
|
||||
alkcall::core). No panics in library code. No `unwrap()` or `expect()`
|
||||
outside tests. For poisoned `RwLock`/`Mutex`, use
|
||||
`unwrap_or_else(|e| e.into_inner())`.
|
||||
3. **`tokio` is the async runtime** — all I/O is async. The adapter's
|
||||
per-session pumps, the channels integration, and `TtySession` are all async.
|
||||
Use `tokio::sync` primitives (`oneshot`, `mpsc`) for request/exit
|
||||
correlation and stdout/stderr streams. The PTY bridge in `src/local/pty.rs`
|
||||
is the one exception: three dedicated std threads feeding tokio mpsc/oneshot
|
||||
channels (the `portable_pty` API is blocking) — behind the `local` feature.
|
||||
4. **WASM target is load-bearing** — the default crate (no `local` feature)
|
||||
MUST compile to `wasm32-unknown-unknown`. This is what makes the downstream
|
||||
TS/Python adapter story work. `Cargo.toml` uses
|
||||
`tokio = { default-features = false, features = ["rt", "sync", "io-util", "macros"] }`
|
||||
(the wasm-clean subset alkcall uses); `local` adds `tokio/process` +
|
||||
`tokio/rt-multi-thread`. **Do NOT use `features = ["full"]`** — it pulls in
|
||||
`signal`/`fs`/`net` which break `wasm32-unknown-unknown`. The adapter's
|
||||
`tokio::spawn` is fine on wasm; the `local` module's std threads,
|
||||
`tokio::process::Command`, and `libc::kill` are the non-wasm parts, and
|
||||
they're feature-gated.
|
||||
5. **Wire formats are stable** — two one-way doors:
|
||||
- **5-byte chunk header** (`[stream_type: u8][length: u32 BE][payload]`) —
|
||||
the raw chunk codec in `wire.rs`. Five stream types: `STREAM_STDIN=0`,
|
||||
`STREAM_STDOUT=1`, `STREAM_STDERR=2`, `STREAM_CTRL_IN=3`,
|
||||
`STREAM_CTRL_OUT=4`. Zero-length data chunks are sentinels (zero-length
|
||||
stdin = EOF from client; zero-length stdout = "drained" from server);
|
||||
control chunks are never zero-length. Changing the header, the
|
||||
stream-type values, or the sentinel semantics breaks all peers.
|
||||
- **Negotiation frame** (4-byte BE length prefix + UTF-8 JSON
|
||||
`NegotiateRequest` body) — self-contained, not reused from alkcall's
|
||||
`EventEnvelope` framing. The `NegotiateRequest` JSON shape is
|
||||
wire-stable once consumers exist.
|
||||
Per ADR-093: TTY always uses its 5-byte format, even inside channels. The
|
||||
channels layer strips its 8-byte header transparently — the same `wire.rs`
|
||||
code runs in both direct (`alk/tty`) and channels (`alk/channels`) modes;
|
||||
only the `BiStream` source differs.
|
||||
6. **`TtyBackend` trait shape is a one-way door** — the trait in
|
||||
`src/backend.rs` is the inversion point between the wire-format adapter and
|
||||
the backend crates (alktty's own `local` feature module, future
|
||||
`alknet-docker`, `alknet-ssh`). alktty defines the trait; the backends
|
||||
implement it. The adapter holds a `HashMap<String, Arc<dyn TtyBackend>>`
|
||||
keyed by the negotiation frame's `backend` string; backends produce
|
||||
`TtyHandle`s, they do not write to the wire. `TtyError` is
|
||||
`#[non_exhaustive]` so new variants are additive.
|
||||
7. **Producer/consumer, not server/client** — both sides of a `alk/tty` or
|
||||
`alk/channels` connection can initiate. Use "producer"/"consumer" or
|
||||
"accept side"/"connect side," not "server"/"client."
|
||||
8. **`local` feature isolation** — the `local` module is the only non-wasm
|
||||
part of the crate. The shared/producer/consumer modules (`wire`, `control`,
|
||||
`negotiation`, `backend`, `adapter`, `channels`, `session`) MUST NOT import
|
||||
from `crate::local` or reference any `local`-only type (`LocalTtyBackend`,
|
||||
`portable_pty`, `tokio::process::Command`). The local backend is injected at
|
||||
the assembly layer, not wired in by the adapter. A regression where a
|
||||
non-`local` module pulls in `portable_pty`, `tokio::process`,
|
||||
`std::thread`, or `libc` (outside the existing `cfg(unix)` calls in
|
||||
`control.rs`/`pipe.rs`) silently breaks the downstream TS/Python adapter
|
||||
story.
|
||||
9. **Module structure** — one module per file under `src/`, re-exported from
|
||||
`src/lib.rs`. Producer half: `src/adapter.rs` + `src/channels.rs`. Consumer
|
||||
half: `src/session.rs`. Shared: `src/wire.rs`, `src/control.rs`,
|
||||
`src/negotiation.rs`, `src/backend.rs`. The `src/local/` module is behind
|
||||
the `local` feature and never imported from the shared/producer/consumer
|
||||
modules — only re-exported from `lib.rs` under `#[cfg(feature = "local")]`.
|
||||
10. **BAST document, not alktype dependency** — `docs/architecture/tty-bast.md`
|
||||
(Phase 4, not yet written) will be the machine-readable spec for the
|
||||
`alk/tty` wire format, conforming to the BAST meta-schema at
|
||||
`https://alk.dev/bast/v1/schema`. alktty does **not** depend on alktype;
|
||||
the hand-rolled `ChunkReader`/`ChunkWriter` in `wire.rs` is the runtime
|
||||
codec, the BAST document is the human-readable contract. If runtime
|
||||
validation against the BAST becomes desirable later, alktype becomes an
|
||||
optional dep — do not roll your own offset map or validator for complex
|
||||
formats.
|
||||
11. **Feature flags** — the only feature is `local` (default = `[]`). `local`
|
||||
is inherently non-wasm; enabling it on wasm is a build error by design.
|
||||
Verify both `cargo test` (default) and `cargo test --all-features` pass.
|
||||
Do not add a feature flag that pulls non-wasm deps into the default crate.
|
||||
12. **Access control** — the direct-ALPN path keeps the ad-hoc scope check
|
||||
(`has_scope(identity, TTY_OPEN_SCOPE)`) and optionally consults
|
||||
`OwnershipProvider` (`provider.owns(id_ref, kind, &id, "tty")` — the 4-arg
|
||||
shape). The channels path gets both for free via
|
||||
`ChannelCore::register_openable`, which wires `AccessControl` into the
|
||||
operation spec. `OwnershipStore::record(&self, identity, resource_type,
|
||||
resource_id)` is the 3-arg shape (alkcall dropped the old `action` arg).
|
||||
13. **Naming conventions** — Rust standard: `snake_case` for functions/variables/
|
||||
modules, `PascalCase` for types/traits, `SCREAMING_SNAKE_CASE` for
|
||||
constants (`STREAM_STDIN`, `TTY_OPEN_SCOPE`, `CHUNK_HEADER_LEN`).
|
||||
14. **No `unsafe`** — the crate has zero `unsafe` blocks. The PTY bridge's
|
||||
`libc::kill(-pgid, sig)` and `libc::kill(pid, sig)` calls are safe `libc`
|
||||
crate APIs (not `unsafe` blocks in this crate). Do not introduce `unsafe`
|
||||
for performance.
|
||||
|
||||
## Key Principles
|
||||
|
||||
|
||||
Reference in new issue
Block a user