v0.2.2 (sync from internal development) - #1
Merged
Merged
Conversation
…nd add regression tests - Updated `buildJSKey` and `buildJSPrefix` functions to honor `ScopeSpec.Sharing` and `spec.Identity`, ensuring correct key layouts for exclusive and shared scopes. - Added comprehensive regression tests to verify cross-agent scope behavior, preventing unintended isolation or leakage in shared scopes.
Lifts retry handling out of every worker into a platform primitive. Adds a proto `RetryPolicy` (max_attempts, backoff, schedule, jitter, retryable status codes, honor_retry_after) carried on `CreateTaskRequest`. Tasks persist the policy; the store's `FailTask` computes `next_retry_at` from it, and the gateway's reschedule path consults it when re-pending. Service principals now register in `implementationIndex` alongside agents (workspace-less `:impl` key). `findWorkerByImplementation` falls back to that key when the workspace-scoped lookup misses, so pool tasks targeted at services (e.g. webhook delivery) find a healthy instance without per-workspace registration. Cross-workspace by design — ACL gates at task creation, not at delivery. Workflow Engine retires its in-DAG immediate-re-dispatch path: WE translates a step's `RetryConfig` into a proto `RetryPolicy` and attaches it to the created task. The task store handles backoff; WE only reacts to terminal status transitions. Changes: - api/proto/aether.proto: BackoffStrategy enum, RetryPolicy message, CreateTaskRequest.retry_policy field (regenerated bindings) - pkg/tasks: RetryPolicy struct, ComputeNextRetryAt, persistence, conformance test for cross-workspace pool claims - internal/gateway: services in implementationIndex, retry_policy proto conversion, rescheduleFn honoring policy - internal/orchestration: RetryPolicy threaded through task_assignment; cross-workspace findWorker fallback - internal/workflow: dag retires broken immediate-re-dispatch; executor translates RetryConfig -> RetryPolicy - sdk/go/aether: CreateTaskOptions.RetryPolicy - migrations/025 (postgres) + sqlite_tasks/003 (sqlite): retry_policy_json column
…locks and improve error management
…ow services to report progress
…es to transition tasks to RUNNING state
…e log spam and improve validation efficiency
…nd exponential backoff
…CompareAndDelete) for distributed coordination
…s to reserved keys in KV store
Adds SetAdd (add member, returning newly-added + cardinality) and SetCard to the KVReadWriter interface, implemented natively in Redis (SADD/SCARD via Lua), Badger (txn read-modify-write), and NATS JetStream (revision-guarded CAS), with a shared JSON set codec for the two software-emulated backends. The unique caller observing added && cardinality==N is the single fan-in firer; added==false marks a duplicate arrival for at-most-once dedup ledgers. Includes Badger and embedded-NATS tests.
Adds the Join type and eight CRUD/sweep store methods (EnsureJoin, GetJoin, UpdateJoinArrived, SetJoinExpected, SetJoinDirty, MarkJoinTerminal, GetDueJoinDeadlines, ListJoins) in lockstep across the engine-internal WorkflowStore mirror, the canonical storage/workflow.Store interface, the legacy and native-sqlite implementations, the type-alias bridge, and both migration trees. The SQL row is the durable mirror, observability surface, and deadline driver for join instances whose authoritative arrival counter lives in KV.
Adds a declarative `type: join` rule destination handled by a new JoinEngine: count-mode barrier (atomic arrival counter plus an exactly-once fire-marker gate), dynamic-N arming via a separate arm_on_event arrival that supplies expected_count without itself counting as a member, coalesce/debounce mode (the server-side kb_update pattern), per-id dedup, and emit_event chaining. Wired into the Router; built on coord.Counter/Locker abstractions so it is unit-tested with in-memory fakes (no gateway). Join keys live under the reserved _sys/coord/ namespace, portable across Redis/Badger/JetStream.
…elds Regenerates Go/Python/TS bindings for: KVOperation SET_ADD/SET_CARD; WorkflowOperation LIST_JOINS/GET_JOIN/CANCEL_JOIN; and CreateTaskRequest/TaskInfo/TaskFilter idempotency_key + correlation_id + root_task_id. All additive (new enum values and optional fields); existing wire semantics unchanged.
Persists on_complete/on_timeout/on_partial_failure on the workflow_joins row so the leader-gated scheduler sweep can fire a join's timeout action with no live event. Adds JoinEngine.HandleDeadline (abort vs proceed/proceed_degraded policy, exactly-once via the fire-marker gate) and wires GetDueJoinDeadlines into the scheduler poll loop. A never-completing barrier is now GC'd at its deadline rather than leaking.
Exposes the atomic set primitive end-to-end: gateway KV handler cases for SET_ADD/SET_CARD (with audit op constants), Go SDK SetAddSync/SetCardSync, and the JoinModeSet path in the engine — members are added to a KV set (inherently deduped) and the join fires when cardinality reaches the expected_set size, with the same fire-marker exactly-once gate. Also backfills SetAdd/SetCard on the gateway test mock.
Joins now stamp a stable idempotency_key (join:name:ws:corr, identical across the on_complete and on_timeout fire paths) on their downstream create_task. The gateway dedups creation on that key via a SetNX ledger under _sys/idem/task/: the first create proceeds and records its task_id; duplicates (retries, engine restart, or a timeout sweep racing a completion) create no second task and echo the original task_id when a response is expected. Fail-open on KV outage.
Adds LIST_JOINS/GET_JOIN/CANCEL_JOIN WorkflowOperation handlers and matching
admin REST endpoints (GET /joins, GET /joins/{name}/{corr}, POST
/joins/{name}/{corr}/cancel), exposing a joinView surface (name, workspace,
correlation_key, mode, arrived, expected, status, deadline) so operators can
list/inspect in-flight barriers and GC a wedged one.
Covers architecture, the event/rule model, all destination kinds, joins in depth (count/coalesce/set, correlation contract, dynamic-N arming, dedup, deadline sweep, exactly-once via fire-marker + idempotent create_task, KV keys, storage, observability), the scheduler, DAG/state-machine engines, admin API, design decisions and rationale, deferred work (feed B, correlation propagation, DAG AND-join), and a proto + source map.
The integration-test and e2e jobs run resource-heavy tests concurrently on GitHub's 2-core runners: embedded NATS clusters (JetStream Raft elections), real sidecar runners, and shared-gateway tunnels. Concurrency starved timing- sensitive setup and produced intermittent flakes (TestAggregator_CNValidation; the TestE2E_Metrics_TunnelOps tunnel PEER_RESET) that pass locally / in isolation. Add -p 1 (one package at a time, so the NATS-cluster and sidecar-tunnel packages no longer overlap) and -parallel 1 (serialize the t.Parallel tunnel tests that share one aetherlite subprocess). Slower jobs, but no cross-test contention.
govulncheck now reports 4 docker/docker advisories reachable via the docker orchestrator (all Fixed in: N/A, docker <= v28.5.2 is the latest module version): the two already-tracked (GO-2026-4887, GO-2026-4883) plus two newer docker cp race conditions — GO-2026-5617 (bind-mount redirection) and GO-2026-5668 (symlink-swap arbitrary empty file). Still no upstream fix to bump to; mitigation unchanged.
The security job only scanned the server module, so the docker/docker cp-race advisories reachable via the SDK's docker orchestrator (SECURITY.md Known Issues) were never surfaced by CI. Add a continue-on-error govulncheck run in sdk/go so those (and any genuinely new SDK vuln) appear in the CI log without turning the build red on the four already-tracked, no-upstream-fix advisories.
TestClusterIntegration_BackupRoundtrip_PerDomain polled 15s for both domains'
.bin uploads then cancelled the coordinator; under CI's 2-core -race load the
second domain (settings_cluster) hadn't finished, so the cancel hit it mid-
upload ('upload manifest: context canceled') and the assertion failed. Widen the
poll deadline to 45s and the overall ctx to 120s. The poll breaks the instant
both domains are present, so a passing run pays nothing.
The cluster/integration suite has ~40 tight (<30s) positive-wait deadlines that blow under CI's `-race` on 2-core runners (embedded-NATS Raft elections/backups/ handshakes run 2-10x slower there) — a different marginal test tipped over each run (aggregator, then backup_roundtrip, ...). Serializing the job (-p 1) helped but the per-test deadlines were still too tight under -race. Add a `scaled(d)` helper that multiplies durations by 4 under the race detector (timescale_race_test.go) and is the identity otherwise (timescale_norace_test.go), and route the package's positive-wait deadlines through it: operation timeouts (context.WithTimeout), poll deadlines (time.Now().Add), and completion waits (time.After in a wait-for-success select). Negative-assertion windows (absence checks) and poll/tick intervals are deliberately left unscaled, as are behavior config values (leader TTLs, MinInterval). Deadlines stay tight for fast local runs; under CI's -race they auto-widen. Verified: gofmt/vet clean, compiles under -race, and phase5 + backup_roundtrip pass both non-race and -race.
README: - CLI Flags table: remove nonexistent --db-password; --config is required-unless---dev (not defaulted to configs/dev.yaml); document --dev - Option C: run commands from server/ (cd server); gateway --lite/bare run fixes so `go run ./cmd/gateway --dev` works without a config file Docs hygiene (server/docs/): - delete proxy-cutover.md (MemoryLayer→Aether migration runbook, not aether reference), proxy-architecture-roadmap.md (Phase-7 planning artifact), and proxy-load-test-results.md (empty benchmark snapshot); clean the dangling Related-documents links in proxy.md / proxy-quickstart.md / proxy-sandbox.md and the two source-comment references Stale content: - lite mode is cmd/aetherlite-only; cmd/gateway --lite now fatals at startup (aetherlite.md, specification.md, horizontal-scaling.md) - principal-type counts: SDK exposes all eight typed clients (specification.md six→eight + add Service/Bridge; workflow-engine.md seven→eight) - aetherlite-clustering.md: drop leaked internal /home/drew/.claude/... path
ListRoleAssignments / ListGroupMembers / ListPrincipalRoles / ListPrincipalGroups scanned granted_at straight into the RoleAssignment/GroupMember time.Time field. SQLite stores timestamps as TEXT (writes go through formatTime), so the driver returns a string and the scan fails: sql: Scan error on column index 3, name "granted_at": unsupported Scan, storing driver.Value type string into type *time.Time — surfacing to the superadmin as "failed to list role assignments". The sibling expires_at was already handled correctly (sql.NullString + assignExpiry), and the working acl_rules read parses granted_at via a string too; the four group/role list reads were the outliers. Add an assignGranted helper mirroring assignExpiry and use it in all four. These read-back paths had NO test coverage (the other tests exercise CheckAccess / ExplainAccess, which read the in-memory enforcer, not the SQL rows), so the bug shipped green. Add TestSQLiteGroupsRoles_ListReadsGrantedAt covering all four list reads + asserting granted_at parses to a non-zero recent time.
… for controlling pool-task targeting
Migrations 003 and 019 seed acl_rules with principal_id
'_any_authenticated'. The enforcer never looks that subject up:
wildcardSubjects() derives the wildcard from the principal TYPE and only
produces _any_authenticated_user / _any_agent / _any_task / _any_service.
No alias exists, and findGlobMatch skips policies containing no glob
character, so those rows match nothing.
This is not cosmetic for 019. Its rows are DEFAULT-DENY (level 0) on the
two cross-agent shared per-user KV scopes — data its own comment calls
privacy-sensitive ("billing, API keys, OAuth tokens") — and 019 exists
because acl_fallback_policies is keyed by (principal_type, resource_type)
and cannot distinguish scope names, so those denies MUST be explicit rows.
With the rows inert, evaluation falls through instead:
evaluateAccessNoAudit -> enforcer nil -> applyFallback(_, "kv_scope")
-> RuleCategory(<type>,"kv_scope") -> 003 seeds *_kv_scope at 20
-> READ_WRITE ALLOWED
So the intended deny is inverted into an allow. 029 restores it, one row
per wildcard subject, since "all authenticated principals" cannot be
expressed as a single row under the current enforcer. The old rows are
deleted so both spellings cannot coexist.
NOT changed: 003's '_global' workspace row (level 20). That is a GRANT,
so re-animating it would WIDEN access — every authenticated principal
would gain READ_WRITE on _global. Left inert deliberately; enabling it is
a separate decision.
029 TIGHTENS access: a caller relying on the accidental allow to read
another agent's user-shared KV will start being denied. That is the
intended policy, but it is a behaviour change and wants a knowing
rollout. 003/019 annotated so the spelling is not copied forward.
TestTenantRelay_TunnelEOFReturnsNil intermittently failed in the -race unit job with "timed out waiting for gateway stream". The relay was not stuck: the CI log carried exactly four "provider session opened (init forwarded verbatim)" lines for the four TestTenantRelay tests, so the failing test's relay had dialed the gateway, opened the stream and forwarded the init. awaitGatewayStream waits on a server-side observation (fakeGateway.Connect being dispatched), but with eofAfterSend the tunnel half-closes at once, so pumpUp sees io.EOF, runPumps returns nil, Run returns and its deferred gatewayClose() calls conn.Close(). Under CPU starvation that teardown beats the gateway server's dispatch of Connect, so nothing is ever pushed to streamCh. This is an ordering bug, not slowness — which is why the earlier 3s -> 15s bump did not help; no timeout is long enough. Gate the half-close on a new fakeAggregator.eofGate that the test closes only after observing the spliced gateway session. Both assertions are preserved (init forwarded verbatim, then EOF -> nil) and the ordering is now deterministic instead of load-dependent. Verified under GOMAXPROCS=1 on two pinned cores with six competing spinners: the original test fails 4/40 with the exact CI message, the gated test passes 40/40. go vet and gofmt clean; proxysidecar and integration_e2e suites pass with -race.
…e generated code Generated protobuf artifacts embed the versions of the tools that produced them, so an unpinned compiler makes any drift check nondeterministic: CI installs a different protoc than a developer has, regenerates, and reports changes in files nobody touched. That is what has been failing proto-check. The repo was producing its outputs with three different compilers, none pinned: Go via a standalone protoc (3.21.12 locally, `27.x` — a floating range — in CI), Python via grpc_tools' own bundled libprotoc (31.1), and TypeScript via proto-loader, with node_modules holding 0.7.15 against a package.json pin of 0.8.1. The committed Go headers said protoc v3.21.12 while the Python gencode said 6.31.1, so the tree was already internally inconsistent. The new `proto:` block pins every one of them in a single place, next to the inputs and outputs they apply to, and `compile-protos` verifies them before generating anything. Aligning on grpcio-tools 1.81.1 (which bundles libprotoc 33.5) sets protoc 33.5 for the Go path, so both languages are now stamped by the same compiler version. Two pins deserve a note. protoc_gen_go is left null and derived from preferred_versions.go["google.golang.org/protobuf"], since protoc-gen-go ships from that module and restating it would create a second source of truth. protoc_gen_go_grpc cannot be derived the same way: it is a separate nested module versioned independently of google.golang.org/grpc, which is why CI's v1.5.1 disagreed with the v1.6.2 that actually produced the committed files. Regenerating moves Python gencode 6.31.1 -> 6.33.5, and the runtime guard in *_pb2.py rejects a protobuf runtime older than the gencode that produced it. The protobuf floor is raised to >=6.33.5 in versions.yaml (propagated to pyproject.toml) and in requirements.txt, which was pinned at >=6.33.2 and would have failed at import. The Python SDK has no opentelemetry dependency, so the long-standing TODO about that constraint does not bind here. Everything else in the diff is upstream-driven and carries no wire-format or semantic change: Go picks up new header comments only; *_pb2_grpc.py gains a GRPC_GENERATED_VERSION bump and `class X(object)` -> `class X`; aether_pb2.pyi corrects bool constructor kwargs to _Optional[bool]; and proto-loader 0.8.1 emits typed MessageTypeDefinition<T, T__Output> in place of the bare form. The three sandbox_relay_tunnel Python modules and six TypeScript files were never committed at all — untracked, and therefore invisible to the `git diff` based proto-check that was supposed to catch exactly that. Verified: regeneration is idempotent (181/181 up to date on re-check), Python imports and instantiates against runtime 6.33.6, `go build` and `go vet` are clean on both modules, `tsc --noEmit` passes, and the suites pass — Go with -short -race, TypeScript 179, Python 547.
…rotos shim The 160-line shell script is superseded by `compile-protos`, which is driven by the `proto:` block added in the previous commit. The replacement is not just a consolidation — it fixes two things the script could not do: - It verifies the toolchain against the pins before generating, so a compiler mismatch fails with the exact install command instead of silently writing artifacts whose embedded version headers disagree with CI. - `--check` compares the *generated* file set, so a generated file that was never committed reports as missing. A `git diff --name-only` check cannot see an untracked file, which is how nine generated files stayed absent from the repo while proto-check passed. The per-file `sed` calls that rewrote bare sibling imports in the generated Python are handled generically by `compile-protos`; that is a universal protobuf-Python problem, not an Aether quirk, so it belongs upstream. Output is byte-identical to what the script produced, which is how the previous commit's Python artifacts were validated. `scripts/compile-protos.py` follows the same shim pattern as `scripts/update-versions.py`: use the installed package if importable, otherwise resolve via uvx, otherwise print install instructions. CLAUDE.md and CONTRIBUTING.md both referenced the deleted script and are updated here rather than in a separate docs commit, so no commit leaves the tree describing a command that does not exist.
Replaces the hand-written workflow with generated output, so the pins in the `proto:` block are the single source of truth for what CI installs. The old version installed protoc `27.x` — a floating range that is not reproducible even between two CI runs — and pinned protoc-gen-go-grpc to v1.5.1 while the committed artifacts were produced by v1.6.2. Its `git diff --name-only` check also could not see untracked files, so generated code missing from a commit passed silently. One job per language rather than one job installing everything: each provisions only its own toolchain (protoc plus the Go plugins, or grpcio-tools, or `npm ci`) and runs `compile-protos --check --lang <lang>`, so the three run in parallel instead of serializing through every ecosystem's installer. `ci.only_workflows` is an allowlist naming just proto-check. The alternative, `skip_workflows`, would mean listing the six workflows this repo does *not* want generated and revisiting that list whenever repo-tools adds a generator. `repo_tools_source` is pinned rather than left to float, for the same reason the compilers are. It is also load-bearing: `only_workflows` did not exist before 0.1.13, and older versions ignore unrecognized `ci:` keys silently — resolving an earlier release would hand the generator every workflow in the repo instead of the one named. Regenerate with `generate-ci-gha --force`; `--check` reports drift.
…ncio deps The client's test suite could not run from a clean `pip install -e ".[dev]"`. Three dependencies were missing from the extra and only satisfied incidentally by a shared developer virtualenv that happened to have them: - `requests` and `httpx` — optional at *runtime*, correctly so: both requests_adapter.py and httpx_transport.py guard the import and raise a clear install hint. But tests/test_requests_adapter.py and tests/test_proxy.py import them unconditionally, so the suite cannot collect or pass without them. - `pytest-asyncio` — not optional at all. pyproject sets `asyncio_mode = "auto"` and roughly 170 tests across eight modules are asyncio-marked. It was listed in requirements.txt but absent from the extra, where a duplicated `pytest==9.0.3` line appears to have displaced it. Found while validating the generated test-python workflow in a throwaway virtualenv rather than the shared one: collection failed on `requests`, then two httpx tests failed. With all three declared, a clean install yields 547 passed. The duplicate pytest entry is removed. This is a prerequisite for running these tests in CI at all — nothing currently does, so the gap was invisible.
Extends `ci.only_workflows` from proto-check to four workflows. Two of these close real gaps rather than just reformatting existing coverage: - Nothing ran the Python or TypeScript test suites on a PR. 547 Python and 179 TypeScript tests existed and passed locally with no CI enforcing them. - The hand-written version-check watched only `versions.yaml`, so editing a version pin in server/go.mod, api/go.mod, sdk/go/aether/version.go or either ag2 file did not trigger it. The generated trigger covers all ten paths `project_rules` references. version-check also switches from `pip install pyyaml uv` plus the scripts/update-versions.py shim to a uvx bootstrap, matching proto-check and removing the hand-maintained transitive dependency list that commit 03a6e7c had to patch. Two `ci.python` settings are needed to make the generated python jobs green: - `install` overrides the generator's `.[test]` default; the extra is named `dev` in both projects. - `lint: none` because ruff currently reports 67 findings across the two Python SDKs (60 in python-client, 7 in ag2). Enabling it today would make every PR red. When those are cleaned up, turn it on and pin `ruff` under preferred_versions.python so the linter cannot float into a red build on an unrelated PR. repo_tools_source stays pinned at 0.1.13: nothing here needs 0.1.14, and the generated files were produced by 0.1.13 so a regeneration at the pinned version is a no-op. Verified before adopting, in clean throwaway virtualenvs rather than the shared one: python-client 547 passed, ag2 108 passed / 7 skipped, and the exact npm commands the workflow runs — `npm ci` (which also confirms package-lock.json is in sync with package.json after the proto-loader bump), `npx --no-install tsc --noEmit`, and `npm test` at 179 passed. All four workflows report in sync. test-go, publish-python and publish-npm remain hand-maintained and are excluded by the allowlist.
…ions.yaml Extends `ci.only_workflows` to seven workflows, retiring build.yml and release.yml. Only test.yml remains hand-written. publish-python now covers both Python projects. The hand-written version had a single job hardcoded to `working-directory: sdk/python-client`, so scitrera-aether-ag2 was version-synced on every release but never actually published. It also gains test gating and a `verify-tag` job that refuses to publish when the pushed tag disagrees with versions.yaml, naming aether-sdk-python because ag2 is versioned independently (0.0.2) and a single tag cannot verify both. release.yml folds into publish-python's github-release job, which attaches the built distributions and generates notes. `github_release` sits at `ci:` level because one tag drives every artifact here, not just the Python ones. build-docker replaces build.yml with identical semantics, verified job by job: dual registry, amd64+arm64 via QEMU, and the gateway -> aetherlite -> aetherlite-dev cascade threading BASE_IMAGE from each parent's freshly built tag. `image_name: aetherlite` on the dev descriptor is load-bearing — without it the descriptor key would publish a new `aetherlite-dev` repository instead of `aetherlite:dev-*`. `ci.docker.test_prereqs` is empty to preserve build.yml's behavior of building on an already-tested tag with no inlined gate. No `dependency_mappings` block, with the reasoning recorded inline. It would order the publish jobs, but `dependencies` cannot be declared without `packages`, and `packages` makes sync-versions rewrite ag2's requirement from `scitrera-aether-client>=0.2.0` to `==0.2.2` — tightening a published package's constraint for downstream users. The ordering buys little: the constraint is a floor, so the previous client release already satisfies it and publishing out of order is harmless. Go module tags are unaffected. scripts/tag-release.sh cuts `v*`, `api/v*` and `sdk/go/v*` per release; only the root tag matches the `v*.*.*` filters, so the submodule tags trigger nothing — which is correct, since Go publishes by tag existence with no CI artifact to build. All seven workflows report in sync and parse as YAML; versions in sync.
…ith pg) lite and full are meant to differ in deployment logistics, not behaviour. They did not: the SQLite ACL tree never carried the equivalent of postgres 019, so the DEFAULT-DENY on the two cross-agent shared per-user KV scopes existed only on postgres. Both trees seed user/agent/task/service_kv_scope at READ_WRITE(20) (postgres 003 == sqlite 001), so with no explicit deny a cross-agent read of another user's shared KV resolved enforcer-no-match -> applyFallback -> ALLOWED. Those scopes hold billing, API keys and OAuth tokens. The new test confirms it was live, not theoretical: with 004 removed, ALL FOUR principal types are allowed on both scopes. An explicit rule is the only way to express this — acl_fallback_policies is keyed by (principal_type, resource_type) and cannot distinguish scope NAMES. One row per wildcard subject, since "all authenticated principals" is not expressible as a single row; note the `_user` suffix on _any_authenticated_user (001 seeds the _global rule WITHOUT it, which matches nothing). Deliberately NOT fixed: 001's `_global` row carries the same inert spelling, but it is a GRANT — re-animating it would WIDEN access. Postgres 029 leaves its counterpart inert for the same reason, so leaving it KEEPS the trees in parity; enabling it is one deliberate decision on both sides. TIGHTENS access on lite tenants. A second test guards the other direction: the deny is limited to the two shared scopes, and global-exclusive / workspace-exclusive still resolve via the fallback. Wider parity audit found no other functional gaps: fallback policies are identical (11 @ 20), authority requests (pg 023 / sqlite 002) and groups/roles (pg 028 / sqlite 003) are parallel, and pg 020's _perm:* namespace migration has no sqlite counterpart because that tree never seeded legacy _perm:* rows.
Go resolves a nested module by its directory, so a module living in `server/` must declare a path ending in `/server`. This one declared `github.com/scitrera/aether`, which made it unfetchable under either name — as the declared path Go looks at the repo root, and as `.../aether/server` the declared path disagrees. It only worked locally because of the `replace` directives. The visible symptom was worse than an unusable import path. The repo root has no go.mod, so proxy.golang.org falls back to synthesizing one, and `github.com/scitrera/aether@v0.2.1` resolves and downloads today serving a go.mod containing exactly one line — `module github.com/scitrera/aether` — with no go directive and no requires. A phantom module that shadowed the real server module for anyone who tried to import it. Nothing outside server/ imports these packages (api and sdk/go are independent modules), so the change is contained: 874 import strings under server/ across 387 files, the module line, one doc reference, and CLAUDE.md. Two things deliberately not renamed: - `github.com/scitrera/aether/gateway` in internal/tracing is an OpenTelemetry instrumentation-scope name, not an import path. Renaming it would change the scope recorded on every span and log record and break existing dashboards. - The bare `vX.Y.Z` tag stays as-is. It is the release signal every workflow triggers on, not a Go module tag; scripts/tag-release.sh now also cuts `server/vX.Y.Z` for the module itself. The rewrite was scoped to the three prefixes that actually belong to this module (internal, pkg, migrations) so that `.../aether/api` and `.../aether/sdk/go` imports were left untouched. A first pass anchored on a trailing slash missed three bare `.../aether/migrations` imports; the build caught it. Verified: go build and go vet clean on server, api and sdk/go still build, and the full server suite passes with -short -race. sync-versions and generate-ci-gha both still report in sync.
Both trees seed ('wildcard', '_any_authenticated', 'workspace', '_global',
20). It has never had any effect: wildcardSubjects() derives the wildcard
subject from the principal TYPE and only ever produces
_any_authenticated_user / _any_agent / _any_task / _any_service, so the
row matches no principal, and the glob pass skips it (no '*' or '?').
Removed rather than repaired. Repairing would GRANT every authenticated
principal READ_WRITE on _global — access nobody holds today. A widening
should be a deliberate decision with its own rationale, not the side
effect of fixing a spelling. A dead row that reads like a grant is also
actively misleading: the superadmin ACL console renders it, so it asserts
something untrue about who can reach _global.
NO-OP FOR ACCESS, and tested as such: _global already resolves through the
seeded *_workspace fallback policies (READ_WRITE 20, seeded alongside the
rule in the same migration), so a real principal's decision is unchanged.
The new test asserts both halves — the row is gone AND user/agent
principals still reach _global.
devinfra stops re-seeding it; otherwise dev fixtures would resurrect the
row the migration deletes and diverge from a real deployment.
Paired pg 030 + sqlite 005 so lite and full stay in lockstep.
FOLLOW-ON: if the *_workspace fallbacks are tightened for production,
_global will need a REAL grant to wildcard subjects that actually resolve.
That was already true before this change, since the deleted row never
contributed anything.
…Fatal `resolveFirstPendingAuditSubmit` is always started with `go`, but reported failures with t.Fatalf/t.Fatal. t.Fatal calls runtime.Goexit, which is only valid on the test's own goroutine — from a helper goroutine it terminates that goroutine without failing the test promptly, so the caller blocks until an unrelated timeout and the real cause is obscured. `go vet` flags this (testinglog: "call to (*testing.T).Fatalf from a non-test goroutine") and it was failing `go vet ./...` for the whole sdk/go module. That went unnoticed because test.yml only vets server/. Switched to t.Error/t.Errorf plus an explicit return, which is goroutine-safe and records the failure. Control flow is unchanged: the helper still stops before resolving the pending request, so a genuinely broken test fails exactly as before — just legibly, and without a vet error. Prerequisite for generating the Go workflows, which run `go vet ./...` across every module rather than only server/.
Eight of the nine workflows are now generated; only test.yml remains
hand-written.
publish-go makes the root `v*.*.*` tag the single release signal. It verifies
that api/, sdk/go/ and server/ each have a matching module tag on the release
commit, which is the whole of "publishing" a Go module — there is no artifact,
only tag existence. `module_tags: verify` for now; flipping to `push` (and
deleting scripts/tag-release.sh) is a one-line change once a real release has
proven the wiring.
server/ is included because the module rename in the previous commit made it
legal: with `module github.com/scitrera/aether` in server/, the generator's
module-path/directory check refused to emit this workflow at all.
Upgrading to repo-tools 0.1.15 also rewrites the publish workflows to gate via
`uses: ./.github/workflows/test-{python,npm}.yml` instead of restating those
matrices inline, and adds a test gate to publish-npm, which previously published
on a tag push with nothing verifying it first.
`ci.go.test_args` is set to `-short -race -count=1`. publish-go inlines the Go
test jobs because test-go is not yet an allowlisted workflow, and the generator's
default omits -short — which would have pointed the unit suite at
RabbitMQ/Valkey/Postgres that only the integration job provides.
Note for the next release: publish-go will fail on any tag that lacks
`server/vX.Y.Z`. tag-release.sh now cuts it alongside the others. Simulated
against the existing v0.2.1 tag, the check correctly reports api/ and sdk/go/ as
present and server/ as missing, since that tag predates the rename.
sdk/go had never been linted — test.yml only ran golangci-lint against server/ — so 51 findings had accumulated. Clearing them is a prerequisite for generating the Go test workflow, which lints every module. staticcheck: - ST1005: eight `Docker client not initialized` error strings lowercased so they read correctly when wrapped. golangci's max-same-issues capped the report at three; all eight are fixed. - SA1019: `client.IsErrNotFound` -> `cerrdefs.IsNotFound` (five call sites, same cap) and `ImageInspectWithRaw` -> `ImageInspect`, whose raw bytes were being discarded anyway. containerd/errdefs was already in the module graph. - S1016: AdminOptions and UserOptions are field-identical, so the conversion replaces the struct literal. This is also safer — a field added to AdminOptions now fails the build here instead of silently not being forwarded. errcheck: the two ignored returns are both best-effort cleanup paths (`stdcopy.StdCopy` into a pipe whose reader surfaces failures, and `RemoveContainer` on an error path where the original error is the one worth returning). Made explicit with `_ =` and a comment rather than handled. Three findings are annotated rather than changed, because the code is deliberate: `TestOboFromContext_NilContext` passes nil because that is the behaviour under test (SA1012); `err.AetherError.Message` spells out the embedding the assertion exists to check (QF1008); and ConfigSnapshot still reads the proto-deprecated kv/global_kv fields so an older gateway that still sends them keeps working (SA1019). Removed three genuinely dead unexported methods: Checkpoint.waitForResponse and KV.waitForResponse, both documented as "legacy queue-based" leftovers from the move to correlation-based responses, and BaseClient.handleProxyHttpResponse, a one-line wrapper nothing called. go build, go vet, gofmt and the test suite are all clean.
…ation.yml Completes the migration: every workflow is now generated except integration.yml. test-go.yml covers all three Go modules with vet, unit tests, golangci-lint and govulncheck — nine jobs where test.yml ran four, and only against server/. api/ and sdk/go were never linted or vulnerability-scanned, which is how sdk/go accumulated the 51 findings cleared in the previous commit. `ci.go.coverage: true` preserves the go-coverage artifact test.yml uploaded, now one per module since the names would otherwise collide. `test_args` keeps `-short` so the unit job does not reach for RabbitMQ/Valkey/Postgres. publish-go drops from four jobs to two: with test-go now a managed workflow it gates via `uses:` instead of inlining copies of those same jobs. integration-test and e2e move to a hand-maintained integration.yml rather than becoming generator config. They are the two jobs that genuinely resist it: integration-test needs three service containers with health checks, non-default host ports and a RabbitMQ Streams advertised-host workaround whose rationale runs to a 26-line comment, and e2e needs its own build tag, package scope and timeout. Expressing that declaratively would mean reimplementing GitHub's `services:` schema for one repo, and would make the RabbitMQ workaround harder to read rather than easier. The file says so at the top so the split is not mistaken for an oversight. Verified: golangci-lint clean on all three modules, go vet clean on all three, sdk/go and server suites pass with -short -race, all ten workflows parse, and generation is idempotent.
test-npm crashed in CI with `FATAL ERROR: v8::ToLocalChecked Empty MaybeLocal` and exit 134 — a V8-level abort, not a test failure. The suite passes locally on Node v24.13.0 but the runner has v24.18.0, and the declared `vitest: ^1.4.0` resolves to 1.6.1, which predates Node 24 entirely. vitest 1.x is roughly two years old and was never supported on this runtime; it happened to work on the older patch release. Upgraded to vitest 3.2.7, which supports Node 24. No config changes were needed — there is no vitest config file and the tests use no 1.x-specific APIs. All 179 tests pass, `tsc --noEmit` is clean, `npm run build` succeeds, and `npm ci` installs cleanly from the regenerated lockfile. Note this was only reachable now because nothing ran the TypeScript suite in CI before test-npm.yml was generated; the incompatibility had been latent. Deliberately not pinning `ci.npm.node_version` to an exact patch. Compiler versions are pinned throughout this repo because they are baked into generated artifacts, but Node is a runtime where patch updates carry security fixes worth taking. The right fix is a toolchain that supports the current runtime rather than a frozen runtime that suits an unmaintained toolchain.
…est runs Picks up repo-tools 0.1.17, which resolves both CI problems the generated workflows surfaced. govulncheck failed for aether-sdk-go on four github.com/docker/docker advisories, all reported "Fixed in: N/A". They are reachable only through sdk/go/orchestrators/docker, the risk was already assessed, and all four are documented in SECURITY.md's Known Issues. They are now waived by id with the reason recorded next to each. The waivers are scoped to aether-sdk-go rather than the repo. aether-api-go and aether-gateway pass govulncheck cleanly and server/ does not import the docker orchestrator at all, so a repo-wide list would have had those two jobs report all four entries as stale on every run — misleading, since the advisories were never reachable there, and corrosive to the warning that stops the list becoming a permanent blanket. This is narrower than the `continue-on-error` the hand-written workflow used: only these four ids are waived, a new advisory still fails the build, and CI warns once an entry stops matching so the waivers can be dropped when docker ships fixes. The test workflows also ran twice for every push to this branch, since a develop -> main PR puts develop in both the push and pull_request trigger lists and github.ref differs between the two events. Their concurrency groups now key on the branch name, so the redundant run is cancelled rather than run to completion. govulncheck is pinned at v1.1.4 instead of installed from @latest. Verified by executing the generated scripts against the four real advisory ids: the sdk-go job passes with all four allow-listed, and the gateway job still fails on the same id, confirming the scoping does not waive globally. Generation is idempotent, versions are in sync, and all ten workflows parse.
…han cancel 0.1.17 collapsed the push and pull_request runs of a single commit into one concurrency group. It removed the redundant work but reported the loser as cancelled, so the PR read "24 successful, 16 cancelled" instead of green, and a cancelled run for a required check can hold up a merge. 0.1.18 reverts that grouping and moves duplicate suppression to `push_branches`, which is left unset here on purpose. develop therefore stays in both trigger lists, every check runs twice for a push to this PR's branch, and both runs complete and report success. The reasoning is recorded next to the setting so it does not read as an oversight: the cost is redundant jobs on a public repo, while the alternative (`push_branches: [ main ]`) would leave pushes to develop unchecked whenever no PR is open. Concurrency is back to `github.ref`, which still cancels a run superseded by a newer push to the same PR — the case where cancelling is the right outcome — without touching the push/pull_request pair.
Replaces the row-based deny from 019/029/004. Net effect is STRICTER: the
old fallback handed out READ_WRITE(20) on every kv_scope lacking a rule.
Two problems with encoding denial as rows at access_level NONE(0):
CONCEPTUAL — NONE is the absence of an access level, not an assertion of
denial. A real explicit-deny sentinel would be a distinct nonzero level
below READ(10). Left for a future change; this just stops leaning on NONE
to mean something it does not.
STRUCTURAL — it could not be carved out. The enforcer returns at the first
matching specificity tier, and a wildcard-principal rule on an exact
resource (tier 2) always outranks an sv::<impl>::* glob grant (tier 5). A
tier-1 exact rule is impossible because service identities embed the pod
name. In production platform-server was denied on kv_scope user-shared
("Wildcard rule: NONE") while HOLDING an explicit grant, silently dropping
per-user WS session state.
Deny-by-default in the fallback expresses the same policy with no tier
pathology: an explicit grant is the only way into any kv_scope, and grants
at every tier resolve because nothing short-circuits ahead of them.
This realizes what 019 already assumed — its comment on the exclusive
scopes reads "Cross-agent access (via OBO) still requires an explicit
grant, which the absence of a fallback enforces correctly", written as
though no kv_scope fallback existed. One did, which is why the shared-scope
deny was needed as a patch at all.
Access retained via explicit acl_seed grants (verified against a live
tenant's seeded set): _any_service @40 and _any_agent @20 on global/
workspace/user/user-workspace, platform-bridge on kv_scope:*, and
platform-server on the two shared scopes. Owners of the *-exclusive scopes
are unaffected — the gateway's owner fast-path short-circuits ALLOW before
the ACL.
Tests pin the regression directly: a glob grant must resolve for a concrete
pod identity, must not leak to the sibling scope, and must not leak to
another service.
…y ctx.Err() TestAggregator_CNValidation failed once in CI with matching CN was rejected: rpc error: code = DeadlineExceeded desc = stream terminated by RST_STREAM with error code: CANCEL while the duplicate run of the same commit passed, so it is timing-dependent rather than a defect in the code under test. aggTunnelHelloOnce decided whether an error was benign by re-reading ctx.Err() *after* Recv returned, rather than by looking at the error it got. The match case is expected to end at the client deadline, and that is supposed to map to nil. But grpc-go tracks the deadline on its own transport timer as well as via the context, so it can return a DeadlineExceeded status in the window before the context package's timer marks the context expired — at which point ctx.Err() is still nil and a benign timeout is reported as a rejection. The error now classifies itself: DeadlineExceeded or Canceled (or io.EOF) means "not rejected", anything else is returned. Honest about the limits of this: the mechanism above is consistent with the observed error but is NOT confirmed. The pre-change test survived 105 runs under deliberate CPU starvation (-cpu=1 with eight busy loops) without once reproducing the failure, so this fixes a real defect in the classification without proving it is the defect that fired in CI. Two things follow from that. The failure message now reports the gRPC status code, so a recurrence distinguishes a genuine server rejection (Unknown, carrying the aggregator's own message) from a transport timeout that slipped past the classification. And the mismatch case now asserts the error actually is the CN-vs-hello rejection instead of accepting any non-nil error, which a dial or TLS failure would also have satisfied while testing nothing. One behaviour change worth noting: previously a real server rejection arriving at the same moment the deadline expired would have been swallowed as nil. It is now returned, so that case fails the test rather than passing silently.
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
1 participant
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.
This pull request introduces a new release (
v0.2.2) with significant feature additions, improvements, and bug fixes across the gateway, SDKs, and supporting infrastructure. The changes also enhance documentation, update CI workflows, and clarify project structure. Below are the most important changes grouped by theme:Major Features and Improvements:
uu::{user_id}) for platform-to-user messaging, with helpers and documentation in the SDKs, and enforced sender-type restrictions. [1] [2] [3]gatewayandaetherlite, gated by the standard environment variable. [1] [2]/auth/verify-optionalendpoint, and the ability for services to opt out of pool-task consumer routing.Bug Fixes:
::separator, resolving silent message drops. [1] [2]Documentation and Project Structure:
CHANGELOG.mdfor the v0.2.2 release, summarizing all new features, changes, and fixes.CLAUDE.mdandREADME.mdto document new binaries, topic schema, observability, cleanup, and configuration flags. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Continuous Integration and Testing:
govulncheckscan for the Go SDK to surface known and new vulnerabilities without failing the build.These changes collectively improve platform robustness, observability, developer experience, and documentation clarity.