dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 2 Packages Projects Releases Wiki Activity

refactor(handlers): use GetOrEnsureExecution for workspace ops (#786)

Browse Source 
* refactor(handlers): use GetOrEnsureExecution for workspace ops

Per CLAUDE.md, workspace-oriented handlers should use GetOrEnsureExecution
so they survive backend restarts. Two git ops in this file (wsGitCommits,
wsCumulativeDiff) already followed this pattern; the shared helper and the
shell.subscribe handler did not.

Changes:
- git_handlers.getAgentCtlClient: now takes ctx and uses GetOrEnsureExecution.
  Propagates to all 14 git ops routing through the helper (pull, push, rebase,
  merge, abort, commit, stage, unstage, discard, createPR, revertCommit,
  renameBranch, reset, commitDiff).
- shell_handlers.wsShellSubscribe: matches wsShellStatus in the same file.

Left alone (intentional):
- shell_handlers.wsShellInput: the 5s workspace-stream wait is too short to
  absorb a cold-start of agentctl, so we still require an existing execution.
- passthrough_handlers: stdin/resize require a live agent process per the rule.
- git_handlers post-success/failure callbacks: just look up TaskID; ensuring
  would be wasteful.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* test(handlers): cover wsShellSubscribe no-instance path

Mirrors TestWsShellInput_NoInstanceFound. Closes a coverage gap flagged in
PR review — the handler was switched to GetOrEnsureExecution but had no
direct tests for the lookup-failure path.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
João Salavisa
2026-05-02 22:57:13 +01:00
committed by GitHub
parent 58fb9ffb0f
commit eee547d891
3 changed files with 46 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/agent/handlers/git_handlers.go
+21 -19
View File
@@ -247,7 +247,7 @@ func (h *GitHandlers) wsPull(ctx context.Context, msg *ws.Message) (*ws.Message,
return nil, fmt.Errorf("session_id is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -272,7 +272,7 @@ func (h *GitHandlers) wsPush(ctx context.Context, msg *ws.Message) (*ws.Message,
return nil, fmt.Errorf("session_id is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -300,7 +300,7 @@ func (h *GitHandlers) wsRebase(ctx context.Context, msg *ws.Message) (*ws.Messag
return nil, fmt.Errorf("base_branch is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -328,7 +328,7 @@ func (h *GitHandlers) wsMerge(ctx context.Context, msg *ws.Message) (*ws.Message
return nil, fmt.Errorf("base_branch is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -356,7 +356,7 @@ func (h *GitHandlers) wsAbort(ctx context.Context, msg *ws.Message) (*ws.Message
return nil, fmt.Errorf("operation must be 'merge' or 'rebase'")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -383,7 +383,7 @@ func (h *GitHandlers) wsCommit(ctx context.Context, msg *ws.Message) (*ws.Messag
return nil, fmt.Errorf("message is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -411,7 +411,7 @@ func (h *GitHandlers) wsRenameBranch(ctx context.Context, msg *ws.Message) (*ws.
return nil, fmt.Errorf("new_name is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -445,7 +445,7 @@ func (h *GitHandlers) wsReset(ctx context.Context, msg *ws.Message) (*ws.Message
return nil, fmt.Errorf("invalid reset mode: %s (must be soft, mixed, or hard)", req.Mode)
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -469,7 +469,7 @@ func (h *GitHandlers) wsStage(ctx context.Context, msg *ws.Message) (*ws.Message
return nil, fmt.Errorf("session_id is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -493,7 +493,7 @@ func (h *GitHandlers) wsUnstage(ctx context.Context, msg *ws.Message) (*ws.Messa
return nil, fmt.Errorf("session_id is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -521,7 +521,7 @@ func (h *GitHandlers) wsDiscard(ctx context.Context, msg *ws.Message) (*ws.Messa
return nil, fmt.Errorf("paths are required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -548,7 +548,7 @@ func (h *GitHandlers) wsCreatePR(ctx context.Context, msg *ws.Message) (*ws.Mess
return nil, fmt.Errorf("title is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -594,7 +594,7 @@ func (h *GitHandlers) wsRevertCommit(ctx context.Context, msg *ws.Message) (*ws.
return nil, fmt.Errorf("commit_sha is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
return nil, err
}
@@ -622,7 +622,7 @@ func (h *GitHandlers) wsCommitDiff(ctx context.Context, msg *ws.Message) (*ws.Me
return nil, fmt.Errorf("commit_sha is required")
}
client, err := h.getAgentCtlClient(req.SessionID)
client, err := h.getAgentCtlClient(ctx, req.SessionID)
if err != nil {
if isSessionNotReadyError(err) {
return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{
@@ -652,11 +652,13 @@ func isSessionNotReadyError(err error) bool {
strings.Contains(msg, "agent client not available for session")
}
// getAgentCtlClient gets the agentctl client for a session
func (h *GitHandlers) getAgentCtlClient(sessionID string) (*client.Client, error) {
execution, ok := h.lifecycleMgr.GetExecutionBySessionID(sessionID)
if !ok {
return nil, fmt.Errorf("no agent running for session %s", sessionID)
// getAgentCtlClient gets the agentctl client for a session.
// Uses GetOrEnsureExecution so git operations survive backend restarts —
// they're workspace-oriented and don't require a running agent process.
func (h *GitHandlers) getAgentCtlClient(ctx context.Context, sessionID string) (*client.Client, error) {
execution, err := h.lifecycleMgr.GetOrEnsureExecution(ctx, sessionID)
if err != nil {
return nil, fmt.Errorf("no agent running for session %s: %w", sessionID, err)
}
c := execution.GetAgentCtlClient()
apps/backend/internal/agent/handlers/shell_handlers.go
+8 -5
View File
@@ -130,10 +130,10 @@ func (h *ShellHandlers) wsShellSubscribe(ctx context.Context, msg *ws.Message) (
return nil, fmt.Errorf("session_id is required")
}
// Verify the agent execution exists for this session
execution, ok := h.lifecycleMgr.GetExecutionBySessionID(req.SessionID)
if !ok {
return nil, fmt.Errorf("no agent running for session %s", req.SessionID)
// Get or create execution on-demand (survives backend restart)
execution, err := h.lifecycleMgr.GetOrEnsureExecution(ctx, req.SessionID)
if err != nil {
return nil, fmt.Errorf("no agent running for session %s: %w", req.SessionID, err)
}
// Get buffered output to include in response
@@ -164,7 +164,10 @@ func (h *ShellHandlers) wsShellInput(ctx context.Context, msg *ws.Message) (*ws.
return nil, fmt.Errorf("session_id is required")
}
// Get the agent execution for this session
// Get the agent execution for this session.
// Use GetExecutionBySessionID (not GetOrEnsureExecution): writing input
// requires the workspace stream to already be live, and the 5s wait below
// is too short to absorb a cold-start of agentctl.
execution, ok := h.lifecycleMgr.GetExecutionBySessionID(req.SessionID)
if !ok {
return nil, fmt.Errorf("no agent running for session %s", req.SessionID)
apps/backend/internal/agent/handlers/shell_handlers_test.go
+17
View File
@@ -242,6 +242,23 @@ func TestWsShellStatus_NoClientAvailable(t *testing.T) {
}
}
func TestWsShellSubscribe_NoInstanceFound(t *testing.T) {
log := newTestLogger()
mgr := newTestManager()
handlers := NewShellHandlers(mgr, nil, log)
msg, _ := ws.NewRequest("test-1", ws.ActionShellSubscribe, ShellSubscribeRequest{SessionID: "session-1"})
_, err := handlers.wsShellSubscribe(context.Background(), msg)
if err == nil {
t.Fatal("expected error for non-existent session")
}
expectedPrefix := "no agent running for session session-1"
if !strings.HasPrefix(err.Error(), expectedPrefix) {
t.Errorf("expected prefix '%s', got: %v", expectedPrefix, err)
}
}
func TestWsShellInput_NoInstanceFound(t *testing.T) {
log := newTestLogger()
mgr := newTestManager()