Skip to content

Commit d954822

Browse files
tumbergerclaude
andcommitted
fix: address code review findings
- Critical: teardown now runs on non-zero agent exit (os.Exit moved after cleanup, signal goroutine properly closed) - Discovery endpoint cached after first call, uses context - Token expiry floor prevents hot-loop on short-lived tokens - Remove dead code: initTemplate, startTime, traceID, unused httpClient - Fix os.Getenv("GOOS") → runtime.GOOS - README: fix stale REST endpoint reference - Use http.DefaultTransport explicitly Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c24f3b5 commit d954822

4 files changed

Lines changed: 58 additions & 82 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ Session lifecycle and tool call events flow to the Kontext backend. This powers
111111
| `hook.post_tool_call` | PostToolUse hook | After every tool execution |
112112
| `hook.user_prompt` | UserPromptSubmit hook | User submits a prompt |
113113

114-
Events are ingested to the `mcp_events` table via `POST /api/v1/mcp-events`. Each session gets a `traceId` for grouping events in the traces view.
114+
Events are streamed to the backend via the ConnectRPC `ProcessHookEvent` bidirectional stream and stored in the `mcp_events` table.
115115

116116
**What governance telemetry captures:**
117117
- What the agent tried to do (tool name + input)

internal/backend/backend.go

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -73,23 +73,21 @@ func envOr(key, fallback string) string {
7373

7474
// Client wraps the ConnectRPC AgentService client with token management.
7575
type Client struct {
76-
rpc agentv1connect.AgentServiceClient
77-
config *Config
78-
token string
79-
tokenExp time.Time
80-
mu sync.Mutex
76+
rpc agentv1connect.AgentServiceClient
77+
config *Config
78+
token string
79+
tokenExp time.Time
80+
tokenEndpoint string
81+
mu sync.Mutex
8182
}
8283

8384
// NewClient creates a ConnectRPC client for the Kontext AgentService.
8485
func NewClient(config *Config) *Client {
85-
httpClient := &http.Client{Timeout: 30 * time.Second}
86-
8786
c := &Client{config: config}
8887

89-
// Wrap the HTTP client with an auth interceptor
9088
authClient := &http.Client{
9189
Timeout: 30 * time.Second,
92-
Transport: &authTransport{client: c, base: httpClient.Transport},
90+
Transport: &authTransport{client: c, base: http.DefaultTransport},
9391
}
9492

9593
c.rpc = agentv1connect.NewAgentServiceClient(
@@ -153,22 +151,14 @@ func (c *Client) getToken(ctx context.Context) (string, error) {
153151
return c.token, nil
154152
}
155153

156-
// Discover token endpoint
157-
resp, err := http.Get(c.config.BaseURL + "/.well-known/oauth-authorization-server")
154+
// Discover token endpoint (cached after first call)
155+
tokenEndpoint, err := c.discoverTokenEndpoint(ctx)
158156
if err != nil {
159-
return "", fmt.Errorf("discovery: %w", err)
160-
}
161-
defer resp.Body.Close()
162-
163-
var meta struct {
164-
TokenEndpoint string `json:"token_endpoint"`
165-
}
166-
if err := json.NewDecoder(resp.Body).Decode(&meta); err != nil {
167-
return "", fmt.Errorf("decode discovery: %w", err)
157+
return "", err
168158
}
169159

170160
// Client credentials flow
171-
req, err := http.NewRequestWithContext(ctx, "POST", meta.TokenEndpoint,
161+
req, err := http.NewRequestWithContext(ctx, "POST", tokenEndpoint,
172162
strings.NewReader("grant_type=client_credentials&scope=management:all+mcp:invoke"))
173163
if err != nil {
174164
return "", err
@@ -195,15 +185,43 @@ func (c *Client) getToken(ctx context.Context) (string, error) {
195185
}
196186

197187
c.token = tokenData.AccessToken
198-
if tokenData.ExpiresIn > 0 {
199-
c.tokenExp = time.Now().Add(time.Duration(tokenData.ExpiresIn-60) * time.Second)
200-
} else {
201-
c.tokenExp = time.Now().Add(50 * time.Minute)
188+
bufferSec := tokenData.ExpiresIn - 60
189+
if bufferSec < 10 {
190+
bufferSec = 10
202191
}
192+
c.tokenExp = time.Now().Add(time.Duration(bufferSec) * time.Second)
203193

204194
return c.token, nil
205195
}
206196

197+
func (c *Client) discoverTokenEndpoint(ctx context.Context) (string, error) {
198+
if c.tokenEndpoint != "" {
199+
return c.tokenEndpoint, nil
200+
}
201+
202+
req, err := http.NewRequestWithContext(ctx, "GET",
203+
c.config.BaseURL+"/.well-known/oauth-authorization-server", nil)
204+
if err != nil {
205+
return "", err
206+
}
207+
208+
resp, err := http.DefaultClient.Do(req)
209+
if err != nil {
210+
return "", fmt.Errorf("discovery: %w", err)
211+
}
212+
defer resp.Body.Close()
213+
214+
var meta struct {
215+
TokenEndpoint string `json:"token_endpoint"`
216+
}
217+
if err := json.NewDecoder(resp.Body).Decode(&meta); err != nil {
218+
return "", fmt.Errorf("decode discovery: %w", err)
219+
}
220+
221+
c.tokenEndpoint = meta.TokenEndpoint
222+
return c.tokenEndpoint, nil
223+
}
224+
207225
// authTransport injects the bearer token into every request.
208226
type authTransport struct {
209227
client *Client

internal/run/run.go

Lines changed: 13 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ import (
99
"os/exec"
1010
"os/signal"
1111
"path/filepath"
12+
"runtime"
1213
"strings"
1314
"syscall"
1415
"time"
1516

1617
"github.com/cli/browser"
17-
"github.com/google/uuid"
1818

1919
agentv1 "github.com/kontext-dev/kontext-cli/gen/kontext/agent/v1"
2020
"github.com/kontext-dev/kontext-cli/internal/auth"
@@ -67,7 +67,7 @@ func Start(ctx context.Context, opts Options) error {
6767
Cwd: cwd,
6868
ClientInfo: map[string]string{
6969
"name": "kontext-cli",
70-
"os": fmt.Sprintf("%s", os.Getenv("GOOS")),
70+
"os": runtime.GOOS,
7171
},
7272
})
7373
if err != nil {
@@ -79,13 +79,11 @@ func Start(ctx context.Context, opts Options) error {
7979
sessionID := createResp.SessionId
8080
fmt.Fprintf(os.Stderr, "✓ Session: %s (%s)\n", createResp.SessionName, sessionID[:8])
8181

82-
traceID := uuid.New().String()
83-
8482
// 4. Start sidecar
8583
sessionDir := filepath.Join(os.TempDir(), "kontext", sessionID)
8684
os.MkdirAll(sessionDir, 0700)
8785

88-
sc, err := sidecar.New(sessionDir, client, sessionID, traceID, opts.Agent)
86+
sc, err := sidecar.New(sessionDir, client, sessionID, opts.Agent)
8987
if err != nil {
9088
return fmt.Errorf("sidecar: %w", err)
9189
}
@@ -123,18 +121,23 @@ func Start(ctx context.Context, opts Options) error {
123121

124122
// 8. Launch agent with hooks
125123
fmt.Fprintf(os.Stderr, "\nLaunching %s...\n\n", opts.Agent)
126-
startTime := time.Now()
127124
agentErr := launchAgentWithSettings(ctx, opts.Agent, env, opts.Args, settingsPath)
128125

129-
// 9. Teardown
130-
_ = time.Since(startTime)
126+
// 9. Teardown (always runs, even on non-zero agent exit)
131127
endCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
132128
defer cancel()
133129

134130
_ = client.EndSession(endCtx, sessionID)
135131
fmt.Fprintf(os.Stderr, "\n✓ Session ended (%s)\n", sessionID[:8])
136132

137133
os.RemoveAll(sessionDir)
134+
135+
// Propagate agent exit code
136+
if agentErr != nil {
137+
if exitErr, ok := agentErr.(*exec.ExitError); ok {
138+
os.Exit(exitErr.ExitCode())
139+
}
140+
}
138141
return agentErr
139142
}
140143

@@ -158,44 +161,6 @@ func ensureSession(ctx context.Context, issuerURL, clientID string) (*auth.Sessi
158161
return result.Session, nil
159162
}
160163

161-
// initTemplate interactively creates a .env.kontext on first run.
162-
func initTemplate(path string) error {
163-
providers := []struct {
164-
Name string
165-
EnvVar string
166-
Handle string
167-
}{
168-
{"GitHub", "GITHUB_TOKEN", "github"},
169-
{"Google Workspace", "GOOGLE_TOKEN", "google-workspace"},
170-
{"Stripe", "STRIPE_KEY", "stripe"},
171-
{"Linear", "LINEAR_API_KEY", "linear"},
172-
{"Slack", "SLACK_TOKEN", "slack"},
173-
{"PostgreSQL", "DATABASE_URL", "postgres"},
174-
}
175-
176-
fmt.Fprintln(os.Stderr, "\nNo .env.kontext found. Which providers does this project need?")
177-
reader := bufio.NewReader(os.Stdin)
178-
179-
var lines []string
180-
for _, p := range providers {
181-
fmt.Fprintf(os.Stderr, " %s? [y/N] ", p.Name)
182-
input, _ := reader.ReadString('\n')
183-
if strings.TrimSpace(strings.ToLower(input)) == "y" {
184-
lines = append(lines, fmt.Sprintf("%s={{kontext:%s}}", p.EnvVar, p.Handle))
185-
}
186-
}
187-
188-
if len(lines) == 0 {
189-
lines = append(lines, "# Add providers: VAR_NAME={{kontext:provider-handle}}")
190-
}
191-
192-
if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")+"\n"), 0644); err != nil {
193-
return fmt.Errorf("write %s: %w", path, err)
194-
}
195-
fmt.Fprintf(os.Stderr, "✓ Wrote %s\n\n", path)
196-
return nil
197-
}
198-
199164
// resolveCredentials exchanges each template entry for a live credential.
200165
func resolveCredentials(ctx context.Context, session *auth.Session, entries []credential.Entry) ([]credential.Resolved, error) {
201166
fmt.Fprintln(os.Stderr, "\nResolving credentials...")
@@ -279,14 +244,9 @@ func launchAgentWithSettings(_ context.Context, agentName string, env, extraArgs
279244

280245
err = cmd.Wait()
281246
signal.Stop(sigCh)
247+
close(sigCh)
282248

283-
if err != nil {
284-
if exitErr, ok := err.(*exec.ExitError); ok {
285-
os.Exit(exitErr.ExitCode())
286-
}
287-
return err
288-
}
289-
return nil
249+
return err
290250
}
291251

292252
func filterArgs(args []string) []string {

internal/sidecar/sidecar.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,16 @@ type Server struct {
2020
socketPath string
2121
listener net.Listener
2222
sessionID string
23-
traceID string
2423
agentName string
2524
client *backend.Client
2625
cancel context.CancelFunc
2726
}
2827

2928
// New creates a new sidecar server.
30-
func New(sessionDir string, client *backend.Client, sessionID, traceID, agentName string) (*Server, error) {
29+
func New(sessionDir string, client *backend.Client, sessionID, agentName string) (*Server, error) {
3130
return &Server{
3231
socketPath: filepath.Join(sessionDir, "kontext.sock"),
3332
sessionID: sessionID,
34-
traceID: traceID,
3533
agentName: agentName,
3634
client: client,
3735
}, nil

0 commit comments

Comments
 (0)