mirror of
https://github.com/agent-substrate/substrate.git
synced 2026-10-02 03:24:42 +08:00
main
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maya Wang
|
c618baa593 |
docs: convention for integration repository structure and naming (#712)
## Summary Adds `docs/integration-repos.md`: where end-to-end integrations live, how their repositories are named, and how the fixes they need flow back into core. The convention in one line — trivial demos stay in the core repo, each non-trivial integration gets one dedicated repo under the `agent-substrate` org, and core gaps get closed by making core configurable with defaults unchanged rather than by patching it downstream. ## Why now We are about to create the first real, end-to-end integrations rather than counter-style demos: a code-execution sandbox, and an always-on agent. Both are large enough to need their own images, dependencies, and release cadence. Whichever repository gets created first will set the precedent for every one after it. This writes the convention down so that precedent is chosen deliberately instead of inherited by accident. ## What it covers - **Where code lives** — the core-repo/dedicated-repo split, the rough test for which side something falls on (API keys, external services, third-party accounts), and why this is a set of peer repos rather than a second org. - **Naming** — capability-named for general capabilities (`code-execution-sandbox`), integration-named for specific third-party products, named for the product rather than the vendor behind it. Plus what to avoid: over-broad names, names that clone a vendor's API or brand, and the redundant `-integration` suffix. - **Third-party names** — allowed descriptively, with a non-affiliation note in the repo README, and brand/policy edge cases cleared before the repo exists. - **Upstreaming** — the part with teeth for this repo. Integration repos that accumulate local patches against core bitrot, and the gap they work around stays invisible to everyone else. So: prefer making core behavior configurable with defaults unchanged. #487 and #465 are linked as illustrations of that pattern — this PR does not depend on either, and branches from `main`. - **Two worked examples** that validate the convention rather than just following it, including the third-party-name edge case. ## Review This was announced at the community meeting and circulated as a shared design doc with a 7-day review window, which has now closed. It synthesizes the `#integrations` thread discussion. Comment history: <https://docs.google.com/document/d/1Tb6u0b1XSvWrNpoyD4jdsQaJ58aAgDtQOM18uxujs-8/edit> This PR is the trimmed version: doc-review scaffolding — status block, reviewer list, self-link — is dropped, and only the durable convention is carried over. ## Left open Two questions are deliberately out of scope, called out in the doc rather than answered. Both are maintainer calls and neither blocks the first repositories: - Governance tiers — whether to distinguish "official" from "community" integrations with different review bars, as Home Assistant and Obsidian do. - Who creates integration repositories and grants per-integration maintainer access. ## Also in this PR - README gets an entry in the docs list, matching every other file in `docs/`. - `CONTRIBUTING.md` gets one sentence pointing there, since "where does my integration go?" is a question a contributor asks before opening a PR. Fixes #<issue_number_goes_here> > It's a good idea to open an issue first for discussion. - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR |
||
|
Maya WangandMaya Wang
|
4eedfef98d |
feat: configurable Envoy route timeout for long-running actor requests (#714)
> Split out of #487, which bundled three unrelated changes. ## Summary Envoy's end-to-end timeout on the workload route is hardcoded at `10s` in `buildRoutes`. An actor that legitimately holds a request open longer gets cut off: a harness relaying an LLM completion keeps the request open for the whole generation, and the client sees a **504 mid-turn**. Adds `--route-timeout` on atenet-router, and pairs it with a route-level `idle_timeout` so the ceiling is actually reachable. **The default is 10s, so behavior is unchanged** unless an operator passes the flag. ## Why the route timeout alone was not enough Raised in review by @LiorLieberman and @yan-vlasov, and they were right — the first version of this PR did not do what it claimed. We never set `stream_idle_timeout` on the HTTP connection manager, so Envoy applies its default of **5 minutes**. Per the HCM proto, that default is "overridable by the route-level `idle_timeout`", and when it fires "the stream is terminated with a 408 Request Timeout error code if no upstream response header has been received, otherwise a stream reset occurs." That is exactly this PR's case. A turn relaying a non-streaming completion sends no bytes at all while the actor is thinking, and a request parked across a suspend/resume is idle by the same measure. Both are progressing; Envoy cannot tell. So `--route-timeout=30m` would still have been cut at 5 minutes with a 408 — the knob would have looked like it worked and silently not. `routeIdleTimeout()` therefore resolves the accompanying idle timeout as `max(routeTimeout, 5m)`. Taking the larger keeps the operator's ceiling honest without ever making the idle timer *stricter* than it is today: below 5 minutes the route timeout fires first regardless, so at the 10s default this is a no-op. Route-level rather than HCM-level, so it stays scoped to workload traffic instead of every stream through the router. It is derived rather than exposed as a second `--route-idle-timeout` flag so the two cannot drift apart, with one silently defeating the other — happy to make it explicit if reviewers prefer. For naming: what this PR sets is the route-level `timeout`, which bounds upstream response time. Envoy's HCM `request_timeout` bounds how long the *request* takes to be received, which is not the limit in question here. ## Changes `cmd/atenet/internal/router/` — adds `XdsServer.routeTimeout` with a `SetRouteTimeout` setter and a `defaultRouteTimeout` const, wired from `routerConfig.RouteTimeout` / `--route-timeout`. Same shape as the adjacent `SetExtProcMessageTimeout` and `SetExtProcMaxRequests`, and a flag on the existing config struct rather than an env read, matching the convention the parked-request work established. Wired in `startEnvoyDataplane`. Adds `envoyDefaultStreamIdleTimeout` (5m) and `routeIdleTimeout()`, applied as the route's `IdleTimeout` in `buildRoutes`. A non-positive value leaves the default in place, since Envoy reads a zero route timeout as *no timeout at all*. The knob bounds the actor's own handling time only. The resume that may precede a request is covered by request parking and the ext_proc message timeout, both of which already derive from `--parked-request-budget`. `manifests/ate-install/atenet-router.yaml` documents it as a commented-out entry. ## Verification - `go build ./...`, `go vet ./...`, `go test ./...` — all pass. - `xds_test.go` reads the timeout back out of `buildRoutes`, where Envoy actually picks it up: default, setter override, and non-positive-keeps-default. The helper pins that route to `OriginalDstClusterName` — a change that moved actor traffic onto some other route would otherwise leave the test passing while the timeout governed a route nothing uses. - Two added subtests cover the pairing: `IdleTimeoutTracksLongerRouteTimeout` and `IdleTimeoutKeepsEnvoyDefaultWhenRouteTimeoutIsShorter`. - **On a live GKE cluster**, read back out of Envoy's own `/config_dump`. With the new image and no flag, the workload route reports `timeout: 10s`, so the default is genuinely unchanged. With `--route-timeout=5m` it reports `timeout: 300s`. Same binary, same manifest, only the flag differs. Caveat on that measurement: it was taken before `ingress: route actor ingress through the atunnel mTLS server` landed, so the route it read was the old `dynamic_forward_proxy` path to pod-IP:80. After rebasing, the timeout attaches to the `actor_original_dst` route that replaced it — which is now pinned by the test above rather than left to inspection. The `idle_timeout` pairing has test coverage only, not a live `/config_dump` read. - **Regression, resume with parking on the path:** a conversation actor that had been suspended for 4 days was resumed by an ordinary request through the router — HTTP 200 in 3.74s, exactly one parked request, `parking_wait_duration_seconds{outcome="served"} = 3.459s`, no shed and no `budget_exhausted`. ## Follow-up Per-ActorTemplate (or per-request) configurability, raised by @ronlv10: agreed it needs an API and is follow-up shaped rather than something to fold in here. The global flag remains useful as the cluster-wide ceiling. ## Relationship to #465 This is a stopgap for the connected-socket suspend/restore problem tracked in **#465 (suspend-safe actor networking)**. Once actor network traffic survives checkpoint/restore natively, much of the need to raise this ceiling should go away; this just makes the current behavior tunable in the meantime. Fixes #<issue_number_goes_here> > It's a good idea to open an issue first for discussion. - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR --------- Co-authored-by: Maya Wang <mymaya@google.com> |
||
|
Maya WangandMaya Wang
|
9e3ee7a3d8 |
feat: readyz: make the overall wait timeout configurable per template (#487)
> **Rescoped again.** This PR previously proposed `--golden-snapshot-warmup`, a > tunable wall-clock delay before the golden checkpoint. Per discussion, that > direction is dropped: the answer for a workload that cannot report readiness > is a readiness endpoint — a small sidecar where the workload itself cannot be > changed — not a longer timer. What survives is the piece that discussion > agreed on, and which the previous revision already flagged as a follow-up: > making the readyz deadline itself configurable. > > The warmup work is not in this branch. It is kept locally in case a workload > genuinely cannot be given a readiness signal before GA, and would come back as > its own PR if so. ## Summary `readyz.Wait` polls until the container returns 200 or a **hardcoded 30s** elapses. A workload that legitimately takes longer to bind its HTTP server cannot be accommodated without raising the ceiling for every actor in the cluster, and losing that race fails the actor start. How long a workload takes to become ready is a property of that workload, so this makes the deadline a per-template setting rather than a package constant. Adds optional `timeoutSeconds` to `ContainerReadyz`. **Unset keeps today's 30s**, so no existing template changes behavior. ## Changes The value rides on the existing probe, so it follows the chain the probe already takes and no call site needs to know about it: `ContainerReadyz.timeoutSeconds` → `toAteletReadyz` → `ateletpb.Readyz` → `toAteomReadyz` → `ateompb.Readyz` → `readyz.Wait` - `pkg/api/v1alpha1/actortemplate_types.go` — `TimeoutSeconds *int32`, `+optional`, `Minimum=1`, `Maximum=3600`. - `internal/proto/ateletpb/atelet.proto`, `internal/proto/ateompb/ateom.proto` — `int32 timeout_seconds = 2` on both `Readyz` messages. - `cmd/ateapi/internal/controlapi/workload_spec.go`, `cmd/atelet/main.go` — pass it through the two conversions. - `internal/readyz/readyz.go` — `OverallTimeout` becomes `DefaultOverallTimeout` (still 30s) and `Wait` resolves its deadline through a new `overallTimeout(probe)` helper. - Regenerated: both `.pb.go`, `zz_generated.deepcopy.go`, and the `actortemplates` CRD. None of the four `readyz.WaitAll` call sites change. **On the zero value.** Unlike a warmup delay — where zero is a real request meaning "checkpoint immediately" — a zero readiness deadline could never be met, so it is never something a template author means. A non-positive value on the wire is therefore read as "unset" and falls back to the default, and the CRD field is a pointer with `Minimum=1` so the API rejects `0` outright rather than silently substituting 30s behind the author's back. **On bounding**, which was the open question left on the previous revision: bounded at `3600`. A template asking to wait longer than an hour for readiness is expressing a broken workload, not a slow one, and the bound keeps a typo from pinning a worker for a day. ## Verification - `go build ./...`, `go vet ./...`, `gofmt`, `go test ./...` — all pass. - `internal/readyz/readyz_test.go` — `overallTimeout` resolves unset and negative to the default and honors an explicit value; `Wait` against a port nothing binds gives up at the probe's 1s deadline rather than the 30s default. - `workload_spec_test.go`, `cmd/atelet/main_test.go` — the timeout crosses both conversions, and a probe without one stays zero on the wire. - `actortemplate_validation_test.go` — the bounds are enforced by a real API server. This suite runs under envtest against the generated CRD directory, so it exercises the regenerated `actortemplates` CRD rather than the Go markers: `300` is accepted, unset is accepted, and `0`, `-1` and `3601` are all rejected by apiserver schema validation. - **On a real cluster, via CI.** `internal/e2e/fixtures/probe` now declares a `readyz` probe with `timeoutSeconds: 60`, pointed at the `/healthz` the probe binary already serves on `:80`. The kind e2e that runs on every PR therefore exercises the value crossing ateapi → atelet → ateom on real binaries, across the auth matrix, on both the run and restore paths. This is also the readyz path's first e2e coverage — no fixture declared a probe before. Wire compatibility degrades safely in both skew directions: `timeout_seconds` is a new field 2 on a `Readyz` message that has only ever had field 1, so an old ateom ignores it and an old ateapi leaves it zero, which reads as the 30s default. No GKE run. What that would add over the above is a workload whose readiness genuinely exceeds 30s, and that is the readiness-sidecar work rather than this PR. Fixes #<issue_number_goes_here> > It's a good idea to open an issue first for discussion. - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR --------- Co-authored-by: Maya Wang <mymaya@google.com> |
||
|
Maya WangandMaya Wang
|
18c8639f65 |
docs: add Agent Executor to Ecosystem & Examples section (#24)
Incorporate Agent Executor as a demonstrative example of a distributed agent runtime and harness built on Agent Substrate. ISSUE=None Fixes #<issue_number_goes_here> > It's a good idea to open an issue first for discussion. - [ ] Tests pass - [ ] Appropriate changes to documentation are included in the PR Co-authored-by: Maya Wang <mymaya@google.com> |