Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/helm-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ on:
- 'tests/check-windows-appservice-normalization.ps1'
- 'tests/check-apply-wrapper-flags.sh'
- 'tests/check-helm-agentteams.sh'
- 'tests/check-create-project-grant-admin.sh'
- 'manager/agent/skills/project-management/**'
workflow_dispatch: ~

jobs:
Expand Down Expand Up @@ -53,6 +55,9 @@ jobs:
- name: Check declarative apply wrapper flags
run: bash tests/check-apply-wrapper-flags.sh

- name: Check create-project grant-admin wiring
run: bash tests/check-create-project-grant-admin.sh

- name: Set up Helm
uses: azure/setup-helm@v4
with:
Expand Down
96 changes: 96 additions & 0 deletions agentteams-controller/internal/controller/human_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package controller
import (
"context"
"errors"
"fmt"
"sort"
"testing"

Expand Down Expand Up @@ -635,3 +636,98 @@ func sortedCopy(in []string) []string {
// Silence unused import lint when the service package is referenced only
// via the mock type alias in some future subtest.
var _ = service.HumanCredentials{}

// TestHumanReconciler_PowerLevelMapping checks that every room in the
// desired set — new and already-observed — gets EnsureRoomPowerLevel with
// the level derived from the Human CR's permissionLevel.
func TestHumanReconciler_PowerLevelMapping(t *testing.T) {
worker := newReadyWorker("w1", "!room-w1:localhost")
team := newReadyTeam("t1", "!room-t1:localhost")
human := newHuman("alice", v1beta1.HumanSpec{
PermissionLevel: 1, // admin-equivalent → 100
AccessibleWorkers: []string{"w1"},
AccessibleTeams: []string{"t1"},
})
human.Status.MatrixUserID = "@alice:localhost"
human.Status.InitialPassword = "stored-pw"
human.Status.Rooms = []string{"!room-w1:localhost"} // already in the worker room
human.Status.Phase = "Active"
human.Finalizers = []string{finalizerName}

rig := newHumanRig(t, human, worker, team)
out, _, err := rig.reconcile("alice")
if err != nil {
t.Fatalf("reconcile: %v", err)
}

byRoom := map[string]int{}
for _, c := range rig.prov.Calls.EnsureRoomPowerLevel {
byRoom[c.RoomID] = c.Level
}
if byRoom["!room-w1:localhost"] != 100 {
t.Errorf("existing room heal: level=%d, want 100 (calls=%+v)", byRoom["!room-w1:localhost"], rig.prov.Calls.EnsureRoomPowerLevel)
}
if byRoom["!room-t1:localhost"] != 100 {
t.Errorf("new room: level=%d, want 100 (calls=%+v)", byRoom["!room-t1:localhost"], rig.prov.Calls.EnsureRoomPowerLevel)
}
for _, c := range rig.prov.Calls.EnsureRoomPowerLevel {
if c.UserID != "@alice:localhost" {
t.Errorf("power level granted to %s, want @alice:localhost", c.UserID)
}
}
if len(out.Status.Rooms) != 2 {
t.Errorf("Status.Rooms=%v, want both rooms", out.Status.Rooms)
}
}

// Team-scoped humans (level 2) get the default member level (50), not room
// ownership.
func TestHumanReconciler_PowerLevelL2GetsDefault(t *testing.T) {
worker := newReadyWorker("w1", "!room-w1:localhost")
human := newHuman("bob", v1beta1.HumanSpec{
PermissionLevel: 2,
AccessibleWorkers: []string{"w1"},
})
human.Status.MatrixUserID = "@bob:localhost"
human.Status.InitialPassword = "stored-pw"
human.Status.Phase = "Active"
human.Finalizers = []string{finalizerName}

rig := newHumanRig(t, human, worker)
if _, _, err := rig.reconcile("bob"); err != nil {
t.Fatalf("reconcile: %v", err)
}
if len(rig.prov.Calls.EnsureRoomPowerLevel) != 1 {
t.Fatalf("power calls=%+v, want 1", rig.prov.Calls.EnsureRoomPowerLevel)
}
if got := rig.prov.Calls.EnsureRoomPowerLevel[0].Level; got != 50 {
t.Errorf("level=%d, want 50", got)
}
}

// A power-level grant failure is non-fatal: the room is still recorded and
// the next cycle retries.
func TestHumanReconciler_PowerLevelErrorNonFatal(t *testing.T) {
worker := newReadyWorker("w1", "!room-w1:localhost")
human := newHuman("carol", v1beta1.HumanSpec{
PermissionLevel: 2,
AccessibleWorkers: []string{"w1"},
})
human.Status.MatrixUserID = "@carol:localhost"
human.Status.InitialPassword = "stored-pw"
human.Status.Phase = "Active"
human.Finalizers = []string{finalizerName}

rig := newHumanRig(t, human, worker)
rig.prov.EnsureRoomPowerLevelFn = func(ctx context.Context, roomID, userID string, level int) error {
return fmt.Errorf("matrix unavailable")
}

out, _, err := rig.reconcile("carol")
if err != nil {
t.Fatalf("power-level failure must be non-fatal, got: %v", err)
}
if len(out.Status.Rooms) != 1 || out.Status.Rooms[0] != "!room-w1:localhost" {
t.Errorf("room not recorded despite power failure: %v", out.Status.Rooms)
}
}
65 changes: 47 additions & 18 deletions agentteams-controller/internal/controller/human_reconcile_rooms.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,29 +44,43 @@ func (r *HumanReconciler) reconcileHumanRooms(ctx context.Context, s *humanScope
next := make([]string, 0, len(h.Status.Rooms)+len(desired))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The healing pass calls EnsureRoomPowerLevel (one GET, conditional PUT) for every desired room on every reconcile cycle, even when nothing has changed. For a human in N rooms this adds N Matrix API GETs per 5-minute requeue. The PR documents this trade-off and the exact-match short-circuit avoids writes, but consider adding a status annotation (e.g. a lastPowerLevelGranted map in status) or a generation check to skip the GET when spec.permissionLevel hasn't changed since the last successful grant. This is a future optimization, not a blocker.

next = append(next, h.Status.Rooms...)

powerLevel := humanRoomPowerLevel(h.Spec.PermissionLevel)

for rid := range desired {
alreadyMember := false
if _, ok := observed[rid]; ok {
continue
alreadyMember = true
}
if err := r.Provisioner.InviteToRoom(ctx, rid, matrixUserID); err != nil {
logger.Error(err, "failed to invite human to room", "room", rid)
continue
if !alreadyMember {
if err := r.Provisioner.InviteToRoom(ctx, rid, matrixUserID); err != nil {
logger.Error(err, "failed to invite human to room", "room", rid)
continue
}
// Acquire a user token lazily — only on the first new-room
// addition of this reconcile. Steady-state passes (desired ==
// observed) and revoke-only passes never reach this call, so
// Matrix Login is not issued on every 5-minute requeue.
token := r.ensureUserToken(ctx, s)
if token == "" {
logger.V(1).Info("user token unavailable; invite-only this cycle",
"room", rid, "human", h.Name, "username", s.username)
continue
}
if err := r.Provisioner.JoinRoomAs(ctx, rid, token); err != nil {
logger.Error(err, "failed to join room as human", "room", rid)
continue
}
next = append(next, rid)
}
// Acquire a user token lazily — only on the first new-room
// addition of this reconcile. Steady-state passes (desired ==
// observed) and revoke-only passes never reach this call, so
// Matrix Login is not issued on every 5-minute requeue.
token := r.ensureUserToken(ctx, s)
if token == "" {
logger.V(1).Info("user token unavailable; invite-only this cycle",
"room", rid, "human", h.Name, "username", s.username)
continue
// Grant the human their power level in every room they should be
// in — new rooms and already-observed ones alike. The existing-room
// pass is the healing path: legacy rooms were created before power
// levels accounted for human members, leaving them at the implicit
// level 0 and 403 on room operations (rename, invite). Non-fatal
// per this file's error policy; the next cycle retries.
if err := r.Provisioner.EnsureRoomPowerLevel(ctx, rid, matrixUserID, powerLevel); err != nil {
logger.Error(err, "failed to ensure human power level", "room", rid, "level", powerLevel)
}
if err := r.Provisioner.JoinRoomAs(ctx, rid, token); err != nil {
logger.Error(err, "failed to join room as human", "room", rid)
continue
}
next = append(next, rid)
}

// Removals: in-place filter. A failed kick keeps the room so the
Expand All @@ -86,6 +100,21 @@ func (r *HumanReconciler) reconcileHumanRooms(ctx context.Context, s *humanScope
h.Status.Rooms = kept
}

// humanRoomPowerLevel maps the Human CR permission level to the Matrix power
// level granted in rooms the human belongs to. Level 1 (admin equivalent)
// co-owns the rooms (full control); levels 2/3 (team/worker scoped) get
// level 50 — Matrix's default member authority: rename, invite, kick, ban
// and redact (the homeserver defaults all sit at 50), but not power-level
// changes, and only against members strictly below 50 — the manager/leader
// at 100 can never be kicked or banned by the human. This authority is
// accepted and documented in docs/design/room-power-levels.md.
func humanRoomPowerLevel(permissionLevel int) int {
if permissionLevel == 1 {
return 100
}
return 50
}

// ensureUserToken returns a Matrix access token for the human,
// acquiring one via Login on first call per reconcile and caching it
// in the scope. Returns "" when login fails — callers degrade to
Expand Down
39 changes: 39 additions & 0 deletions agentteams-controller/internal/matrix/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ type Client interface {
// it falls back to the homeserver-admin identity.
SetRoomState(ctx context.Context, roomID, eventType, stateKey string, content map[string]interface{}, userToken string) error

// GetRoomState reads the content of a single state event from a room
// using the homeserver-admin identity (the event's `content` object,
// not the full event envelope). A room that has never had the event
// set (e.g. legacy rooms with no m.room.power_levels) yields (nil, nil)
// rather than an error; any other failure is returned.
GetRoomState(ctx context.Context, roomID, eventType, stateKey string) (map[string]interface{}, error)

// JoinRoom makes the user identified by token join the given room.
JoinRoom(ctx context.Context, roomID, userToken string) error

Expand Down Expand Up @@ -753,6 +760,38 @@ func (c *TuwunelClient) SetRoomState(ctx context.Context, roomID, eventType, sta
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GetRoomState builds the URL as state/{eventType} when stateKey is empty, while the existing SetRoomState always uses state/{eventType}/{stateKey} (producing a trailing slash for empty stateKey, e.g. state/m.room.power_levels/). Both forms are valid per the Matrix spec and most homeservers accept either, but the inconsistency could matter against strict implementations. Consider aligning GetRoomState to the same state/%s/%s pattern used by SetRoomState for symmetry, or document the deliberate difference.

}

func (c *TuwunelClient) GetRoomState(ctx context.Context, roomID, eventType, stateKey string) (map[string]interface{}, error) {
token, err := c.ensureAdminToken(ctx)
if err != nil {
return nil, fmt.Errorf("get room state %s %s: %w", roomID, eventType, err)
}
encodedRoom := encodeRoomID(roomID)
// Always include the state-key segment (trailing slash when the key is
// empty) to match SetRoomState — some strict homeservers reject the
// segment-less form for empty-key events.
path := fmt.Sprintf("/_matrix/client/v3/rooms/%s/state/%s/%s",
encodedRoom, url.PathEscape(eventType), url.PathEscape(stateKey))
statusCode, respBody, err := c.doJSON(ctx, http.MethodGet, path, token, nil, nil)
if err != nil {
return nil, fmt.Errorf("get room state %s %s: %w", roomID, eventType, err)
}
if statusCode == http.StatusNotFound {
return nil, nil // state event never set on this room
}
if statusCode != http.StatusOK {
return nil, fmt.Errorf("get room state %s %s: HTTP %d: %s",
roomID, eventType, statusCode, truncate(respBody, 500))
}
// The state endpoint returns the state CONTENT object directly
// (e.g. {"users":{...},"ban":50}), not an event envelope — decode
// the wire response as-is.
var content map[string]interface{}
if err := json.Unmarshal(respBody, &content); err != nil {
return nil, fmt.Errorf("get room state %s %s: decode: %w", roomID, eventType, err)
}
return content, nil
}

func (c *TuwunelClient) JoinRoom(ctx context.Context, roomID, userToken string) error {
encodedRoom := encodeRoomID(roomID)
statusCode, respBody, err := c.doJSON(ctx, http.MethodPost,
Expand Down
52 changes: 52 additions & 0 deletions agentteams-controller/internal/matrix/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,58 @@ func TestSetRoomState(t *testing.T) {
}
}

func TestGetRoomState(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/_matrix/client/v3/login":
adminLoginHandler(t, w)
case "/_matrix/client/v3/rooms/!room:d/state/m.room.power_levels/":
// Trailing slash: the empty state key is always included in the
// URL (aligned with SetRoomState, strict-homeserver safe).
if r.Method != http.MethodGet {
t.Errorf("method = %s, want GET", r.Method)
}
if auth := r.Header.Get("Authorization"); auth != "Bearer admin-token" {
t.Errorf("Authorization = %q, want Bearer admin-token", auth)
}
w.WriteHeader(http.StatusOK)
// A compliant homeserver returns the state CONTENT object
// directly — no event envelope (no type/state_key/sender, no
// "content" wrapper).
json.NewEncoder(w).Encode(map[string]interface{}{
"users": map[string]interface{}{"@a:d": 100.0},
})
case "/_matrix/client/v3/rooms/!room:d/state/room.meta/":
w.WriteHeader(http.StatusNotFound)
w.Write([]byte(`{"errcode":"M_NOT_FOUND"}`))
default:
t.Errorf("unexpected path: %s", r.URL.Path)
w.WriteHeader(http.StatusNotFound)
}
}))
defer server.Close()

c := NewTuwunelClient(Config{ServerURL: server.URL, Domain: "d"}, server.Client())

st, err := c.GetRoomState(context.Background(), "!room:d", "m.room.power_levels", "")
if err != nil {
t.Fatalf("GetRoomState: %v", err)
}
users, ok := st["users"].(map[string]interface{})
if !ok || users["@a:d"] != 100.0 {
t.Fatalf("users=%#v, want @a:d=100 (content, not event envelope)", st)
}

// A room that never had the state set yields (nil, nil), not an error.
st, err = c.GetRoomState(context.Background(), "!room:d", "room.meta", "")
if err != nil {
t.Fatalf("missing state must not error: %v", err)
}
if st != nil {
t.Errorf("missing state = %#v, want nil", st)
}
}

// adminLoginHandler returns a handler that responds to admin login with a
// fixed token, allowing tests that exercise admin-driven endpoints.
func adminLoginHandler(t *testing.T, w http.ResponseWriter) {
Expand Down
18 changes: 18 additions & 0 deletions agentteams-controller/internal/service/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ type WorkerProvisioner interface {
// KickFromRoom removes userID from roomID using the admin token.
// Idempotent: returns nil when the user is not a member.
KickFromRoom(ctx context.Context, roomID, userID, reason string) error

// EnsureRoomPowerLevel reconciles userID's entry in the room's
// m.room.power_levels to exactly the given power level (raising or
// lowering it), using the admin token. The complete existing content is
// preserved (every other user, every non-user field); only the target
// users entry is mutated. The call is a no-op write when the user
// already has exactly that level. Rooms that never had power_levels set
// (legacy) are treated as starting from an empty users map.
EnsureRoomPowerLevel(ctx context.Context, roomID, userID string, level int) error
// ForceLeaveRoom removes a user whose room power level prevents a normal
// admin kick.
ForceLeaveRoom(ctx context.Context, userID, roomID string) error
Expand Down Expand Up @@ -238,6 +247,15 @@ type HumanProvisioner interface {
// Idempotent: returns nil when the user is not a member.
KickFromRoom(ctx context.Context, roomID, userID, reason string) error

// EnsureRoomPowerLevel reconciles userID's entry in the room's
// m.room.power_levels to exactly the given power level (raising or
// lowering it), using the admin token. The complete existing content is
// preserved (every other user, every non-user field); only the target
// users entry is mutated. The call is a no-op write when the user
// already has exactly that level. Rooms that never had power_levels set
// (legacy) are treated as starting from an empty users map.
EnsureRoomPowerLevel(ctx context.Context, roomID, userID string, level int) error

// ForceLeaveRoom asks the Tuwunel admin bot to force-leave userID out
// of roomID via "!admin users force-leave-room". Fire-and-forget at
// the bot layer, but the admin message delivery itself is confirmed.
Expand Down
39 changes: 39 additions & 0 deletions agentteams-controller/internal/service/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -1074,6 +1074,45 @@ func (p *Provisioner) EnsureRoomNonMember(ctx context.Context, roomID, userID, r
return p.matrix.KickFromRoom(ctx, roomID, userID, reason)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The shallow copy content[k] = v means nested objects (e.g. events, notifications maps) are shared between content and the cur map returned by GetRoomState. This is safe today because cur is freshly unmarshalled per call and not cached, and only the top-level users key is mutated. However, if a future caller or middleware caches GetRoomState results, the shared nested maps could be silently corrupted. A defensive deep-copy of cur (or at minimum of the users sub-map before mutation) would be more robust against future refactoring.

}

// EnsureRoomPowerLevel reconciles userID's entry in the room's
// m.room.power_levels to EXACTLY `level` (raising or lowering it) via the
// admin token. The complete existing content is preserved — every other
// user and every non-user field (events, invite, notifications,
// users_default, state_default, ban, kick, redact, extension fields); only
// the target users entry is mutated. Idempotent: no write when the user
// already has exactly `level`. Rooms that never had power_levels set
// (legacy rooms) start from an empty users map.
func (p *Provisioner) EnsureRoomPowerLevel(ctx context.Context, roomID, userID string, level int) error {
cur, err := p.matrix.GetRoomState(ctx, roomID, "m.room.power_levels", "")
if err != nil {
return fmt.Errorf("read power levels %s: %w", roomID, err)
}
// Preserve the complete existing content and mutate only the target
// users entry — rebuilding the struct would drop fields we don't
// explicitly know about (events, invite, notifications, extensions).
content := map[string]interface{}{}
var users map[string]interface{}
if cur != nil {
for k, v := range cur {
content[k] = v
}
users, _ = cur["users"].(map[string]interface{})
}
if users == nil {
users = map[string]interface{}{}
}
// Exact-target semantics: a demoted human (e.g. permissionLevel 1 → 2,
// 100 → 50) must actually be lowered, not kept at the old level.
if have, ok := users[userID]; ok {
if n, ok := have.(float64); ok && int(n) == level {
return nil // already at exactly the desired level — no write
}
}
users[userID] = float64(level)
content["users"] = users
return p.matrix.SetRoomState(ctx, roomID, "m.room.power_levels", "", content, "")
}

// ReconcileRoomMembership drives the membership of roomID to match `desired`
// (a list of full Matrix user IDs). Users present in `desired` but not in
// the room are invited; users in the room but not in `desired` are kicked.
Expand Down
Loading
Loading