refactor(session): retire legacy runtime surfaces#28
Open
LichKing-2234 wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR retires the legacy session/local-proxy runtime path from the active CLI/backend product surface and updates documentation to describe the sessionless attribution workflow as the formal path.
Changes:
- Removed backend
/sessions*handler/routes and related session lifecycle tests. - Deleted dormant ae-cli proxy, dispatcher, and toolconfig runtime packages; trimmed session/client runtime code.
- Updated README and architecture/cutover docs to reflect the retired legacy runtime.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates current runtime wording around retired session/proxy workflow. |
| docs/architecture.md | Revises runtime flow and module responsibility documentation. |
| docs/ae-cli/session-pr-attribution.md | Deletes the legacy session attribution runbook. |
| docs/superpowers/specs/2026-05-14-legacy-session-staged-cutover-design.md | Updates cutover status and retired-surface list. |
| docs/superpowers/plans/2026-05-14-legacy-session-staged-cutover.md | Adds completed tasks for session route and proxy package deletion. |
| backend/internal/handler/router.go | Removes /sessions route registration. |
| backend/internal/handler/session.go | Deletes session HTTP handler implementation. |
| backend/internal/handler/session_bootstrap_http_test.go | Deletes bootstrap HTTP tests. |
| backend/internal/handler/handler_test.go | Removes session lifecycle handler tests. |
| backend/internal/handler/handler_extended_test.go | Removes session error-path tests. |
| backend/internal/handler/handler_boost_test.go | Removes additional session tests. |
| backend/internal/handler/handler_90_test.go | Removes session coverage tests. |
| backend/internal/handler/handler_final_coverage_test.go | Removes final session coverage tests/imports. |
| ae-cli/README.md | Updates CLI usage docs to sessionless-only workflow. |
| ae-cli/cmd/start.go | Removes internal proxy server command wiring. |
| ae-cli/cmd/shell.go | Replaces shell runtime implementation with retired shim command. |
| ae-cli/internal/client/client.go | Removes session/proxy-specific client request types and methods. |
| ae-cli/internal/client/client_test.go | Removes tests for retired session/proxy client methods. |
| ae-cli/internal/session/manager.go | Reduces manager to local state helpers after removing lifecycle/proxy logic. |
| ae-cli/internal/tmux/tmux_test.go | Adjusts tmux test timing. |
| ae-cli/internal/toolconfig/codex.go | Deletes Codex local-proxy config writer. |
| ae-cli/internal/toolconfig/claude.go | Deletes Claude local-proxy config writer. |
| ae-cli/internal/toolconfig/toolconfig_test.go | Deletes toolconfig tests. |
| ae-cli/internal/proxy/config.go | Deletes proxy runtime config type. |
| ae-cli/internal/proxy/server.go | Deletes local proxy server/process runtime. |
| ae-cli/internal/proxy/openai.go | Deletes OpenAI proxy handling. |
| ae-cli/internal/proxy/anthropic.go | Deletes Anthropic proxy handling. |
| ae-cli/internal/proxy/events.go | Deletes proxy event persistence/forwarding. |
| ae-cli/internal/proxy/hookforward.go | Deletes hook forwarding to local proxy. |
| ae-cli/internal/proxy/hookforward_test.go | Deletes hook forwarding tests. |
| ae-cli/internal/proxy/credentials.go | Deletes proxy credential cache. |
| ae-cli/internal/proxy/credentials_test.go | Deletes proxy credential cache tests. |
| ae-cli/internal/proxy/sysproc_unix.go | Deletes Unix proxy process attributes. |
| ae-cli/internal/proxy/sysproc_windows.go | Deletes Windows proxy process attributes. |
| ae-cli/internal/dispatcher/dispatcher.go | Deletes legacy tool dispatcher. |
| ae-cli/internal/dispatcher/dispatcher_test.go | Deletes dispatcher tests. |
Comments suppressed due to low confidence (4)
docs/ae-cli/session-pr-attribution.md:1
- Deleting this runbook leaves existing documentation references to
docs/ae-cli/session-pr-attribution.mddangling (for example, the current cutover plan still lists it under files to modify, and older specs/plans link to it). Either replace it with a short retirement/redirect stub or update/remove the remaining references so docs navigation does not 404.
backend/internal/handler/handler_final_coverage_test.go:519 - This section header refers to a deleted session test and is now immediately followed by an unrelated efficiency section. Remove the stale header so the test file no longer advertises coverage for a removed
session.gopath.
// =====================
// Additional: session.go Create — repo found by clone_url fallback
// =====================
// =====================
// Additional: efficiency Aggregate with period param
backend/internal/handler/handler_90_test.go:517
- This
Session Stopcoverage header is now orphaned after deleting the session test. Remove the stale header to avoid implying there is still coverage for the retired session handler path.
// =====================
// Session Stop — invalid UUID
// =====================
// =====================
// SCM Provider Update — with status field
backend/internal/handler/handler_90_test.go:716
- The full session lifecycle test was deleted, but this section header remains empty. Please remove the stale marker so the test file does not claim coverage for the retired session lifecycle.
// =====================
// Session — full lifecycle covering more branches
// =====================
// =====================
// PR Get — success path
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+225
to
+246
| - [x] **Step 1: Remove backend `/sessions*` lifecycle/read routes from the public router** | ||
| - [x] **Step 2: Delete session-only handler files and tests that exercised the retired surface** | ||
| - [x] **Step 3: Remove frontend `/sessions` routes, API module, and pages** | ||
| - [x] **Step 4: Re-run backend handler and frontend tests to confirm pass** | ||
|
|
||
| ## Task 12: Delete Dormant Proxy Runtime Packages | ||
|
|
||
| **Files:** | ||
| - Delete: `ae-cli/internal/proxy/*` | ||
| - Delete: `ae-cli/internal/dispatcher/*` | ||
| - Delete: `ae-cli/internal/toolconfig/*` | ||
| - Modify: `ae-cli/internal/client/client.go` | ||
| - Modify: `ae-cli/internal/client/client_test.go` | ||
| - Modify: `ae-cli/internal/session/manager.go` | ||
| - Delete: `ae-cli/internal/session/session_test.go` | ||
| - Modify: `ae-cli/cmd/shell.go` | ||
| - Modify: `ae-cli/internal/tmux/tmux_test.go` | ||
|
|
||
| - [x] **Step 1: Remove client methods and test coverage that existed only for the retired proxy/session runtime** | ||
| - [x] **Step 2: Replace `session.Manager` with minimal local-state helpers so it no longer drags proxy/runtime behavior into builds** | ||
| - [x] **Step 3: Delete dormant proxy, dispatcher, and toolconfig packages** | ||
| - [x] **Step 4: Re-run `ae-cli go test ./...` to confirm the CLI/runtime tree still passes** |
Comment on lines
+243
to
+246
| - [x] **Step 1: Remove client methods and test coverage that existed only for the retired proxy/session runtime** | ||
| - [x] **Step 2: Replace `session.Manager` with minimal local-state helpers so it no longer drags proxy/runtime behavior into builds** | ||
| - [x] **Step 3: Delete dormant proxy, dispatcher, and toolconfig packages** | ||
| - [x] **Step 4: Re-run `ae-cli go test ./...` to confirm the CLI/runtime tree still passes** |
Comment on lines
+176
to
+179
| Codex --> Backend | ||
| Claude --> Backend | ||
| Hooks --> Backend | ||
| Relay --> Backend |
Comment on lines
70
to
73
| analysisHandler := NewAnalysisHandler(analysisService, optimizer, repoService) | ||
| prHandler := NewPRHandler(entClient, repoService, syncService, prAttributionService) | ||
| efficiencyHandler := NewEfficiencyHandler(entClient, aggregator) | ||
| sessionHandler := NewSessionHandler(entClient, sessionBootstrapSvc) | ||
| toolUsageHandler := NewToolUsageHandler(toolusage.NewService(entClient)) |
Comment on lines
158
to
162
| effGroup.POST("/aggregate", auth.RequireAdmin(), efficiencyHandler.Aggregate) | ||
| } | ||
|
|
||
| // Sessions (ae-cli) | ||
| sessionGroup := protected.Group("/sessions") | ||
| { | ||
| sessionGroup.POST("/bootstrap", sessionHandler.Bootstrap) | ||
| sessionGroup.POST("", sessionHandler.Create) | ||
| sessionGroup.PUT("/:id", sessionHandler.Update) | ||
| sessionGroup.POST("/:id/stop", sessionHandler.Stop) | ||
| sessionGroup.POST("/:id/invocations", sessionHandler.AddInvocation) | ||
| } | ||
|
|
||
| toolUsageGroup := protected.Group("/tool-usage-events") | ||
| toolUsageGroup.POST("", toolUsageHandler.Create) |
Comment on lines
22
to
+23
| - The formal CLI workflow is now sessionless: `ae-cli init`, `ae-cli sync`, and `ae-cli doctor`. | ||
| - Legacy `ae-cli start/stop/run/...` commands remain only as hidden compatibility shims that return migration guidance. | ||
| - Legacy `ae-cli start/stop/run/...` session commands and local-proxy runtime are retired. |
| ``` | ||
|
|
||
| Legacy `ae-cli start/stop/run/...` session commands are retired from the formal workflow and kept only as hidden compatibility shims with migration guidance. | ||
| Legacy `ae-cli start/stop/run/...` session commands are retired. Use the sessionless workflow only. |
Comment on lines
22
to
+23
| - The formal CLI workflow is now sessionless: `ae-cli init`, `ae-cli sync`, and `ae-cli doctor`. | ||
| - Legacy `ae-cli start/stop/run/...` commands remain only as hidden compatibility shims that return migration guidance. | ||
| - Legacy `ae-cli start/stop/run/...` session commands and local-proxy runtime are retired. |
Comment on lines
115
to
120
| // ===================== | ||
| // 3. session.go:40 Create — duplicate session ID | ||
| // ===================== | ||
|
|
||
| func TestSessionCreateDuplicateID(t *testing.T) { | ||
| env := setupTestEnv(t) | ||
|
|
||
| // Create provider and repo | ||
| provider, err := env.client.ScmProvider.Create(). | ||
| SetName("dup-gh"). | ||
| SetType("github"). | ||
| SetBaseURL("https://api.github.com"). | ||
| SetCredentials("encrypted"). | ||
| Save(context.Background()) | ||
| if err != nil { | ||
| t.Fatalf("create provider: %v", err) | ||
| } | ||
| _, err = env.client.RepoConfig.Create(). | ||
| SetScmProviderID(provider.ID). | ||
| SetName("dup-repo"). | ||
| SetFullName("org/dup-repo"). | ||
| SetCloneURL("https://github.com/org/dup-repo.git"). | ||
| SetDefaultBranch("main"). | ||
| Save(context.Background()) | ||
| if err != nil { | ||
| t.Fatalf("create repo: %v", err) | ||
| } | ||
|
|
||
| sessionID := "660e8400-e29b-41d4-a716-446655440000" | ||
| createReq := map[string]interface{}{ | ||
| "id": sessionID, | ||
| "repo_full_name": "org/dup-repo", | ||
| "branch": "main", | ||
| } | ||
|
|
||
| // First create should succeed | ||
| w := doRequest(env, "POST", "/api/v1/sessions", createReq) | ||
| if w.Code != http.StatusCreated { | ||
| t.Fatalf("first create: expected 201, got %d: %s", w.Code, w.Body.String()) | ||
| } | ||
|
|
||
| // Second create with same ID should fail | ||
| w = doRequest(env, "POST", "/api/v1/sessions", createReq) | ||
| if w.Code != http.StatusInternalServerError { | ||
| t.Fatalf("duplicate create: expected 500, got %d: %s", w.Code, w.Body.String()) | ||
| } | ||
| } | ||
|
|
||
| // ===================== | ||
| // 4. session.go:110 Stop — invalid UUID | ||
| // ===================== | ||
|
|
||
| func TestSessionStopInvalidUUID(t *testing.T) { | ||
| env := setupTestEnv(t) | ||
| w := doRequest(env, "POST", "/api/v1/sessions/not-a-valid-uuid/stop", nil) | ||
| if w.Code != http.StatusBadRequest { | ||
| t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) | ||
| } | ||
| } | ||
|
|
||
| // ===================== | ||
| // 5. repo.go:99 Update — invalid body (bad JSON) |
Comment on lines
382
to
387
| // ===================== | ||
| // Session AddInvocation — cover more paths | ||
| // ===================== | ||
|
|
||
| func TestSessionAddInvocationWithEndTime(t *testing.T) { | ||
| env := setupTestEnv(t) | ||
|
|
||
| // Create repo + session | ||
| provider, _ := env.client.ScmProvider.Create(). | ||
| SetName("gh").SetType("github"). | ||
| SetBaseURL("https://api.github.com").SetCredentials("enc"). | ||
| Save(context.Background()) | ||
| rc, _ := env.client.RepoConfig.Create(). | ||
| SetScmProviderID(provider.ID).SetName("r").SetFullName("org/r"). | ||
| SetCloneURL("https://github.com/org/r.git").SetDefaultBranch("main"). | ||
| Save(context.Background()) | ||
|
|
||
| sessionReq := map[string]interface{}{ | ||
| "id": "550e8400-e29b-41d4-a716-446655440020", | ||
| "repo_full_name": rc.FullName, | ||
| "branch": "main", | ||
| } | ||
| w := doRequest(env, "POST", "/api/v1/sessions", sessionReq) | ||
| if w.Code != http.StatusCreated { | ||
| t.Fatalf("create session: %d, %s", w.Code, w.Body.String()) | ||
| } | ||
|
|
||
| // Add invocation with end time | ||
| invReq := map[string]interface{}{ | ||
| "tool": "cursor", | ||
| "start": "2026-03-17T10:00:00Z", | ||
| "end": "2026-03-17T10:30:00Z", | ||
| } | ||
| w = doRequest(env, "POST", "/api/v1/sessions/550e8400-e29b-41d4-a716-446655440020/invocations", invReq) | ||
| if w.Code != http.StatusOK { | ||
| t.Errorf("status = %d, want %d, body: %s", w.Code, http.StatusOK, w.Body.String()) | ||
| } | ||
|
|
||
| // Add second invocation | ||
| invReq2 := map[string]interface{}{ | ||
| "tool": "claude-code", | ||
| "start": "2026-03-17T11:00:00Z", | ||
| "end": "2026-03-17T11:15:00Z", | ||
| } | ||
| w = doRequest(env, "POST", "/api/v1/sessions/550e8400-e29b-41d4-a716-446655440020/invocations", invReq2) | ||
| if w.Code != http.StatusOK { | ||
| t.Errorf("status = %d, want %d", w.Code, http.StatusOK) | ||
| } | ||
| } | ||
|
|
||
| // ===================== | ||
| // Repo CreateDirect — success with valid provider |
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.
Summary
Test Plan
cd ae-cli && go test ./... -count=1cd backend && AE_TEST_POSTGRES_DSN='postgres://postgres:postgres@127.0.0.1:15432/postgres?sslmode=disable' go test ./internal/handler -count=1