Refactor and upgrade codebase#80
Open
djthorpe wants to merge 3 commits into
Open
Conversation
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the codebase to use the newer github.com/mutablelogic/go-auth module layout and bumps several Go dependencies, including modelcontextprotocol/go-sdk, go-pg, and go-server.
Changes:
- Migrated auth-related imports from
github.com/djthorpe/go-auth/...togithub.com/mutablelogic/go-auth/.... - Updated manager APIs to accept
*auth.UserInfoand switched user scoping/attribution touser.Sub. - Upgraded multiple dependencies in
go.mod/go.sum.
Reviewed changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/test/integration.go | Updates auth schema import path for integration test helpers. |
| mcp/client/run_test.go | Removes blank import separator in imports. |
| mcp/client/run.go | Removes blank import separator in imports. |
| mcp/client/probe.go | Updates auth http client import path. |
| mcp/client/connect_test.go | Updates auth http client import path. |
| mcp/client/connect.go | Updates auth http client import path. |
| mcp/client/client.go | Updates auth http client import path. |
| kernel/schema/table.go | Adds // Packages separator comment in imports. |
| kernel/schema/http.go | Adds explicit import alias for go-pg. |
| kernel/schema/connector.go | Updates OIDC import path. |
| kernel/manager/tool.go | Switches auth schema import + updates user parameter type to *auth.UserInfo. |
| kernel/manager/session.go | Switches to *auth.UserInfo and uses user.Sub for DB scoping/ownership checks. |
| kernel/manager/provider.go | Switches to *auth.UserInfo and uses user.Sub for optional DB scoping. |
| kernel/manager/opt.go | Updates crypto import path. |
| kernel/manager/model_test.go | Updates auth schema import path. |
| kernel/manager/model.go | Switches to *auth.UserInfo across model APIs. |
| kernel/manager/message.go | Switches to *auth.UserInfo and uses user.Sub for message listing scoping. |
| kernel/manager/embedder_test.go | Updates auth schema import path. |
| kernel/manager/embedder.go | Switches to *auth.UserInfo and uses user.Sub for usage attribution. |
| kernel/manager/delegate.go | Uses explicit import alias for pkg/opt. |
| kernel/manager/connector.go | Switches to *auth.UserInfo, updates auth/http/oidc imports, and uses user.Sub for optional scoping. |
| kernel/manager/chat.go | Switches to *auth.UserInfo and uses user.Sub for usage attribution + conversation loading. |
| kernel/manager/ask_test.go | Updates auth schema import path. |
| kernel/manager/ask.go | Switches to *auth.UserInfo and uses user.Sub for usage attribution. |
| kernel/manager/agent.go | Switches to *auth.UserInfo and uses explicit import alias for pkg/opt. |
| kernel/httphandler/tool.go | Updates auth middleware import path. |
| kernel/httphandler/session.go | Updates auth middleware import path. |
| kernel/httphandler/register.go | Updates auth manager import path. |
| kernel/httphandler/model.go | Updates auth middleware import path and import ordering. |
| kernel/httphandler/message.go | Updates auth middleware import path. |
| kernel/httphandler/embedding.go | Updates auth middleware import path. |
| kernel/httphandler/connector.go | Updates auth middleware import path. |
| kernel/httphandler/chat_test.go | Updates auth schema import path. |
| kernel/httphandler/chat.go | Updates auth middleware import path and import ordering. |
| kernel/httphandler/channel.go | Updates auth middleware import path. |
| kernel/httphandler/ask.go | Updates auth middleware import path and import ordering. |
| kernel/httphandler/agent.go | Updates auth middleware import path. |
| kernel/httpclient/client.go | Updates auth http client import path. |
| kernel/cmd/server.go | Updates auth manager/handler import paths. |
| kernel/cmd/embedding_test.go | Adds // Packages separator comment in imports. |
| kernel/cmd/connector_test.go | Updates OIDC import path. |
| kernel/cmd/channel_test.go | Adds explicit import alias for require. |
| heartbeat/manager/list.go | Updates auth schema import path. |
| go.sum | Updates checksums to match dependency upgrades (adds mutablelogic/go-auth). |
| go.mod | Replaces djthorpe/go-auth with mutablelogic/go-auth and bumps multiple dependency versions. |
Comments suppressed due to low confidence (4)
kernel/manager/ask.go:99
- Ask supports resolving providers/models without a user (see comments about "all candidates if no user is provided"), but the usage insert unconditionally dereferences
user.Sub. Ifusercan be nil, this will panic; consider using a zero UUID for anonymous usage attribution or skipping usage insertion when no user is present.
// Insert the usage into the database if we have usage information
if response.Usage != nil {
if _, err := m.CreateUsage(ctx, schema.UsageInsert{
Type: schema.UsageTypeAsk,
User: uuid.UUID(user.Sub),
Model: model.Name,
Provider: types.Ptr(model.OwnedBy),
UsageMeta: types.Value(response.Usage),
}); err != nil {
return nil, err
kernel/manager/embedder.go:128
- Embedding resolves providers with
useroptional, but the usage insert unconditionally dereferencesuser.Sub. This will panic whenuseris nil; either attribute anonymous usage with a zero UUID (uuid.Nil) or skip inserting a usage record when no user is provided.
if response.Usage != nil {
if _, err := m.CreateUsage(ctx, schema.UsageInsert{
Type: schema.UsageTypeEmbedding,
User: uuid.UUID(user.Sub),
Model: model.Name,
Provider: types.Ptr(model.OwnedBy),
UsageMeta: types.Value(response.Usage),
}); err != nil {
return nil, err
kernel/manager/chat.go:479
- conversationForSession unconditionally dereferences
user.Subwhen creating the query context. Since sessions are documented to support a nil user (user-less sessions), this will panic when loading conversation history for anonymous sessions. Handleuser == nilexplicitly (e.g., use a zero/unscoped user context).
func (m *Manager) conversationForSession(ctx context.Context, session uuid.UUID, user *auth.UserInfo) (schema.Conversation, error) {
conn := m.PoolConn.With("session", session, "user", user.Sub)
req := schema.MessageListRequest{}
conversation := make(schema.Conversation, 0)
for {
var page schema.MessageList
if err := conn.List(ctx, &page, req); err != nil {
return nil, pg.NormalizeError(err)
}
kernel/manager/chat.go:214
- executeConversationTurn unconditionally converts/dereferences
user.Subwhen building theUsageInsert. If chat endpoints are callable without authentication (middleware may return nil), this will panic and prevent persisting usage for anonymous sessions. Guarduser == niland use a zero UUID (or skip usage insertion) when no user is present.
if turn.Usage != nil {
turn.UsageEntry = &schema.UsageInsert{
Type: schema.UsageTypeChat,
User: uuid.UUID(user.Sub),
Session: session,
Model: model.Name,
Provider: types.Ptr(model.OwnedBy),
UsageMeta: types.Value(turn.Usage),
}
Comment on lines
33
to
47
| // All work runs in a single transaction: parent fetch + ownership check + | ||
| // meta merge + provider/model resolution + insert. | ||
| var result schema.Session | ||
| if err := m.PoolConn.Tx(ctx, func(conn pg.Conn) error { | ||
| conn = conn.With("user", user.UUID()) | ||
| conn = conn.With("user", user.Sub) | ||
|
|
||
| // If a parent session is provided, fetch it (no user filter — we check | ||
| // ownership explicitly), then merge generator settings as defaults. | ||
| if req.Parent != uuid.Nil { | ||
| var parent schema.Session | ||
| if err := conn.With("user", uuid.UUID{}).Get(ctx, &parent, schema.SessionIDSelector(req.Parent)); err != nil { | ||
| return normalizeSessionError(req.Parent, err) | ||
| } | ||
| if parent.User != user.UUID() { | ||
| if parent.User != uuid.UUID(user.Sub) { | ||
| return httpresponse.ErrForbidden.With("parent session belongs to another user") |
Comment on lines
79
to
82
| // Get the session - if user is provided, ensure session belongs to that user. | ||
| var result schema.Session | ||
| if err := m.PoolConn.With("user", user.UUID()).Get(ctx, &result, schema.SessionIDSelector(session)); err != nil { | ||
| if err := m.PoolConn.With("user", user.Sub).Get(ctx, &result, schema.SessionIDSelector(session)); err != nil { | ||
| return nil, normalizeSessionError(session, err) |
Comment on lines
34
to
40
| if _, err := m.GetSession(ctx, session, user); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| result := schema.MessageList{MessageListRequest: req} | ||
| if err := m.PoolConn.With("user", user.UUID()).List(ctx, &result, req); err != nil { | ||
| if err := m.PoolConn.With("user", user.Sub).List(ctx, &result, req); err != nil { | ||
| return nil, pg.NormalizeError(err) |
Comment on lines
101
to
107
| // Fetch, merge GeneratorMeta, and update in one transaction. | ||
| var result schema.Session | ||
| if err := m.PoolConn.Tx(ctx, func(conn pg.Conn) error { | ||
| conn = conn.With("user", user.UUID()) | ||
| conn = conn.With("user", user.Sub) | ||
| var existing schema.Session | ||
| if err := conn.Get(ctx, &existing, schema.SessionIDSelector(session)); err != nil { | ||
| return normalizeSessionError(session, err) |
Comment on lines
143
to
146
| // Delete the session - if user is provided, ensure session belongs to that user. | ||
| var result schema.Session | ||
| if err := m.PoolConn.With("user", user.UUID()).Delete(ctx, &result, schema.SessionIDSelector(session)); err != nil { | ||
| if err := m.PoolConn.With("user", user.Sub).Delete(ctx, &result, schema.SessionIDSelector(session)); err != nil { | ||
| return nil, normalizeSessionError(session, err) |
Comment on lines
166
to
168
| result := schema.SessionList{SessionListRequest: req} | ||
| if err := m.PoolConn.With("user", user.UUID()).List(ctx, &result, req); err != nil { | ||
| if err := m.PoolConn.With("user", user.Sub).List(ctx, &result, req); err != nil { | ||
| return nil, pg.NormalizeError(err) |
| // Packages | ||
| authmanager "github.com/djthorpe/go-auth/pkg/authmanager" | ||
| authhanders "github.com/djthorpe/go-auth/pkg/httphandler/authmanager" | ||
| authhanders "github.com/mutablelogic/go-auth/auth/httphandler" |
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
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.
No description provided.