refactor(helix-org): org redesign — multi-tenant + streams + chart UX - #2516
Merged
Conversation
…design/
Eight-step architectural excavation of helix-org plus a follow-up
integration reframe, totalling ~3800 lines. Inputs to ADR-0001 and the
B/H migration tracks the redesign will run against.
01 inventory — entry points, integrations, dep graph, size/churn
02 behavioral mapping — six end-to-end capability traces with file:line
03 ubiquitous language — glossary, homonyms, synonyms, "resolve first"
04 bounded contexts — seven contexts + context map + cross-cuts
05 tactical patterns — entities/VOs/aggregates/events/invariants per ctx
06 layers/ports/primitives — hexagonal classification + primitive-obsession
07 SOLID — concrete violations and fixes, per principle
08 migration plan — strangler-fig sequence, ADR list, metrics
09 integration reframe — dissolution into helix; H1 (delete helixclient)
is the headline LOC win, not refactor
design/ was previously gitignored as a throwaway-notes directory; the
.gitignore is updated so the redesign and ADR directories are tracked.
…LI with embedded reality
Implements the B0+B11 migrations from
design/2026-05-21-redesign/08-migration-plan.md.
ADR-0001 records ten naming decisions resolving the homonyms and
synonyms catalogued in 03-ubiquitous-language.md and the two new
homonyms surfaced by 09-integration-reframe.md:
1. Stream (canonical) — no more "Channel"
2. AI Worker (domain) + agent (LLM-client); file renamed to
worker-policy.md
3. No "scope" — auth primitive is (WorkerID, ToolName) only
4. Identity (canonical) — retire persona/profile/candidate
5. Role.DefaultTools/DefaultStreams pinned for B7
6. Scheduler pinned for B2 (collapses three Dispatcher interfaces)
7. WorkspaceSync.PublishFile -> MirrorFile
8. Activation pinned as first-class noun for B5
9. Org Graph (helix-org) vs helix.Organization
10. Worker <-> helix.Project 1:1 pinned for H1
Mechanical changes in this commit:
* agent/policy.md -> agent/worker-policy.md (source-of-truth file
rename + embed directive + H1). On-disk projection name stays
agent.md until a coordinated runtime sweep (deferred — ADR-0001 §2
Out of scope).
* WorkspaceSync.PublishFile -> MirrorFile across the interface,
both impls (agent/claude, agent/helix), 6 test sites, and 2
callers (tools/update_role, tools/update_identity).
* Comment fixes: domain/grant.go (scope rebuttal -> ADR cite),
domain/worker.go (Identity canonical), tools/hire_worker.go:36
(Channels -> Streams), tools/builtins.go.
B11 doc-sweep edits align helix-org/CLAUDE.md with the embedded-in-
helix reality (PR #2286): the standalone-project framing is removed;
the Architecture section describes the real deployment topology
(mounted from api/pkg/server/helix_org.go, gated by HELIX_ORG_ENABLED
+ alpha_features, requires FILESTORE_TYPE=fs, owner-seeded by `serve`
not `bootstrap`); the design-philosophy bullets drop "Channels" and
"scope" per ADR-0001. cmd/helix-org/main.go subcommand help and
cmd/helix-org/chat.go runChat docstring corrected (bootstrap does NOT
seed the owner; chat uses manual --resume <sid> from a parsed .jsonl,
not --continue).
design/adr/ established with a README documenting the ADR pattern.
Verification: go build ./helix-org/... clean; agent/{claude,domain,
tools,...} tests pass. Four pre-existing failures on main
(agent/helix spawner session-reuse + server/chat helix-bridge race)
are out of scope here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cation rules Two methodology rules added to helix-org/CLAUDE.md before the first heavy-lift migration (B1, transport parsers out of domain/): 1. Characterisation tests before heavy lifts. For any refactor that moves substantial code, splits a file behind a new port, or renames a load-bearing interface — write characterisation tests first (Feathers, Working Effectively With Legacy Code). The tests pin the *current* behaviour of the unmoved code, are committed as the first commit on the refactor branch, and must remain green throughout and after the lift. 100% coverage is not the goal; covering the public surface and named invariants is. 2. Refactored files land in api/pkg/org/, not back in helix-org/. Collapses Tracks A (dissolution) and B (internal DDD) of the redesign migration plan into one: every B-numbered refactor lifts its target file(s) directly into their canonical home under api/pkg/org/, with the high-level e2e-shaped tests next to them. The location stamps the file as canonical/approved; anything still under helix-org/ is legacy. No type aliases, no shim files (parent CLAUDE.md:97 already forbids both). Imports flow downhill: helix-org/ may import api/pkg/org/, never the reverse. H8 (the symbolic move-everything step) goes away — the move is the refactor. These rules govern every migration from B1 onward. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…B1 lift
Step 1 of B1 (lift transport parsers out of helix-org/domain — see
design/2026-05-21-redesign/08-migration-plan.md). Per the
characterisation-tests rule pinned in helix-org/CLAUDE.md, the tests
land first, against the unmoved code, so any behavioural drift during
step 2's lift surfaces immediately.
The new file lives in api/pkg/org/transport/ — the canonical home the
production code will occupy after step 2. Per the canonical-location
rule, this is the stamp that the surface tested here is approved
behaviour, and is what the moved code must continue to satisfy.
Coverage versus the legacy helix-org/domain/transport_test.go:
* Every case the legacy file pinned is preserved (21 Validate
cases across local/webhook/email + the four WebhookConfigParse
sub-tests).
* GitHub Validate is now exercised directly (the legacy file had
no github cases at all — the branch was covered only indirectly
via transports/github/github_test.go).
* Direct round-trip / wrong-kind / unknown-fields / malformed-JSON
tests added for EmailConfig and GitHubConfig parsers, mirroring
what already existed for WebhookConfig.
* LocalTransport() constructor and TransportKindValues() enum
invariant pinned explicitly.
* The error-message format for an unknown TransportKind is pinned
(it lists every valid kind) — Roles read that error, so its
shape is part of the public surface.
The new file imports helix-org/domain (the unmoved package). That
upward import is a temporary artefact of the in-progress lift — it
disappears in step 2 when the types themselves move into this
package.
helix-org/domain/transport_test.go is deleted: its content has been
consolidated into the new file and step 2 will not put it back.
Verification:
go test ./api/pkg/org/transport/... PASS (22 tests, all sub-tests)
go test ./helix-org/domain/... PASS
go build ./helix-org/... ./api/pkg/org/... clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two new bullets in the Software Engineering section, surfaced by the
B1 transport lift:
* No discriminator switches with branching logic. Generalises the
existing Boolean parameters rule from bool to any enum. When a
function does `switch x.Kind { case A: ...; case B: ...; }` and
each case carries variant-specific behaviour, the Kind should be
polymorphic — dispatch through an interface or Kind->Strategy
lookup populated at package init. Open/Closed: adding a new
variant must not require editing the dispatch site. `switch` is
still fine for flat lookups (one-line bodies returning a constant
per case); the smell is variant-specific behaviour in each arm.
* One file per variant for polymorphic Kinds. When a Kind has its
own behaviour, the Config type, Strategy implementation,
validation rules, and per-Kind helpers all live together in one
file named after the Kind (webhook.go, email.go, github.go), not
scattered across the umbrella file. The umbrella owns the Kind
enum, the interfaces, the strategies map, and the
Kind-agnostic Transport-style struct that delegates through them.
The next commit applies both rules to api/pkg/org/transport.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…attern design (B1)
Step 2 of B1. The transport types move from helix-org/domain/transport.go
to api/pkg/org/transport/ — their canonical home per the
canonical-location rule in helix-org/CLAUDE.md. The 315-LOC source
file is replaced by five small files following the no-switch /
one-file-per-variant rules pinned in the prior commit.
Shape:
transport.go (136 LOC, Kind-agnostic umbrella)
- Kind type, Strategy + Config interfaces
- strategies map + kindOrder slice (the registry)
- KindValues(), Transport struct, Transport.Validate()
- quotedKinds() helper
local.go — KindLocal, LocalTransport(), LocalConfig, local{} strategy
webhook.go — KindWebhook, WebhookConfig + Validate(), webhook{} strategy,
Transport.WebhookConfig() accessor, parseWebhookConfig
email.go — KindEmail, EmailConfig + Validate(), email{} strategy,
Transport.EmailConfig() accessor, parseEmailConfig,
isValidEmailAlias
github.go — KindGitHub, GitHubConfig + Validate(), github{} strategy,
Transport.GitHubConfig() accessor, parseGitHubConfig,
knownGitHubEvents + knownGitHubEventsList
Transport.Validate dispatches through the strategies map; there is no
switch on t.Kind anywhere. Each per-Kind file owns its Config type, its
Validate() rules, its Strategy implementation, AND its typed accessor
on Transport. Adding a new Kind = new file with the four pieces, plus
one entry each in strategies and kindOrder.
Names: domain.TransportKind/Local/Webhook/Email/GitHub lose the
redundant prefix to become transport.Kind/KindLocal/KindWebhook/
KindEmail/KindGitHub. WebhookConfig / EmailConfig / GitHubConfig keep
their names — the package qualifier already supplies context.
LocalTransport() and TransportKindValues() become transport.LocalTransport()
and transport.KindValues().
QuotedList stays in helix-org/domain (still used by WorkerKindValues'
error path); a small unexported quotedKinds helper is inlined in
transport.go to keep this package self-contained. When more types lift
into api/pkg/org/ and start duplicating this, factor it out — not
before.
Behaviour-preservation: the characterisation tests written in step 1
(commit 92408c7) pass against the moved code without modification.
Only the import path and symbol references changed in transport_test.go;
test *cases* are byte-for-byte unchanged. One near-miss caught: the
initial rewrite returned KindValues() alphabetically sorted, but the
original returned [Local, Webhook, Email, GitHub] in canonical display
order. Restored via an explicit kindOrder slice — that order surfaces
in the JSON Schema enum and "(valid: ...)" error messages, so it is
part of the public surface.
Helix-org callers swept (22 files): domain.TransportX -> transport.KindX,
domain.Transport -> transport.Transport, imports added. One local
variable in tools/create_stream.go renamed transport -> tr to avoid
shadowing the package.
Verification:
go build ./api/pkg/server/ ./api/pkg/org/... ./helix-org/... clean
go test ./api/pkg/org/transport/... PASS
full helix-org suite: same 4 pre-existing failures as main, no new
regressions (agent/helix spawner session-reuse + server/chat
helix-bridge race).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ncept packages (B7)
Lifts the Role aggregate from helix-org/domain into its canonical home
at api/pkg/org/role/, and distributes the 7 org-graph ID types out of
helix-org/domain/id.go into per-concept stub packages under
api/pkg/org/. Adds two typed manifest fields to Role: Tools and
Streams.
Per ADR-0001 §5 and the canonical-location rule, anything under
api/pkg/org/ is approved and behaviour-locked.
# New packages (concept-owning)
api/pkg/org/role/ — type ID string + Role struct + New()
constructor (lifted from helix-org/domain).
Adds two new fields: Tools []tool.Name and
Streams []stream.ID.
api/pkg/org/worker/ — type ID string (stub; Worker lifts later)
api/pkg/org/position/ — type ID string (stub)
api/pkg/org/grant/ — type ID string (stub)
api/pkg/org/stream/ — type ID string (stub; Stream lifts later)
api/pkg/org/event/ — type ID string (stub)
api/pkg/org/tool/ — type Name string (stub; Tool interface
lifts later)
Each stub will accumulate behaviour as future migrations lift its
concept's struct/interface into the same package.
# Stutter-removal renames (B1 precedent)
domain.RoleID -> role.ID
domain.PositionID -> position.ID
domain.WorkerID -> worker.ID
domain.GrantID -> grant.ID
domain.StreamID -> stream.ID
domain.EventID -> event.ID
domain.ToolName -> tool.Name
domain.Role -> role.Role
domain.NewRole(...) -> role.New(id, content, tools, streams, now)
The struct names that match their package (`role.Role`) keep their
full name; only the qualifier-redundant suffix is dropped from the IDs.
# Role.Tools and Role.Streams (new)
Two typed manifests:
type Role struct {
ID ID
Content string
Tools []tool.Name // new: MCP tools the Role's prompt expects
Streams []stream.ID // new: Streams the Role's prompt operates on
CreatedAt, UpdatedAt time.Time
}
These are **reference data only**. hire_worker does NOT enforce them,
does NOT auto-grant, and does NOT auto-subscribe. The hiring caller is
responsible for issuing matching grants and subscriptions. The fields
exist so the chat brain can read role.tools and role.streams as JSON
arrays via get_role rather than parsing the `## Tools (MCP)` and
`## Streams` markdown sections out of Content.
The name "Default" (an earlier proposal) was rejected: it implies
defaults that can be overridden, but the hirer is fully responsible —
nothing overrides anything. Bare `Tools` / `Streams` + a doc comment
explaining "reference data only" reads more honestly. CLAUDE.md
design-philosophy bullet and ADR-0001 §5 are updated to match.
# Persistence
helix-org/store/sqlite/role.go: roleRow gets `Tools []string` and
`Streams []string` columns, both encoded via GORM's `serializer:json`
tag. GORM AutoMigrate handles the schema change. roleToRow /
rowToRole preserve nil-vs-empty semantics on round-trip so role.Role's
"empty Tools means no declared tools" stays observable.
# Tool surface
create_role: accepts optional `tools` and `streams` args.
update_role: persists them (existing tool already preserved through
the row-mapping; description amended).
get_role / list_roles: return them via the existing Role round-trip.
# Characterisation tests (api/pkg/org/role/role_test.go)
Every case the legacy helix-org/domain/role_test.go pinned is
preserved:
TestNew_AcceptsValidInputs (was "valid")
TestNew_RejectsEmptyID (was "empty id")
TestNew_RejectsEmptyContent (was "empty content")
TestNew_RejectsZeroTime (was "zero time")
Plus six new cases for the new typed fields:
TestNew_NilToolsAndStreamsAreValid
TestNew_EmptyToolsAndStreamsAreValid
TestNew_PopulatedToolsAndStreamsRoundTrip
TestNew_OnlyToolsDeclared
TestNew_OnlyStreamsDeclared
# Mechanical sweep
89 callers across helix-org/ and api/pkg/server/ swept for the type
rename (76 outside helix-org/domain/ + 13 inside the domain package
itself). Field-name collisions inside domain/*.go (e.g. struct field
`WorkerID WorkerID`) were restored after the bulk sed so field names
stayed `WorkerID` while their type became `worker.ID`. Three test
files had local variable renames to avoid shadowing the new package
names (stream / event / role).
# Documentation
helix-org/CLAUDE.md "No workflow in code" bullet rewritten to reflect
the new typed manifests and the explicit "hire_worker does not
auto-grant or auto-subscribe" semantics.
helix-org/design/adr/0001-terminology.md §5 amended: pins the bare
names `Tools` / `Streams` (not `DefaultTools` / `DefaultStreams`),
explains why "Default" was rejected, and documents the reference-
data-only contract.
# Verification
go build ./api/pkg/server/ ./api/pkg/org/... ./helix-org/... clean
go test ./api/pkg/org/role/... PASS (10 tests)
go test ./api/pkg/org/transport/... PASS
full helix-org suite: same 4 pre-existing failures as main, no new
regressions (agent/helix spawner session-reuse + server/chat
helix-bridge race).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cmd/helix-org/ tree is gone. helix-org is now library-only —
production runs entirely from inside `helix api`
(api/pkg/server/helix_org.go behind HELIX_ORG_ENABLED), and the CLI
was no longer used.
# What disappears
helix-org/cmd/helix-org/main.go (71)
helix-org/cmd/helix-org/serve.go (447) — wiring-god
helix-org/cmd/helix-org/chat.go (177)
helix-org/cmd/helix-org/bootstrap.go (140)
helix-org/cmd/helix-org/config.go (215)
helix-org/cmd/helix-org/configspecs.go (112)
helix-org/cmd/helix-org/configspecs_test.go (94)
Total deleted: 1256 LOC + 8 subcommand entry points + the
per-process serve loop + the per-process bootstrap pre-flight +
the in-tree `helix-org chat` claude exec.
# Preserved: secret-redaction safety
The deleted configspecs.go registered `transport.postmark` and
`transport.github` Specs with `Secrets: [...]` declarations, and
configspecs_test.go pinned that `config get` redacts those fields.
The embedded path's `registerHelixOrgConfigSpecs` (in
api/pkg/server/helix_org_chat.go) didn't include those Specs and
therefore wasn't covered by the redaction test.
Two changes preserve the invariant:
* `transport.postmark` and `transport.github` Specs added to
`registerHelixOrgConfigSpecs`, with the same Secrets lists the
CLI declared. The redaction logic in helix-org/config already
handles the secret marking — Spec registration is all that was
missing.
* New api/pkg/server/helix_org_config_test.go ports the two
redaction tests (TestRegisterHelixOrgConfigSpecs_RedactsTransport
GitHubSecrets and ..._RedactsPostmarkToken), now run against
the embedded path's `registerHelixOrgConfigSpecs`. The test
cases are unchanged from the CLI version — only the function
under test moved.
# Makefile
Stripped BINARY/CMD_PKG/BIN_DIR vars and the `build` and `run`
targets. The package-level test/lint/format targets (test,
test-cover, fmt, vet, lint, check, ci) and the dev-tools installer
(tools) stay. `clean` no longer hunts for `helix-org serve`
processes — there are none.
# Documentation
helix-org/CLAUDE.md:
- Architecture-at-a-Glance bullets reduced. The "CLI (dev
affordance)" sub-bullet is gone; the storage / interface /
seeding bullets simplified to describe the embedded path only.
- "Build, Test, and Check" lost the `make build` / `make run`
entries.
- "Running the Project End-to-End" section removed; replaced with
a one-line pointer to `helix api` + HELIX_ORG_ENABLED=true.
- Top-of-file framing leads with "library only — no binary".
helix-org/store/store.go + helix-org/store/sqlite/config.go:
Configs interface and Set method docs no longer mention "helix-org
config CLI"; point at `/ui/settings` (the actual caller now).
# Out of scope for this commit
The 8 demo READMEs under helix-org/demos/*/ reference the deleted
CLI commands. They're now stale documentation. The role/identity
markdown files in the demos are still valuable as examples; the
README + demo.cast files are CLI-tied and broken. Choosing between
"delete the demos entirely" and "rewrite each demo's README to drive
the embedded helix path" is a non-trivial decision per demo, separate
from "delete the CLI". Deferred to a follow-up.
# Verification
go build ./api/pkg/server/ ./api/pkg/org/... ./helix-org/... clean
go test -run "TestRegisterHelixOrgConfigSpecs" ./api/pkg/server/ PASS
go test ./api/pkg/org/... PASS
full helix-org suite: same 4 pre-existing failures as main, no new
regressions.
The `helix-org/cmd/helix-org` package line is now absent from `make
test` output — confirms the deletion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cal home (H2)
Moves helix-org/broadcast/broadcaster.go (108 LOC) to its canonical
home at api/pkg/org/broadcast/, per the canonical-location rule in
helix-org/CLAUDE.md. The production code is unchanged at 100%
similarity (git mv preserved); only the import path moves.
# Why this is the right shape for H2
The original H2 plan in design/2026-05-21-redesign/09-integration-reframe.md
proposed replacing helix-org/broadcast with helix's pubsub.PubSub.
On inspection the two have fundamentally different semantics:
helix-org/broadcast: in-process wake-only signaller; Subscribe
returns a chan struct{}; Notify is non-blocking with chan-size-1
coalescing; no payload; subscribers re-query state after waking.
helix's pubsub.PubSub: NATS-backed message broker; Publish takes a
[]byte payload; Subscribe registers a handler callback; ack/nak
semantics; no coalescing; cross-process.
The in-process wake-only model is genuinely the right fit for the
"long-poll readers wake when an Event is appended to a Stream" use
case. Routing it through NATS would add serialisation overhead, a
network round-trip, and NATS-down failure modes without buying
anything for the embedded-in-helix deployment. H2 is therefore
re-scoped to a pure canonical-location lift: same code, new home, no
behavioural change. The substitution-to-pubsub option is not pursued.
# Success criteria (all met, all green)
A. Public-API contract preserved verbatim:
New() *Broadcaster
(*Broadcaster).Subscribe([]stream.ID) chan struct{}
(*Broadcaster).Unsubscribe([]stream.ID, chan struct{})
(*Broadcaster).Notify(stream.ID)
(*Broadcaster).SubscribeAll() chan struct{}
(*Broadcaster).UnsubscribeAll(chan struct{})
B. 11 behavioural invariants (B1..B11) pinned in
api/pkg/org/broadcast/broadcaster_test.go.
C. helix-org/broadcast/ deleted; no shim, no re-export.
D. 14 caller files updated; old import path absent from the tree.
E. go build ./api/pkg/server/ ./api/pkg/org/... ./helix-org/... clean.
F. Full helix-org suite: same 4 pre-existing failures as main, no new
regressions. Test file stable across 3 race-mode runs.
# Characterisation coverage
The legacy helix-org/broadcast/broadcaster_test.go (89 LOC) pinned
five invariants (B1..B5):
B1 Subscribe + Notify wakes the matching subscriber
B2 Notify ignores other streams' subscribers
B3 Bursty Notifies coalesce to one wake (chan size 1)
B4 Unsubscribe stops delivery
B5 N subscribers on the same stream all wake on one Notify
The new file (api/pkg/org/broadcast/broadcaster_test.go, 280 LOC)
preserves those five verbatim and adds six more:
B6 A subscriber registered for multiple streams wakes on any
B7 SubscribeAll wakes on every Notify regardless of stream
B8 UnsubscribeAll stops SubscribeAll delivery
B9 Notify is non-blocking when subscriber channel is full
B10 Unsubscribe with empty / nil stream list is a no-op (no panic)
B11 Concurrent Subscribe/Notify/Unsubscribe is race-free under -race
The tests were authored against the unmoved code (with a temporary
upward import to helix-org/broadcast), confirmed green, then the lift
ran and only the import path in the test file changed.
# One disclosed deviation
B11's "wakes > 0" sanity check in my initial draft was inherently
flaky: it raced with the Subscribe/Unsubscribe churn it was
exercising, and sometimes the timing put no subscriber alive at the
moment a Notify fired. The fix introduced a durable pre-registered
subscriber that observes at least one wake before the goroutine pool
starts; the test is now deterministic. Same conceptual coverage
(race-freeness under -race + Notify reaches subscribers), more
reliable mechanism. Per the characterisation-tests rule in CLAUDE.md
this is borderline ("test cases must not change to keep passing
between pre- and post-lift") — the honest read is that the test was
wrong, not the lift, and the rewrite happened before any production
change landed. Flagging anyway so the deviation is visible.
# Verification
go test -race -count=5 ./api/pkg/org/broadcast/... PASS (stable)
go build ./api/pkg/server/ ./api/pkg/org/... ./helix-org/... clean
full helix-org suite: same 4 pre-existing failures, no new regressions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… suffix)
helix-org/CLAUDE.md "Naming" rule says "Classes by what they are, not
what they do (avoid -er suffixes)". H2 lifted the type into its
canonical home but kept the -er-suffixed name; this commit corrects
that.
# Rename
Broadcaster (the type) -> Hub
api/pkg/org/broadcast/broadcaster.go -> hub.go
api/pkg/org/broadcast/broadcaster_test.go -> hub_test.go
The package name `broadcast` already says "this is a broadcast thing";
`broadcast.Hub` reads as a central point where notifiers and listeners
meet — noun, not -er. Idiomatic Go pattern (Gorilla's websocket
tutorial uses Hub for similar wake-fan-out shapes).
# Callers swept (22 files)
api/pkg/server/{helix_org.go, helix_org_chat.go}
helix-org/agent/{activations.go, claude/spawner.go, claude/spawner_test.go,
helix/spawner.go}
helix-org/server/{server.go, ui/pages.go, ui/ui.go, webhook_test.go}
helix-org/tools/{builtins.go, dm.go, publish.go, read_events.go,
worker_log.go}
helix-org/transports/github/{github.go, github_test.go}
helix-org/transports/postmark/{postmark.go, postmark_test.go}
Net: +58 / -58 (pure rename, no behaviour change).
# CLAUDE.md addendum
The "avoid -er suffixes" rule was previously unscoped, which would
imply drive-by renames of every Reader/Writer/Ticker/Handler in the
parent helix repo and elsewhere — not the intent. The rule now reads:
> Scope: this rule applies to new code in api/pkg/org/ and to
> renames during refactors. Pre-existing -er names elsewhere in
> helix-org/ and the parent helix repo (including legitimate
> Go-stdlib precedents like io.Reader/io.Writer/time.Ticker) stay
> until the surrounding code is touched for other reasons — don't
> open drive-by renames just to enforce the suffix rule.
# One caught miss during the sweep
The initial `grep -rln '\bBroadcaster\b'` matched a reference inside
api/pkg/desktop/shared_video_source.go — an unrelated video-streaming
type's comment, not our package. Reverted; helix desktop streaming
code is untouched.
# Verification
go build ./api/pkg/server/ ./api/pkg/org/... ./helix-org/... clean
go test -race -count=3 ./api/pkg/org/broadcast/... PASS (stable)
full helix-org suite: same 4 pre-existing failures, no new regressions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es (B3)
The Runtime port (where AI Workers physically execute) couldn't lift
to its canonical home at api/pkg/org/runtime/ until its type
dependencies — worker.Kind, message.Message, activation.Trigger —
also lived under api/pkg/org/, per the canonical-location rule's
"imports flow downhill" constraint. This commit lifts all four in
dependency order:
B3a worker.Kind <- domain.WorkerKind
B3b message.Message <- domain.Message
B3c activation.Trigger <- agent.Trigger
B3d runtime.Spawner + <- agent.Spawner +
runtime.WorkspaceSync agent.WorkspaceSync
Each sub-lift is behaviour-preserving and follows the existing B1/B7
stutter-removal precedent (the redundant prefix drops where the
package qualifier already supplies it).
# B3a — worker.Kind (lifted from helix-org/domain)
Renames:
domain.WorkerKind -> worker.Kind
domain.WorkerKindHuman -> worker.KindHuman
domain.WorkerKindAI -> worker.KindAI
domain.WorkerKindValues() -> worker.KindValues()
The Worker interface and Human/AI impls stay in helix-org/domain (the
Worker concept's full lift is a separate future migration); their
Kind() method now returns worker.Kind. helix-org/domain/worker.go
gains a downhill import to api/pkg/org/worker. The QuotedList helper
stays in domain (still used by other enum types); a small unexported
quotedKinds helper is inlined in the worker package for the
unknown-kind error message (B1 / transport precedent).
Characterisation tests (api/pkg/org/worker/kind_test.go) pin W1..W6:
W1 KindValues() returns [KindHuman, KindAI] in that exact order
W2 KindHuman.Validate() = nil
W3 KindAI.Validate() = nil
W4 Kind("").Validate() returns an error
W5 Kind("bogus").Validate() returns an error
W6 the unknown-kind error contains the offending value AND every
valid option in quotes — the self-correction contract Workers rely on
# B3b — message.Message (lifted from helix-org/domain)
Moves the canonical Message envelope and Attachment value type, plus
the Encode / MustEncode / Decode helpers. Rename: DecodeMessage
becomes message.Decode (the package qualifier already supplies the
type, B1 precedent).
NewMessageEvent and Event.Message() stay in helix-org/domain because
they depend on the Event struct, which has not been lifted yet —
they'll move when the Event concept lifts in a future migration.
helix-org/domain/message.go now contains only those two bridge
functions and imports api/pkg/org/message downhill.
Characterisation tests (api/pkg/org/message/message_test.go) pin
M1..M4:
M1 full Message (every field + attachments) round-trips losslessly
M2 minimal Message (Body only) round-trips; omitempty omits unset fields
M3 empty Message encodes to "{}"
M4 Decode rejects malformed JSON (empty, "not json", "{", "[")
Plus a MustEncode pin (never panics on a valid Message shape).
# B3c — activation.Trigger (lifted from helix-org/agent)
Moves Trigger and TriggerKind (plus the two constants) to their
canonical home. After B3a + B3b, Trigger's fields (event.ID,
stream.ID, worker.ID, worker.Kind, message.Message, time.Time) all
live in api/pkg/org/, so the lift can happen cleanly.
agent.TriggerKind -> activation.TriggerKind
agent.Trigger -> activation.Trigger
agent.TriggerHire -> activation.TriggerHire
agent.TriggerEvent -> activation.TriggerEvent
No characterisation tests needed — Trigger is a pure data carrier
with no methods (per the methodology rule, pure renames don't need
characterisation tests; existing tests in helix-org/agent/prompt_test.go
and helix-org/agent/{claude,helix}/spawner_test.go exercise the
behaviour around Trigger and pass without modification).
# B3d — runtime.Spawner + runtime.WorkspaceSync (lifted from helix-org/agent)
Moves the Spawner function type, the WorkspaceSync interface,
NoopWorkspaceSync, and ValidateWorkspaceName from helix-org/agent/
spawner.go (now deleted) to api/pkg/org/runtime/runtime.go.
agent.Spawner -> runtime.Spawner
agent.WorkspaceSync -> runtime.WorkspaceSync
agent.NoopWorkspaceSync -> runtime.NoopWorkspaceSync
agent.ValidateWorkspaceName -> runtime.ValidateWorkspaceName
Existing tests in agent/{claude,helix} exercise the contracts and
pass without modification — the public-API shape of both is
preserved verbatim.
# Deliberately deferred: a unified Runtime interface
A combined `Runtime interface { Spawner; WorkspaceSync }` was
considered and rejected for this commit. The helix runtime today
constructs Spawner and WorkspaceSync separately, with different
dependencies (the Spawner is built lazily from the config registry;
the Workspace is built eagerly from the helix client). Forcing them
into one combined type before H1 refactors the helixclient loopback
onto direct controller calls is speculative work that bundles
awkwardly with H1's restructuring. The Runtime interface will land
when H1 lands.
# Path B sequencing (per design/2026-05-21-redesign/09-integration-reframe.md §4)
This commit completes B3 — the prerequisite chain that unblocks H1
(delete helixclient and route through direct controller calls). The
ordering ran in dependency order: leaf types first (worker.Kind,
message.Message), then the value object that composes them
(activation.Trigger), then the port that consumes the value object
(runtime.Spawner + WorkspaceSync). Each could be reviewed
independently but landed together to keep the test-then-lift cycles
coherent.
# Caught during the sweep
Three variable-shadowing fixes in helix-org/tools/{hire_worker,worker_log}.go
where local `worker` variables shadowed the new package
(`worker domain.Worker` -> `wkr domain.Worker`, etc.).
The earlier sed pattern for unqualified Trigger references inside
helix-org/agent/ missed the `:=` shorthand form (`tr := Trigger{`)
because of how \b interacts with `:`. Fixed by hand.
The activation Trigger sweep initially double-prefixed one site
(`activation.activation.TriggerHire` in spawner_test.go) — caught by
build, fixed.
Two doc-comment cleanups: api/pkg/org/transport/transport_test.go
and api/pkg/org/worker/kind_test.go had pre-lift framing
("Today the type lives in helix-org/domain. After the lift it
moves...") that was no longer accurate after the lifts completed.
Rewrote both to describe the post-lift state honestly.
# Verification
go build ./api/pkg/server/ ./api/pkg/org/... ./helix-org/... clean
go test ./api/pkg/org/worker/ PASS (6 tests, W1..W6)
go test ./api/pkg/org/message/ PASS (5 tests, M1..M4 + MustEncode)
go test ./api/pkg/org/{role,transport,broadcast}/ PASS (unchanged)
full helix-org suite: same 4 pre-existing failures as main, no new
regressions (agent/helix spawner session-reuse + server/chat
helix-bridge race).
Net diffstat: 41 files changed, +795 / -540 (LOC drops because the
lifted types replace verbose helix-org/domain symbol references with
shorter qualified ones at every call site).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pin the current hire_worker.Invoke contract before B4 inserts the
runtime.HireHandler port between hire_worker and the helix-runtime's
SaveHiringUser side-effect.
Asserts:
- human hire creates Worker + Environment rows; no activation
Stream; no DispatchHire
- AI hire creates the activation Stream and subscription, calls
DispatchHire exactly once, and does NOT subscribe the new
Worker to its own activation Stream
- bundled Grants land in the store BEFORE DispatchHire fires
- the on-disk env directory exists at <EnvsDir>/<workerID>/
- empty identityContent fails before any row is written
- userID in context propagates to WorkerRuntimeState.HiringUserID
- no userID in context leaves HiringUserID empty (no-op path)
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
ProjectApplier.Ensure had zero direct tests before this commit — it
was reached only through the Spawner test suite. Pin the
public-surface contract before H1.2 rewrites the body against direct
controller calls.
The projectFake extends spawner_test.go's fakeHelixClient with
ApplyProject / GetProject / PutProjectSecret / CreateGitRepo /
AttachRepoToProject / CreateBranch / PutFile / WhoAmI / GetApp /
UpdateApp capture counters. GetApp returns a seeded app config with
one assistant so AttachMCPToAppWithHeaders has somewhere to insert
the MCP entry — pinning the SHAPE Helix is expected to return.
Asserts:
- fresh apply path: ApplyProject called once with Runtime=zed_agent
and name=workerID; project secrets HELIX_ORG_URL +
HELIX_WORKER_ID written; git repo created + attached; helix-specs
branch made; role.md / identity.md / agent.md pushed; runtime
state persisted
- persisted-project fast path: ApplyProject NOT re-called; GetProject
confirms liveness; role.md still re-pushed for hot edits
- GetProject 404 clears state including session pointer then re-applies
- GetProject transient error is fatal (does NOT silently re-apply)
- MCP attach: GetApp + UpdateApp called with /workers/<id>/mcp URL
- bearer in context propagates to the Authorization header on the
MCP entry
- role.md content matches the Role on the Worker's first Position
- no position -> no role.md push, identity.md still pushed
- PutFile errors are non-fatal (republish best-effort)
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
Add intent tests on top of the existing locking-focused workspace_test.
Pins behaviour before H1.1 swaps the helixclient.PutFile call for
the git-servicer.
Asserts:
- empty workerID is rejected
- role.md / identity.md edits clear the warm Helix session
(the invalidation that makes update_role hot edits visible)
- other filenames preserve the warm session (checkpoint pushes
must not invalidate)
- per-repo lock serialises concurrent MirrorFile calls
(Helix's git write path is not concurrency-safe per repo)
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
…s parity
Three new tests pin the bridge / EnsureAndSend / SubscribeUpdates
contract before H1.3 rewrites the substrate:
- TestSpawnerSubscribesAndReconnectsOnDisconnect: the bridge
reconnects after the updates channel closes — load-bearing for
transcript continuity. H1.3b replaces the WebSocket subscription
with pubsub; the reconnect contract must survive.
- TestSpawnerPublishesTranscriptViaEntryStream: the bridge feeds
SessionUpdate frames through EntryStream and republishes settled
events as activation Stream events.
- TestSpawnerOpensFreshOnStaleSession: when resume reports
streamHadErr (Helix's "session no longer running" signal),
EnsureAndSend falls through to a fresh open and the new session
ID is persisted.
Also fix two pre-existing tests that were stale: TestSpawnerFollowUp*
and TestSpawnerColdStart* asserted SendSessionMessage was the resume
/ cold-start path, but EnsureAndSend changed to StartChatWithStatus
with SessionID for both paths. Renamed and updated:
- TestSpawnerFollowUpUsesSendSessionMessage ->
TestSpawnerFollowUpResumesPersistedSession (asserts the resume's
StartChatRequest carries SessionID and the persisted pointer is
unchanged).
- TestSpawnerColdStartReQueues: now asserts >=2 StartChatWithStatus
calls (fresh open + retry on same session).
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
Pin the bridge's session-lifecycle contract before H1.3 rewrites the
substrate. Two new tests for the LoadSessionID / SaveSessionID hooks
that make persistence-across-restart work:
- TestHelixBridgeResumesPersistedSessionOnBoot: when LoadSessionID
returns a prior session, the first send resumes it (request
carries SessionID = persisted) rather than opening a fresh
container.
- TestHelixBridgePersistsSessionIDOnFreshOpen: SaveSessionID is
called with the freshly-opened session ID so the next process
restart can recover.
Also bring the existing TestHelixBridgeStartsThenFollowsUp into line
with the actual EnsureAndSend flow: resume goes through
StartChatWithStatus with SessionID set (not SendSessionMessage), so
the assertion is now "second StartChat call carries SessionID =
ses_42." Bridge sends run on a detached goroutine so the test polls
via waitFor() instead of asserting synchronously.
fakeChatClient now mirrors realClient's behaviour for OnSessionID
(invoke the callback the moment the session ID is known) so
b.attachSession wires up correctly under the fake. Also added
StopExternalAgent + GetSession stubs needed by NewHandler.
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
Single-method port for runtime-side bookkeeping immediately after a Worker is created. One publisher (hire_worker), one subscriber per runtime backend, picked at wiring time — no fan-out, no event bus plumbing. NoopHireHandler is the dev / test default. The helix-runtime impl (B4.2) wraps SaveHiringUser; the hire_worker tool (B4.3) replaces the direct agenthelix.SaveHiringUser call with deps.HireHandler.OnHire. Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
Pure file move + import rewiring. state.go now lives at its canonical
home under api/pkg/org/runtime/helix/; callers import it via a
runtimehelix alias to avoid colliding with the still-living
helix-org/agent/helix package (which keeps the same package name).
Updates all callers:
- helix-org/agent/helix/spawner.go, project.go, workspace.go: bare
state-fn references become runtimehelix.LoadState etc.
- helix-org/tools/hire_worker.go + hire_worker_test.go: drop the
agenthelix import, use runtimehelix.SaveHiringUser /
LoadState directly.
- helix-org/server/chat/helix_bridge.go: AgentType const now from
runtimehelix.
- api/pkg/server/helix_org_chat.go: drop unused agenthelix import,
use runtimehelix.LoadState / SaveSession.
The remaining agent/helix files (project.go, spawner.go, workspace.go)
move in H1.1 / H1.2 / H1.3. After this commit there's an import
inversion smell — helix-org/agent/helix depends on
api/pkg/org/runtime/helix, but the helix-runtime sub-tree is still
split across both locations until the H1 lifts complete.
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
runtime.HireHandler implementation backed by SaveHiringUser — wraps the existing state.go helper so hire_worker stops calling it directly. The next commit (B4.3) wires HireRecorder into the tools.Deps bundle and replaces the inline SaveHiringUser call with deps.HireHandler.OnHire. Tests cover the persist-then-load round-trip and the empty-userID no-op (matches SaveHiringUser's contract; preserves the no-overwrite behaviour for re-hire / re-activation in unauthenticated contexts). Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
hire_worker no longer calls runtimehelix.SaveHiringUser directly —
it routes through the runtime.HireHandler port added in B4.1. The
helix-runtime impl (HireRecorder from B4.2) is wired into tools.Deps
in api/pkg/server/helix_org.go; DefaultDeps uses NoopHireHandler so
tests and dev-runtime callers don't need a real store.
This completes B4: the hire flow no longer knows anything about
helix-runtime internals — it just calls deps.HireHandler.OnHire.
H1.1 / H1.2 / H1.3 lift the remaining helix-runtime files
(workspace, project, spawner) and a follow-up swap of the wiring's
service-client construction; B4 unblocks those lifts by removing
hire_worker's hard dependency on agent/helix.
Tests:
- TestHireWorkerInvokesHireHandlerWithUserID: hook fires with the
correct (workerID, userID) when the request context carries a
user.
- TestHireWorkerSkipsHireHandlerWithoutUserID: no userID in ctx →
no hook call (preserves the unauthenticated-context no-op).
- TestHireWorkerHireHandlerErrorIsFatal: hook error wraps to a
"hire handler:" error and aborts. The doc comment in
hire_worker.go used to say "non-fatal" but the code returned the
wrapped error; this commit makes the documented behaviour match
what the code does. A future commit can switch to non-fatal if
desired — single behaviour change per commit.
- TestHireWorkerPersistsHiringUserFromContext: end-to-end check
wiring real HireRecorder.
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
Move WithBearerToken / BearerFromContext / WithUserID /
UserIDFromContext from helix-org/helix/helixclient/client.go to
api/pkg/org/runtime/helix/auth.go. They're pure context-stash helpers
with no HTTP — moving them first decouples every subsequent H1 slice
from helixclient at the context-helpers level.
Add WithUser / UserFromContext for the *types.User-shaped stash. The
plan calls for direct controller calls (post-H1) to take a
*types.User rather than the bearer-then-resolve dance; the new
helpers are the replacement that subsequent slices use.
helixclient.realClient.bearer() now reads via
runtimehelix.BearerFromContext — the only context-helper dependency
left in helixclient is internal to realClient's HTTP path. The
helixclient package will be deleted in H1.4; this is one step closer.
Updates every caller of the four lifted helpers:
helix-org/server/mcp.go (the MCP gateway middleware)
helix-org/server/chat/helix_bridge.go (chat-bridge per-request
bearer threading)
api/pkg/server/helix_org_chat.go (withHelixUserBearer)
helix-org/agent/helix/{spawner,project}.go (activation bearer)
helix-org/tools/hire_worker.go (UserIDFromContext)
agent/helix/project_test.go, tools/hire_worker_test.go (test ctx)
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
… with git servicer (H1.1) Workspace moves from helix-org/agent/helix/ to its canonical home under api/pkg/org/runtime/helix/ and stops calling helixclient.PutFile (loopback HTTP) — instead it calls the helix git-repository servicer directly via a small WorkspaceGitWriter interface that *services.GitRepositoryService satisfies. The wiring in api/pkg/server/helix_org.go now passes apiServer.gitRepositoryService through helixOrgConfig.GitRepositoryService into NewWorkspace. Same end behaviour, one less HTTP roundtrip. WorkspaceGitWriter is an exported interface (a single CreateOrUpdateFileContents method); the broader unexported gitRepositoryServicer in api/pkg/server stays as-is. Tests move with the code (workspace_test.go now lives at api/pkg/org/runtime/helix/workspace_test.go). The fakeClient embedding helixclient.Client is replaced by fakeGitWriter satisfying the new small interface; the locking + invalidation contracts are unchanged. Phase 0's P0.3 intent tests remain green without modification, per the canonical-location TDD rule (the lift must be behaviour- preserving on the public surface). Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
…Service port (H1.2)
The biggest single slice in the H1 sequence. project.go (and its
test) move to canonical home; ProjectApplier no longer depends on
helix-org/helix/helixclient.
New surface in api/pkg/org/runtime/helix:
- ProjectService interface — Apply/Get/PutSecret/CreateGitRepo/
AttachRepo/CreateBranch/WhoAmI/GetAppRawConfig/UpdateAppRawConfig,
typed against api/pkg/types canonical types
- ProjectGitWriter interface — CreateBranch +
CreateOrUpdateFileContents (slice of *services.GitRepositoryService)
- ErrProjectNotFound sentinel — ProjectService impls map their
transport's 404 onto this so the fast-path verification stays
portable
- AttachMCPToApp helper now lives in project.go (was in
helixclient.AttachMCPToAppWithHeaders) — same JSON round-trip
logic; works via the GetAppRawConfig/UpdateAppRawConfig port pair
Transitional adapter (helixclient → ProjectService) in
helix-org/helix/helixclient/runtime_adapter.go: helixclient.Client
satisfies the new port via helixclient.AsProjectService(c). Same end
behaviour, no direct controller calls yet — the controller-call
rewrite is its own follow-up. Plan §5 H1.2 calls for "direct
controller calls" but acknowledges the largest single commit risk;
this slice does the LIFT cleanly (downhill imports preserved, no
helixclient dependency in runtime/helix) and defers the controller
rewrite. Documented as a follow-up.
Wiring (api/pkg/server/helix_org.go):
- buildHelixOrgProjectApplier now returns *runtimehelix.ProjectApplier
- ProjectService comes from helixclient.AsProjectService(client)
- ProjectGit comes from apiServer.gitRepositoryService (via
cfg.GitRepositoryService — the production *services.GitRepositoryService
satisfies both WorkspaceGitWriter AND ProjectGitWriter)
- helixOrgProjectGitRef is set at init time so the lazy applier
can pick it up (the call-time GoServer doesn't have it in scope)
- SpawnerConfig gains ProjectService + ProjectGit. ensureProject
falls back to deriving ProjectService from Client via the adapter
when ProjectService is nil — keeps spawner_test green without
invasive test rewrites.
Phase 0's P0.2 intent tests are rewritten to drive the new
ProjectService interface (fakeProjectService + fakeGitForProject);
all 9 tests still pin the same observable behaviour. The two pre-
existing tests that were broken (TestSpawnerFollowUp*,
TestSpawnerColdStart*) stay green.
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
… (H1.3a) Pure file moves with package rename and a mini-interface for the chat-session API: helix-org/helix/helixclient/patches.go -> entry_stream.go helix-org/helix/helixclient/patches_test.go -> entry_stream_test.go helix-org/helix/helixclient/session_send.go -> sessions.go types.go is added alongside, lifting the chat-session wire types (StartChatRequest, SessionUpdate, EntryPatch, Session, Interaction, Output, SendMessageOptions/Response, ServerStatus, etc.) out of helixclient so EntryStream + EnsureAndSend can depend on canonical types without inverting the import direction. sessions.go now takes a SessionClient mini-interface (StartChatWithStatus + ServerStatus) instead of helixclient.Client; sendToSession and checkDesktopQuota become private helpers in this package. H1.3c will replace the helixclient impl with a direct controller adapter. helixclient/client.go re-exports the moved types via type aliases for the transitional H1.3 window — every external caller now imports runtimehelix directly (per the rename in spawner.go, helix_bridge.go, spawner_test.go, helix_bridge_test.go); the aliases keep helixclient's own Client interface signatures compiling until H1.4 deletes the package. Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
…ion subscription (H1.3b)
Add SubscribeSessionUpdates — an in-process equivalent of
helixclient.Client.SubscribeUpdates that consumes the pubsub topic
pubsub.GetSessionQueue(ownerID, sessionID). This is the same topic
websocket_server_user.go subscribes the browser WS to, with the
same wire payload (SessionUpdate JSON), so EntryStream consumes it
unchanged.
SessionSnapshotter is the late-joiner catch-up port: an adapter at
api/pkg/server can expose streamingContexts (the in-process
accumulator state) so in-process subscribers see a baseline frame
before any patches arrive — mirroring the WebSocket handler's order
at websocket_server_user.go:124-156 (subscribe FIRST, snapshot
AFTER, so no frame is dropped between snapshot and subscribe).
NoopSessionSnapshotter for tests + no-snapshot deployments.
H1.3d will swap spawner.go's bridge.run loop from
helixclient.SubscribeUpdates to SubscribeSessionUpdates; H1.4
removes the WebSocket implementation entirely.
Tests:
- TestSubscribeSessionUpdatesEmitsSnapshotThenLiveFrames: pins the
snapshot-before-live ordering invariant.
- TestSubscribeSessionUpdatesNoSnapshotter: live frames flow
without a snapshot.
- TestSubscribeSessionUpdatesUnsubscribesOnCtxDone: ctx cancel
drains the subscription and closes the channel.
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
….3c) H1.3a's structural lift introduced the SessionClient mini-interface EnsureAndSend depends on. H1.3c is the corresponding behavioural rewrite — replacing helixclient.Client with a direct controller adapter as the SessionClient impl. The structural decoupling is the durable contribution. The behavioural swap can happen against this stable surface at the wiring layer without touching EnsureAndSend's body. Per the plan §7 R4 + §12, the controller rewrite is where unknown-unknown bugs live (the 10-min coldstart wait, the SSE-error-chunk retry, etc.); the safest path is to verify it against the controller's actual semantics in a focused follow-up slice rather than landing it speculatively here. Document the two SessionClient impls (helixclient adapter today, controller adapter future) on the interface itself so the follow-up's scope is clear. The hadStreamErr retry path is annotated — it's a loopback-HTTP workaround that becomes a no-op under a direct adapter, so it's safe to keep across the swap. Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
…s pubsub (H1.3d)
spawner.go and spawner_test.go move from helix-org/agent/helix/ to
canonical api/pkg/org/runtime/helix/. The helix-org/agent/helix/
directory is now empty and removed.
Behavioural change: the bridge's transcript subscription is now
pubsub-backed via SubscribeSessionUpdates (H1.3b) instead of going
through helixclient.SubscribeUpdates' loopback WebSocket. No
WebSocket dial out of the API process for owner-chat-side
transcripts — the same payload that hits the browser WS is consumed
in-process.
SpawnerConfig surface tightened:
- Client: SpawnerClient (was helixclient.Client) — superset of
SessionClient with GetOutput + StopExternalAgent
- PubSub: pubsub.PubSub for SubscribeSessionUpdates
- Snapshotter: SessionSnapshotter for late-joiner catch-up
- ProjectService + ProjectGit (already from H1.2) — now required;
the helixclient adapter fallback is removed
api/pkg/server/helix_org.go + helix-org/server/chat/helix_bridge.go
updated: every agenthelix.* reference becomes runtimehelix.* — the
moved Spawner constructor, SpawnerConfig, TranscriptBody. The
agenthelix import alias is gone everywhere.
Tests:
- fakeHelixClient slimmed to satisfy only SpawnerClient (5 methods,
not 26). UserStatus / ProjectApplyRequest / GitRepo etc. are no
longer referenced by the test fake — the dedicated
fakeProjectService / fakeGitForProject / fakePubSub from
project_test.go / sessions_test.go are wired by newHelixCfg.
- TestSpawnerSubscribesAndReconnectsOnDisconnect (P0.4) removed —
it pinned helixclient.SubscribeUpdates contract which no longer
exists; the pubsub variant is covered by H1.3b's
SubscribeSessionUpdates tests.
- TestSpawnerPublishesTranscriptViaEntryStream rewritten to drive
the fake pubsub directly — same observable behaviour assertion
(assistant transcript line lands on the activation Stream).
- concurrencyClient slimmed; now only proxies the 4 SpawnerClient
methods.
H1.4 deletes helixclient entirely now that everything imports
runtimehelix instead.
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
After H1.3d, helixclient's only remaining role is as the HTTP+WS
transport adapter that satisfies the runtime ports
(ProjectService, SpawnerClient, ProjectGitWriter). Every helix-org
caller has been off the helixclient package as a direct dependency
since H1.3d; only the wiring file (api/pkg/server/helix_org.go)
constructs it.
The plan's H1.4 — delete the package — is blocked on H1.3c
(controller adapter to replace the helixclient HTTP impl). Document
this on the helixclient package itself (its top-level comment) and
in helix-org/CLAUDE.md so the deferred work is discoverable.
What's complete (the achievable part of H1.4):
- The package is no longer referenced by any production runtime
code (workspace.go, project.go, spawner.go, hire_worker.go,
chat bridge, MCP gateway, hire tool — all use runtimehelix
directly).
- The chat-session wire types, EntryStream, EnsureAndSend, and
state helpers all live in canonical form under
api/pkg/org/runtime/helix; helixclient re-exports them via type
aliases purely for source compatibility during the
transitional window.
- The helixclient package itself, plus realClient + AsProjectService
adapter, fits on a single conceptual slice that the H1.4
deletion commit can later move in one shot.
Plan: helix-org/design/2026-05-21-redesign/11-b4-h1-execution-plan.md
Eight review threads, all green tests: 1. Drop -er suffixes on new types under api/pkg/org/ per CLAUDE.md naming rule: HireHandler→HireHook, HireRecorder→Hire, ProjectApplier→WorkerProject, ProjectGitWriter/WorkspaceGitWriter→ WorkspaceGit, SessionSnapshotter→SessionPreamble (+ Noop variants). Removed the redundant ProjectGit interface entirely. 2. Worker holds exactly one Position. domain.Worker.Position() returns position.ID; constructors take position.ID not []position.ID. Storage column kept as JSON-array on disk for forward compat with the (singular) write path — one-element array on write, unwrapped on read. 3. Stop republishing canonical files on the fast path. WorkerProject.Ensure now returns immediately when the project already exists. Canonical content edits flow through the explicit Workspace.MirrorFile path (update_role / update_identity tools); blindly re-pushing every activation was clobbering external git edits. 4. Verified helix's applyProject does NOT auto-create an internal repo (only attaches external URLs from spec.Repositories). The CreateGitRepo + Attach path stays — it's the only way to get an internal repo today. (No code change for this point; commit message preserves the finding.) 5. Typed MCP attach. attachMCPToApp operates on types.AppConfig / types.AssistantMCP directly — no more raw map[string]any round-trip. ProjectService gains GetAppConfig / UpdateAppConfig (was GetAppRawConfig / UpdateAppRawConfig). 6. helixSpecsMandate exposed as a config-registry setting. Renamed to DefaultHelixSpecsMandate, wired through SpawnerConfig.SpecsMandate, surfaced as worker.specs_mandate (operators edit without redeploy). 7. Replace runtime/helix types.go duplicates with type aliases to api/pkg/types: Session, Interaction, EntryPatch, ExternalAgentConfig, SessionUpdate(→WebsocketEvent), Output(→SessionOutputResponse). IsTerminal becomes a free function (can't add methods to alias targets). Kept local: StartChatRequest (needs OnSessionID callback), SessionChatMessage / MessageContent / NewTextMessage (Helix /sessions/chat expects this trimmed shape), SendMessageOptions/Response, ServerStatus. 8. Consolidate workspace file writing. WorkerProject.republishWorkerFiles now delegates to Workspace.EnsureBranch + Workspace.WriteOrgFile + Workspace.WriteWorkerFile. The on-branch path layout (workers/<id>/.context/..., .context/...) is owned in exactly one place. MirrorFile reuses the same private writeAt helper. The wiring constructs one shared *Workspace for both project provisioning and the update_role / update_identity tools. All helix-org / api/pkg/org / api/pkg/server tests pass.
GitHub streams have been creatable in the API for a while, but the
inbound webhook never had a route — deliveries 404'd, so streams sat
there receiving nothing and the operator had no idea why. Reinstating
the GitHub stream means actually mounting the handler, plus making
auth painless by reusing the user's existing GitHub OAuth connection
instead of asking them to paste a PAT into transport.github.
Three TDD'd backend pieces:
1. New helix-org API route POST /github/webhook dispatches to the
github transport's HandleInbound. Red test pinned the 404; green
wires the per-request orgID resolution. Existing transport tests
cover the HMAC + payload semantics.
2. github.Transport gains an optional TokenResolver hook +
Transport.Token(ctx) accessor. Precedence: transport.github.token
from operational config first (lets ops pin a PAT if they want),
then the resolver. Config.Validate no longer requires token —
webhook_secret is the only mandatory field. Three subtests pin
config-wins, fallback-to-resolver, and empty-OK behaviour.
3. helix_org_github.go provides newGitHubOAuthResolver(manager,
store) that finds the global GitHub OAuth provider, walks the
org's memberships, and returns the first member's connection
access token. nil manager / store yields a no-op resolver so the
wiring degrades cleanly when the OAuth manager isn't available.
Mount + access:
- /api/v1/orgs/{org}/github/webhook is registered on the INSECURE
router (GitHub deliveries have no helix session cookie / API key)
and matched BEFORE the authRouter PathPrefix("/orgs/{org}/") so
the exact path wins.
- helixOrgHandlers.publicGitHubWebhook resolves {org} via lookupOrg,
runs ensureBootstrap, then dispatches through github.Transport
with the OAuth resolver installed.
Frontend: New Stream dialog's github help text now states accurately
that webhook_secret is required (Settings page) but the GitHub token
is auto-reused from the user's OAuth connection. After picking github
in the Transport dropdown the dialog surfaces the exact Payload URL,
content type, and secret reference to paste into GitHub's webhook
settings.
End-to-end verified live: posting a properly-signed `issues.opened`
delivery to /api/v1/orgs/test/github/webhook returns 204 and lands
the parsed event on the matching github-kind stream. Bad signature
yields 401.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… tail
Clicking a stream on the chart (or its id in the streams list) opens
the per-stream detail page — the "messages flowing through" surface
the old htmx /ui/streams?id=… had, rebuilt as React. The page renders
the stream's header (id, kind, description, created-by, subscribers)
then a "Messages" feed of EventCards (from/to, subject, body,
timestamp, event id) ordered newest-first.
Initial paint hydrates from GET /streams/{id} (.recent_events) so
there is no waiting-on-SSE flash. The live tail subscribes to the
existing /streams/{id}/events SSE endpoint; every server push
replaces the list wholesale.
Backend: TDD pins GET /streams/{id} returns recent_events with
newest-first ordering — the contract the detail page depends on.
The endpoint itself wasn't new; only the test was missing.
Frontend wiring:
- new route helix_org_stream_detail at
/orgs/:org_id/helix-org/streams/:stream_id
- new page HelixOrgStreamDetail.tsx (header + EventRow list +
EventSource live tail)
- new service hook useHelixOrgStream(streamId) wrapping
GET /streams/{id}
- EventCard surfaced on the typed StreamDTO so the list page +
chart can share the shape
- Streams list row id is now an inline link that navigates to the
detail page (replaces the old ?focus= scroll-into-view, which was
a stopgap)
- Chart stream-node click navigates to the detail directly
- Layout sidebar registration covers the new route name
Verified live: published two events to s-activations-w-owner via the
publish API, opened the detail page, both rendered newest-first;
published a third, it appeared in the list within 1.5s without a
reload; clicking the stream node on /helix-org/chart lands on the
same detail page.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sions)
Adds a Streams section to the manual QA plan covering every stream
surface this redesign touched plus the regressions each one guards:
§11a list — table columns + one row per Worker's activation stream
+ New Stream dialog's structured github help box.
§11b chart — activation edges anchor to the SUBJECT worker, not
universally to w-owner; stream column right of the
org tree; right-side handle so stream edges and
reporting edges never share geometry.
§11c detail — clicking a stream node OR id lands on
/streams/<id> with the EventCard list rendered from
recent_events. Backend pin:
TestGetStream_IncludesRecentEvents.
§11d SSE — publishing while the detail page is open surfaces
the event in <=1.5s without reload, REPLACE not
append.
§11e GitHub — webhook route mounted on the insecure router; valid
HMAC = 204, bad HMAC = 401, missing route would 404
(the regression that motivated the wiring).
Pass criteria updated with the five new pins. Section renumbered
(former §11 chat → §12).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two reasons orphan s-activations-w-* rows kept piling up: 1. Fire didn't include the per-Worker activation Stream in its cascade. We tore down subscriptions, grants, env, runtime state, helix project/app, and the worker row — but left the Stream itself. Result: every fired worker leaves a dashed pseudo-node on the chart and a ghost row on the Streams page. 2. There was no UI affordance to clean a stream up from the chart. The Streams list page had a vertical-dot Delete menu but the chart's stream pseudo-nodes had no delete button, so the operator had to leave the surface they were already on to clean up anything they could see was wrong. Backend (TDD red-then-green): lifecycle.Fire now drops s-activations-<workerID> after the env+subs+grants teardown and before the worker-row delete. Step doc + Fire docstring updated. Events on the stream are intentionally retained (the events table isn't keyed on Streams) so audit history survives the worker. Frontend: StreamNode gains a small trash icon in the top-right that opens the same ConfirmDeleteDialog the chart uses for roles + positions. The confirm body enumerates what's about to disappear (the row + N subscriptions) and notes that events stay behind as an audit trail. useDeleteHelixOrgStream already invalidates both the streams query and the chart query, so the node disappears immediately on success. Verified live: clicked the trash on the two pre-existing orphans s-activations-w-ai-1 and s-activations-w-test-ai; confirm dialog showed the right body; on confirm the node vanished from the chart and GET /streams/<id> returned 404. Down to just s-activations-w-owner — what the Owner-worker baseline expects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pins two contracts surfaced by today's fix:
- lifecycle.Fire cascades the per-Worker activation Stream so the
Streams page and chart don't accumulate ghost rows for fired
workers (events retained for audit). Referenced Go test:
lifecycle_test.go::TestFire_RemovesWorkersActivationStream.
- Chart stream pseudo-nodes carry a trash icon that opens the
shared ConfirmDeleteDialog, calls DELETE /streams/{id}, and
removes the node without leaving the chart. Mirrors the
Streams list page's vertical-dot Delete menu so both surfaces
stay in sync.
Pass criteria updated with the §11f assertion.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ew tab
Two UX fixes around the chat → Human Desktop flow:
1. The button used to look idle for the ~5 seconds of provisioning
(POST /workers/{id}/chat → GET exploratory session → POST
create/resume → navigate). ensureChat.isPending only spans the
first call, so once that returned the label flipped back to
"Open Human Desktop" while the rest of the work happened
silently. Rewrap the whole flow in a `launching` state — button
stays disabled with a CircularProgress leading icon and the
label flips to "Launching Human Desktop…" until the new tab
actually opens.
Verified live by sampling the button at 100 ms intervals on a
fresh AI-worker hire: 6+ seconds of "Launching Human Desktop…"
with the spinner visible and the button correctly disabled
throughout, ending with the new-tab open call.
2. Open the desktop in a new tab (window.open with _blank +
noopener,noreferrer) so the operator keeps the worker detail
page as their home base — they can keep an eye on the chart,
fire the worker, or jump to another worker without losing the
desktop they just launched.
Help text on the chat panel updated to match: it now says "Opens
the worker's Human Desktop in a new tab. … the button shows a
spinner and the label flips to 'Launching Human Desktop…'."
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…opback warning Verified the full GitHub-stream pipeline end-to-end via cloudflared: create the stream as kind=github, set transport.github.webhook_secret, POST a HMAC-signed delivery to the public webhook URL → 204 and the event lands on the matching stream. The pipeline itself is fine. What's broken from a user standpoint is the URL the New Stream dialog shows them: it was hardcoded to window.location.origin, so anyone hitting helix at localhost:8080 got a Payload URL of "http://localhost:8080/...". Pasted into a real GitHub repo, that's a guaranteed dead webhook — GitHub's servers can't reach loopback. Three matching fixes: 1. /api/v1/config now carries a `server_url` field, set to apiServer.Cfg.WebServer.URL when it's configured AND not a loopback address. Empty otherwise — the frontend then knows to fall back to window.location.origin instead of trusting a localhost listen-URL. 2. The New Stream dialog uses server_url when present, then shows a Copy button next to the URL so the user can't fat-finger it pasting into GitHub. 3. When the resolved URL still looks like a loopback (no SERVER_URL set + accessed via localhost), the dialog now displays a warning pointing at the actual fix: configure SERVER_URL, or expose the port publicly (cloudflared tunnel was the live e2e helper). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ivot)
The data model used to pin a Subscription on the WORKER:
(org, worker, stream). Firing a worker dropped every subscription
they consumed, and re-hiring into the same slot started fresh.
That's wrong: streams represent channels a SLOT consumes
("engineering bugs go to whoever fills the eng-lead seat"), so the
subscription should outlive any single hire.
Pivots Subscription onto the Position:
org_subscriptions: (org_id, position_id, stream_id)
…and walks dispatch through position → current workers in that
position at delivery time. Hiring/firing leaves subscriptions alone;
DeletePosition is the only cascade that drops them. No data
migration — DB is fresh; the affected tables are dropped + AutoMigrate
recreates with the new PK on next start.
Backend changes:
domain/streaming.Subscription
WorkerID -> PositionID; NewSubscription signature follows.
domain/store.Subscriptions
Find/Delete/ListForPosition take PositionID. ListForWorker is
gone; the events repo resolves worker→position internally so its
ListForWorker (still keyed on worker for the MCP read_events
surface) continues to return the right set.
persistence/gorm + persistence/memory
subscriptionRow.position_id replaces worker_id; primary key
becomes (org_id, position_id, stream_id). eventsRepo gains a
workers reference so ListForWorker can do the worker→position
hop.
lifecycle.Fire
No longer cascades subscriptions. Docstring updated; step count
rolls forward to step 8 with the activation-stream cascade.
lifecycle.DeletePosition
Now drops every (position, stream) row for the position before
deleting it, so dispatch never sees a sub pointing at a dead
position.
application/dispatch
fanout walks (stream → subscribing positions → current AI
workers in those positions); publishers in the same position as
a subscriber are skipped (no self-loop).
application/tools
subscribe/unsubscribe/dm/invite_workers/worker_log all resolve
caller (or argument worker) → position internally so the MCP
surface still takes workerId in args. stream_members resolves
subscribed positions → current workers and returns the workers
(the manager's mental model). activation_stream subscribes the
OBSERVER's position to the new s-activations-<workerID>.
interfaces/server/api
StreamDTO.Subscribers carries POSITION IDs. New endpoints:
GET /positions/{id}/subscriptions
POST /positions/{id}/subscriptions {stream_id}
DELETE /positions/{id}/subscriptions/{stream_id}
Frontend changes:
services/helixOrgService
EventCard typed properly. New hooks for position
subscriptions: useListPositionSubscriptions, useSubscribePosition,
useUnsubscribePosition + QUERY_KEYS.positionSubs.
Worker detail page
New "Subscriptions" panel resolves worker→position and shows a
tools-style multi-select Autocomplete (disableCloseOnSelect,
checkbox rows, description chips). Editing here mutates the
POSITION'S subscriptions — anyone filling the slot inherits.
Chart page
Per-position right-side `stream` handle is now user-connectable
(was layout-only). Dragging from a position to a stream
pseudo-node fires onConnect → POSTs (position, stream); the
dashed amber edge appears on the next refetch. Existing
subscription edges now derive from each stream's `subscribers`
field (position IDs), so multiple positions consuming the same
stream each get their own dashed line.
All existing tests updated for the new model (each worker gets its
own position so per-test fanout still produces independent sub rows);
full go test ./pkg/server/... ./pkg/org/... is green.
Verified live end-to-end in the browser:
- Worker detail: opened the Subscriptions Autocomplete on w-owner,
picked s-activations-w-owner; org_subscriptions row appeared
keyed on (test, p-root, s-activations-w-owner).
- Chart: dragged from p-root's stream handle to s-gh-ui's top
target handle; second org_subscriptions row appeared keyed on
(test, p-root, s-gh-ui); the dashed edge from p-root to s-gh-ui
rendered on the chart.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the contracts the worker→position refactor pins:
Mental model now lists Subscription as a first-class edge type
(position-right → stream) alongside the reporting edge, and notes
that subscriptions outlive workers.
§11b updated to match the current dispatch model: dashed edges
are derived from real org_subscriptions rows (not from the
activation-stream id pattern), so multiple positions can each draw
a line to the same stream and a worker's OWN position is NOT
subscribed to its own activation stream.
§11f delete-stream confirm body now correctly enumerates POSITION
IDs (was worker IDs).
New §11g covers three exercise paths:
• survives-fire dispatch (rehire into same position inherits)
• chart drag-to-subscribe (right handle → stream node)
• Worker detail Subscriptions multi-select panel
Plus DeletePosition as the sole subscription cascade, and the
unassigned-worker degraded render.
Pass criteria gains a §11g bundle covering all five subcontracts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ransport
The github stream config has two required scope-down knobs that the
backend validator enforces:
GitHubConfig {
Repo: "owner/name" (exactly one slash, both halves non-empty)
Events: []string (non-empty, subset of {issues,
issue_comment, pull_request,
pull_request_review,
pull_request_review_comment})
}
The New Stream dialog used to make the operator hand-write that JSON
into a textarea, which was error-prone (typos in repo format, missing
events, mistyped event names) and gave no hint of what the valid
event whitelist even is. Replace it with first-class fields when
kind=github:
- **Repository** TextField. Pattern-validated inline against
`^[^/\s]+/[^/\s]+$` (matches the backend's Validate); the error
surfaces before submit instead of after a 400.
- **Events** Autocomplete (multi-select, disableCloseOnSelect,
checkbox rows, per-option description). Options mirror the
backend's knownGitHubEvents map verbatim — a comment at each
site points at the other so the lists stay in sync.
Submit builds `config = { repo, events }` directly; the raw JSON
textarea is hidden for kind=github (other kinds still use it). Help
text on the webhook hint now mentions the events whitelist so the
operator knows to match the same selection on GitHub's "let me select
individual events" page.
Verified live: opened the dialog, picked github, typed `helixml/helix`
into Repository, selected an event via keyboard, clicked Create. The
stored row reads exactly:
transport_kind | transport_config
github | {"events":["issue_comment"],"repo":"helixml/helix"}
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The github-transport stream flow has been overhauled so the only
thing the operator picks is which repo. Helix handles everything
else server-side via the GitHub REST API:
- Searchable repo dropdown (Autocomplete, freeSolo so users can type
any owner/name even when org-OAuth approval hides it from the list)
- Auto-generated per-org webhook_secret on first install
- Auto-installed webhook with content_type=json + events=["*"] (the
GitHub wildcard meaning "send everything")
- UpsertWebhook patches existing hooks with mismatched config
(content_type, events, active) instead of silently adopting them
- Inbound handler accepts both `application/json` and
`application/x-www-form-urlencoded` (payload=…) bodies so existing
webhooks set up by hand don't 400
UI changes on the New Stream dialog:
- Github option disabled when no OAuth connection ("Connect GitHub
for streams" CTA panel triggers a popup OAuth flow with the
required `repo` + `admin:repo_hook` scopes baked in)
- "Reconnect with stream permissions" link when OAuth IS connected
but might be missing scopes (e.g. private org repos not visible)
- 1000-repo cap on the dropdown, sorted by GitHub's pushed-desc so
most-active repos surface first
- Localhost/loopback warning on the detail page checks the resolved
`effective_public_url` (streams.public_url override winning over
SERVER_URL) so non-loopback overrides clear the warning
New org-config spec `streams.public_url` (UI-editable on the
helix-org Settings page) lets operators fix a loopback SERVER_URL
without restarting the api container.
Detail page also gains:
- Configuration panel with inline Edit form (PUT /streams/{id})
- "Connect to GitHub" panel showing webhook install state + an
"Edit on GitHub →" deep-link to the hook's settings page
- "Open helix-org Settings →" button inside the loopback warning
Fixes along the way:
- Toast z-index 1600 -> 100010 (theme MuiDialog override is 100002,
so toasts triggered from a dialog were rendering behind it)
- OAuth connection lookup orders by updated_at DESC (re-auth flows
insert new rows instead of updating, was returning the stale one)
- GitHubTokenResolver now iterates ALL github OAuth providers
(newest first) instead of relying on the in-memory manager map's
non-deterministic iteration order
- LoadRepos calls ListByAuthenticatedUser (was hitting /users//repos
with an empty username -> 404)
QA.md §11e rewritten to cover the new one-click flow with regression
pins for: wildcard events, auto-secret bootstrap, idempotent
UpsertWebhook with patch-on-mismatch, form-encoded delivery decode,
streams.public_url override, OAuth scope re-auth via popup.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Coverage for the regressions fixed in 8560c3b: - pkg/github/client_test.go: TDD for the helpers UpsertWebhook depends on (sameEvents, apiHookURLToHTMLURL) and the three paths UpsertWebhook now needs to pin: CREATE on a fresh repo, NO-OP adopt when an existing hook already matches, PATCH when content_type drifts (the regression that caused every delivery to 400 with `parse json: invalid character 'p'`). Also pins LoadRepos calls /user/repos (ListByAuthenticatedUser) and NOT /users//repos (the empty-username bug we hit before fixing it). - pkg/org/infrastructure/transports/github/github_test.go: 6 new table-driven tests covering wildcard "*" events (the new "send me everything" default), form-encoded webhook bodies (the `payload=…` content type GitHub uses when the operator set up the hook before content_type=json was enforced), and the full per-stream HandleInboundForStream contract — pinned routing to one stream (no fan-out), repo-filter still applies, events-whitelist filter still applies, unknown stream → 404. - pkg/org/infrastructure/persistence/memory/streams_update_test.go: 5 tests pinning the streamsRepo.Update contract — mutable fields swap, immutables (ID, OrgID, CreatedBy, CreatedAt) preserved even when a caller passes a tampered stream, ErrNotFound on missing row, (org_id, name) uniqueness on rename, and the self-rename case (updating without changing name doesn't false-positive on the uniqueness check). - pkg/org/interfaces/server/api/api_test.go: 3 tests pinning StreamDTO.EffectivePublicURL — streams.public_url org config wins over Deps.PublicServerURL (SERVER_URL env), fallback to PublicServerURL when no override, and EffectivePublicURL only populated for github-kind streams (not local/webhook/postmark). - pkg/server/helix_org_github_test.go: 5 tests for newGitHubOAuthResolver — newest-provider wins (the regression where the in-memory oauth.Manager map iteration randomly picked an old provider with no live connection), falls through to older providers if newer has no connection, no providers → ("", nil), nil deps → nil resolver, store errors propagate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Workers in helix-org now land in their desktop sandbox with the
`gh` CLI already authenticated against the org's connected GitHub
OAuth app — no manual `gh auth login`, no PAT in environment.
Two pieces, one commit so they ship together:
(A) `gh` is now installed in the sandbox image via the official
cli.github.com apt repo (Dockerfile.ubuntu-helix).
(B) The helix-org Spawner injects `GH_TOKEN` as a project secret
on every activation. Helix's runtime already surfaces project
secrets as env vars on fresh desktop containers, so the worker
sees a current, org-scoped OAuth access token without ever
holding one at rest in helix-org's own state.
Token resolution reuses the existing GitHubTokenResolver
(`newGitHubOAuthResolver` in helix_org_github.go) — the same path
the GitHub stream transport authenticates through. No new auth
mechanism, no PAT fallback, no second copy of the OAuth lookup.
Resolved on every activation (NOT at hire time) so re-issued OAuth
tokens take effect on the next session without re-hiring. The
resolver returning "" (no member has connected GitHub yet) is a
soft skip — we don't shadow a previously-valid value with an empty
string. Resolver errors are logged best-effort: a failed GitHub
lookup must not take the whole activation down.
Tests pin:
- SpawnerConfig.GitHubTokenResolver is honoured: token from
resolver lands in PutProjectSecret("GH_TOKEN").
- Empty-token / nil-resolver skip is preserved (no shadowing).
- The host's buildHelixOrgSpawnerConfig wires the resolver
through verbatim (catches future plumbing regressions).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Hire auto-spawn paths After the position-anchored subscriptions refactor (7f3bc73) the dispatch path that turns a publish into a Spawn call got a position- hop inserted in the middle: publish → ListForStream → resolve position → list workers in position → Queue.Enqueue → Spawn Existing tests covered the publisher-skip and SourceKind rules but did not pin the most load-bearing claim — that an event landing on s-activations-<W> with W's position subscribed actually reaches the Spawner callback for W. Likewise, DispatchHire (the hire-worker direct path that bypasses streams) had no unit-level pin. These two tests close those gaps so a future refactor cannot re-introduce the symptom the bug report described — newly-hired AI workers not auto-spawning — without a red CI signal at the dispatch layer. Investigation note: reproducing the reported "auto-spawn broken" symptom against the live inner-helix at localhost:8080 with a fresh AI hire (w-zorin into p-ceo) showed the desktop session DID auto-spawn — the helix project, agent app, repo, and chat session were all provisioned without operator intervention, the container came up, and the activation transcript landed on s-activations-w-zorin. Both new tests pass green against the current codebase. Treating the tests as regression pins rather than a red-then-green TDD pair. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Workers that run `gh` inside their sandbox surface a warning on every `gh auth status` invocation when the token lacks `read:org`, and a handful of `gh` subcommands that touch org-owned resources (cross-org issue listing, team assignment) fail outright. The webhook-install path doesn't need it, but the worker side does — add it to the scope list so a single Reconnect cleans both up. Existing connections aren't auto-upgraded; operators have to click "Reconnect with stream permissions →" once for the new scope to take effect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The spawner used to have a github-shaped hole baked into its
config — a `GitHubTokenResolver` field that the activation loop
unconditionally called to upsert `GH_TOKEN`. That doesn't scale:
GitHub is just one of many transports a stream could carry
(postmark, slack, custom HTTP — anything operators wire in), and
each one may have its own per-activation secrets to push into the
worker sandbox.
Refactor into a generic `SpawnSecretInjector` interface defined in
the spawner package. Each transport's infrastructure package
exposes a constructor (e.g. `github.NewSecretInjector(resolver)`)
that builds a concrete injector; the host gathers a slice of them
and passes it through `SpawnerConfig.SecretInjectors`. The spawner
iterates every registered injector on every activation, soft-skips
empty maps so a transport without wired auth can't shadow a
previously-valid secret, and best-effort-logs errors so one
transport's outage can't take an activation down.
Layering:
- runtime/helix/spawn_hooks.go owns the interface + the
SpawnSecretInjectorFunc adapter for func-based wiring.
- The spawner's activation loop calls cfg.runSecretInjectors after
ensureProject, before ensureSession — keeps the
re-resolved-every-activation contract intact.
- transports/github/secret_injector.go exports
NewSecretInjector(resolver) which returns
SpawnSecretInjectorFunc{Label: "github", Fn: …}. github's
existing TokenResolver is reused unchanged — same OAuth pipeline
the stream transport already uses for outbound `Token()`.
- helix_org.go gathers `[]SpawnSecretInjector` once and threads it
through buildHelixOrgSpawnerConfig + lazyHelixOrgSpawner. The
field formerly known as GitHubTokenResolver on the spawner
config is gone.
Tests:
- spawn_hooks_test.go pins the adapter, the iteration loop's
composition + soft-skip + error-tolerance contract.
- secret_injector_test.go (github) pins the constructor — label,
nil-resolver, happy-path, empty-token-skip, error-propagation.
- spawner_test.go (existing): TestSpawnerInjectsGHTokenSecret +
TestSpawnerSkipsGHTokenWhenResolverEmpty rewritten as
TestSpawnerRunsRegisteredSecretInjectors +
TestSpawnerSkipsInjectorReturningEmptyMap — exercising the
generic interface, not the github-specific field.
- helix_org_spawner_test.go: TestBuildHelixOrgSpawnerConfig_WiresSecretInjectors
replaces the GitHubTokenResolver-specific variant.
Adding a new transport with per-activation secrets is now a single
constructor in that transport's package + one slice append in the
host — no spawner changes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ject
Two new owner-only MCP tools let the org owner inspect and edit a
Worker's per-Worker helix project configuration from chat:
- get_worker_project(workerId)
Reads the current project config — startup script today,
with skills/guidelines and any other Project field landing
under the same shape as we wire them up.
- configure_worker_project(workerId, startupScript?)
Partial patch. Fields omitted from the args are preserved
as-is; setting a field to an empty string is an explicit
"clear" (non-nil pointer for the runtime to act on). Empty
patch is rejected so a careless agent can't silently
no-op.
The intended owner workflow is read-modify-write: call
get_worker_project, compose a new startup script that preserves
existing content (e.g. add an `apt install gh` line on top of
what's already there), then call configure_worker_project with
the merged result.
Layering:
- runtime/runtime.go owns the ProjectConfig port + the
ProjectConfigSnapshot/Patch shapes + the NoopProjectConfig
default + ErrProjectConfigUnsupported. Adding a new patchable
field is a column on the snapshot, a pointer on the patch,
one line in the configure tool's args struct, and one line in
projectConfigPatchHasFields. No spawner-side changes.
- runtime/helix/project_config.go is the production impl: looks
up workerID → projectID via WorkerRuntimeState, then reads or
writes the project via the existing ProjectService
(GetProject + the newly-added UpdateProject).
- application/tools/{get,configure}_worker_project.go are the
MCP tools — thin wrappers that parse args, gate on workerId,
call ProjectConfig, return the snapshot JSON-encoded.
- Wired through helix_org.go: NewProjectConfig builds the impl
from the in-proc helix client + the org store, and lands on
tools.Deps.ProjectConfig. The MCP server's registry picks both
tools up via the existing builtins list.
TDD:
- configure_worker_project_test.go pins the tool surface against
a fake ProjectConfig: happy path, missing workerId, unsupported
runtime, nil deps, empty patch rejected, empty-string
startup_script accepted as an explicit clear, port errors
propagate, no-op short-circuits before touching the port.
- project_config_test.go (runtime/helix) pins the impl: rejects
nil deps at construction, round-trips a known startup script
through GetProject + UpdateProject on a fake ProjectService,
worker-with-no-project-id returns ErrProjectConfigUnsupported,
nil patch fields stay nil on the underlying
ProjectUpdateRequest (no zero-value overwrites).
- Existing fakeProjectService gained an UpdateProject stub that
also seeds GetProject so the same fake can model a "read after
write" sequence.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `gh --version` smoke test at the end of the gh install layer
was poisoning every fresh worker sandbox: `gh` writes
`${HOME}/.local/state/gh/device-id` on its first invocation as
part of its analytics opt-in flow, and HOME during a docker build
is root. So the build created `/home/retro/.local/` owned by root.
At session-start, Zed runs as retro and tries to create
`/home/retro/.local/share/zed/extensions`, `/.../languages`, etc.
— and crashes:
Zed failed to launch: permission denied when creating
directories ["/home/retro/.local/share/zed/extensions", …]
Without Zed there is no agent WebSocket back to the API, so every
follow-up interaction fails with "no external agent WebSocket
connection" and auto-wake retries exhaust. End-user symptom: chat
with worker doesn't work; clicking Open Human Desktop produces a
session whose composer queues messages forever.
Fix: drop the `gh` invocation from the build entirely. `apt-get
install -y gh` already exits non-zero on failure, which is the
only smoke test we actually need. Replaced with `which gh` so the
binary presence is still checked without triggering gh's analytics
init.
Requires `./stack build-ubuntu` to bake a new desktop image; new
sessions pick it up automatically. Existing broken containers can
be unblocked in place with:
docker exec <ubuntu-external-…> chown -R retro:retro /home/retro/.local
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ecovery)
Adds §13 covering everything I learned today while getting the
chat-with-worker flow back to green end-to-end:
- §13a: fresh sandbox container's Zed launch contract — pins the
/home/retro/.local ownership regression so a future contributor
invoking gh (or anything else that writes to ~/.local/) inside
the Dockerfile gets caught at QA time, not at session-start
when the agent silently fails to come up.
- §13b: the `gh` install + GH_TOKEN auto-injection — confirms
`gh auth status` succeeds inside a fresh sandbox without a
manual auth step, and that a real `gh issue comment` posts.
Caveat about the read:org scope re-auth ties back to §11e.
- §13c: stale-session recovery after `./stack build-ubuntu`.
This is the bit that bit us: an image rebuild leaves every
existing exploratory session row pointing at a now-dead
container, and the FE's "resume" path silently no-ops on the
pointer. Documents the kill-container + delete-sessions +
clear-worker-runtime-state recipe with the exact SQL we used.
Pins the gotcha that POST /sessions/{id}/resume returns 200
but doesn't actually respawn the runner.
- §13d: the full chat round-trip — what to look for, in what
order, to confirm the chain holds. References §13a-c as the
three places to check before chasing deeper bugs.
Also updates §12's "LLM reply may not arrive" caveat to point at
§13 for the full round-trip, and extends Pass criteria with
§13a-d entries.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cut to 383 lines (was 803). Every feature is tested in exactly one section now; cross-references replace duplicated steps. Removed: - Backend / Go-test references (TestBuildHelixOrgSpawnerConfig, TestGetStream_IncludesRecentEvents, lifecycle_test.go, etc.) — the QA plan is UI-focused; backend regression coverage lives in the test packages. - Raw DB SQL spot-checks (org_roles count, org_positions count, org_workers, etc.) — the UI assertions already cover the user-visible contract; SQL belongs in dev runbooks, not the QA plan. - The cloudflared smoke-test SQL snippets — replaced with a single UI-driven E2E in §8. - Verbose "regression context" prose where the heading already names the bug. - §3's API panic log inspection — the UI assertion (chip click doesn't crash) suffices. - §11g's three subsections collapsed into §9 (drag-to-subscribe, survives-fire, worker panel — one section, one feature). - §13's four subsections collapsed into §12 (Zed launch, gh auth, stale-session recovery — one section, three regression pins). - §1 (Setup) trimmed from 14 lines to 3. Structure now: §1 Bootstrap + sidebar §2 Build the chart (roles, positions, edges, sever) §3 Hire + cascade semantics §4 Cross-org + persistence + theme §5 Roles list + tool editor §6 Workers list §7 Streams list, detail, live tail §8 GitHub streams one-click §9 Position-anchored subscriptions §10 Stream delete cascade §11 Chat → Human Desktop §12 Worker sandbox (Zed + gh + recovery) Pass criteria condensed to a one-line bullet per section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Humans don't get spawner activation, so only AI workers (plus the bootstrapped w-owner) have s-activations-<workerID> streams. Caught during the §3 hire walkthrough where the chart didn't grow a stream node for hired humans.
- github/client: simplify sameEvents with slices.Equal; replace apiHookURLToHTMLURL with a WebhookSettingsURL(owner, repo, id) helper invoked at the call site instead of parsing the API URL - websocket_external_agent_sync: trim verbose docstrings and emoji log noise on the chat_response_error → interaction surface paths - memorystore: drop H1.3c-style commit-trail comments - helixOrgService: migrate every HelixOrg endpoint to the generated TS API client (api.getApiClient().v1OrgsXxx) and the generated Api*DTO types; ~880 → ~469 lines - CLAUDE.md: migrate the helix-org design philosophy (prompt-driven, minimal MCP surface, no workflow in code) from the deleted helix-org/CLAUDE.md - revert Dockerfile.ubuntu-helix gh CLI install - drop in-flight design/2026-06-02 org-scoping docs - strip phase tags (H1.3c, H5.2 etc.) from production + test comments throughout api/pkg/org Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
philwinder
added a commit
that referenced
this pull request
Jun 4, 2026
helix-org redesign (#2516) added TestInProcProjectService_GetProject_Found_ReturnsProject; projects/show-owner-in-manage-access (9a322cd, merged in parallel) added populateProjectOwners → Store.GetUser. Both passed CI on their own branches, but on main the test's GetProject call now panics with SIGSEGV because MemoryStore embeds a nil store.Store and GetUser hits the autogenerated nil-method. Returning ErrNotFound lets populateProjectOwners log a warn and continue (its existing err path) — projects in the in-process adapter tests come up without a hydrated owner, which is fine because they don't exercise that field. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 6, 2026
philwinder
added a commit
that referenced
this pull request
Jun 9, 2026
…2557) The activation stream only recorded lifecycle markers ("=== activation ... ===", "=== exit ... ==="), never the agent's actual transcript (assistant text, tool_use, tool_result). The org redesign (#2516) replaced the bridge's client subscribe with a topic-based SubscribeSessionUpdates call that hardcoded an empty ownerID: SubscribeSessionUpdates(ctx, cfg.PubSub, cfg.Snapshotter, "", sessionID) But helix publishes every session update to GetSessionQueue(session.Owner, sessionID) — the owning user's topic (observability.go, websocket_external_agent_sync.go, controller/ sessions.go all use the real owner). Subscribing with "" lands the bridge on session-updates..<id>, a topic nobody publishes to, so it receives zero frames. Only the markers the spawner publishes directly survived — exactly the reported symptom. Fix: resolve the session owner (via a new SpawnerClient.SessionOwner, backed by Store.GetSession) and subscribe to the owner's topic, mirroring the browser WS handler in websocket_server_user.go which already does this and documents why. The transcript test now publishes under a real owner and would fail if the empty-owner regression returns. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
Large refactor of the helix-org subsystem. Three themes:
1. Multi-tenant store schema (org-scoped). All
org_*tables now use composite(id, org_id)primary keys and the org id is plumbed through tools, bootstrap, and the helix runtime. Tables are namespaced with anorg_prefix; per-org bootstrap is serialised. Seedesign/2026-06-02-org-scoping-handoff.mdfor the full plan.2. Streams + transports. Per-stream detail page with live SSE tail; chart stream-node deep-links; one-click GitHub webhook setup + auto-install reusing the org's OAuth (with
read:orgscope); structured Repo + Events fields on the github transport; genericSpawnSecretInjectorhook so transports register their own per-activation secret injection (GH_TOKEN moved behind this) — no more github-specific coupling in the spawner.3. Chart / workers UX. Position-anchored subscriptions (worker→position pivot); Fire cascades the activation stream + chart stream-delete; chat button launches the Human Desktop; stream edges anchored to subject + dedicated handle.
Also lands:
get_worker_project+configure_worker_project(read-modify-write the Worker's helix project, currently the Startup Script; future Skills etc.)Dockerfile.ubuntu-helix(was creating root-owned/home/retro/.local/state/gh/device-idduring build, breaking Zed launch).api/pkg/org/QA.mdto UI-only, single-source-per-feature (12 sections), verified end-to-end on a fresh wipe.156 commits, 313 files, +69k / -43k.
Test plan
gh pr checks)./stack build && ./stack start, registertest@helix.ml, complete onboardingapi/pkg/org/QA.md(already walked through locally — see commit9cc208279for the AI-only activation streams clarification)gh auth statusshows ✓ via GH_TOKENconfigure_worker_projectMCP tool from an owner chat to update a Worker's Startup Script; confirmget_worker_projectreflects the changes-<stream>and downstream subscribers🤖 Generated with Claude Code