refactor(session): simplify code reuse and clean up interfaces

- Replace hand-rolled atomic file writes in saveIndexLocked() and
  writeSessionSnapshot() with fileutil.WriteFileAtomic(), gaining
  parent-dir fsync for flash storage durability (+12 existing callers)
- Extract duplicated filename validation into validateSessionFilename()
- Use utils.Truncate() in sessionLabel() instead of inline rune slicing
- Remove unused route parameter from buildCommandsRuntime()
- Remove ResolveActive from SessionOps interface (no handler uses it;
  only the agent loop calls it directly on agent.Sessions)

Net: -68 lines, 0 behavioral changes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
mingmxren 2026-03-06 18:51:12 +08:00
parent e0e2cb0724
commit 0a8d3cea97
4 changed files with 16 additions and 84 deletions

View file

@ -1554,7 +1554,7 @@ func (al *AgentLoop) handleCommand(
return "", false return "", false
} }
rt := al.buildCommandsRuntime(route, agent) rt := al.buildCommandsRuntime(agent)
executor := commands.NewExecutor(al.cmdRegistry, rt) executor := commands.NewExecutor(al.cmdRegistry, rt)
var commandReply string var commandReply string
@ -1584,10 +1584,7 @@ func (al *AgentLoop) handleCommand(
} }
} }
func (al *AgentLoop) buildCommandsRuntime( func (al *AgentLoop) buildCommandsRuntime(agent *AgentInstance) *commands.Runtime {
route routing.ResolvedRoute,
agent *AgentInstance,
) *commands.Runtime {
rt := &commands.Runtime{ rt := &commands.Runtime{
Config: al.cfg, Config: al.cfg,
ListAgentIDs: al.registry.ListAgentIDs, ListAgentIDs: al.registry.ListAgentIDs,

View file

@ -8,6 +8,7 @@ import (
"time" "time"
"github.com/sipeed/picoclaw/pkg/session" "github.com/sipeed/picoclaw/pkg/session"
"github.com/sipeed/picoclaw/pkg/utils"
) )
func sessionCommand() Definition { func sessionCommand() Definition {
@ -109,11 +110,7 @@ func sessionLabel(summary, preview string, maxLen int) string {
return "(empty)" return "(empty)"
} }
text = strings.ReplaceAll(text, "\n", " ") text = strings.ReplaceAll(text, "\n", " ")
runes := []rune(text) return utils.Truncate(text, maxLen)
if len(runes) <= maxLen {
return text
}
return string(runes[:maxLen]) + "..."
} }
// extractSessionTag returns the "#N" suffix from a session key, or "#1" for // extractSessionTag returns the "#N" suffix from a session key, or "#1" for

View file

@ -9,7 +9,6 @@ import (
// rely on. Implementations are expected to be scope-aware and deterministic // rely on. Implementations are expected to be scope-aware and deterministic
// for a given scopeKey. SessionManager satisfies this interface. // for a given scopeKey. SessionManager satisfies this interface.
type SessionOps interface { type SessionOps interface {
ResolveActive(scopeKey string) (string, error)
StartNew(scopeKey string) (string, error) StartNew(scopeKey string) (string, error)
List(scopeKey string) ([]session.SessionMeta, error) List(scopeKey string) ([]session.SessionMeta, error)
Resume(scopeKey string, index int) (string, error) Resume(scopeKey string, index int) (string, error)

View file

@ -12,6 +12,7 @@ import (
"sync" "sync"
"time" "time"
"github.com/sipeed/picoclaw/pkg/fileutil"
"github.com/sipeed/picoclaw/pkg/providers" "github.com/sipeed/picoclaw/pkg/providers"
) )
@ -654,39 +655,7 @@ func (sm *SessionManager) saveIndexLocked() error {
return err return err
} }
tmpFile, err := os.CreateTemp(sm.storage, "index-*.tmp") return fileutil.WriteFileAtomic(sm.indexPath, data, 0o644)
if err != nil {
return err
}
tmpPath := tmpFile.Name()
cleanup := true
defer func() {
if cleanup {
_ = os.Remove(tmpPath)
}
}()
if _, err := tmpFile.Write(data); err != nil {
_ = tmpFile.Close()
return err
}
if err := tmpFile.Chmod(0o644); err != nil {
_ = tmpFile.Close()
return err
}
if err := tmpFile.Sync(); err != nil {
_ = tmpFile.Close()
return err
}
if err := tmpFile.Close(); err != nil {
return err
}
if err := os.Rename(tmpPath, sm.indexPath); err != nil {
return err
}
cleanup = false
return nil
} }
func (sm *SessionManager) ensureScopeLocked(scopeKey string, now time.Time) (*scopeIndex, bool) { func (sm *SessionManager) ensureScopeLocked(scopeKey string, now time.Time) (*scopeIndex, bool) {
@ -855,13 +824,8 @@ func (sm *SessionManager) writeSessionSnapshot(snapshot Session) error {
} }
filename := sanitizeFilename(snapshot.Key) filename := sanitizeFilename(snapshot.Key)
if err := validateSessionFilename(filename); err != nil {
// filepath.IsLocal rejects empty names, "..", absolute paths, and return err
// OS-reserved device names (NUL, COM1 … on Windows).
// The extra checks reject "." and any directory separators so that
// the session file is always written directly inside sm.storage.
if filename == "." || !filepath.IsLocal(filename) || strings.ContainsAny(filename, `/\`) {
return os.ErrInvalid
} }
data, err := json.MarshalIndent(snapshot, "", " ") data, err := json.MarshalIndent(snapshot, "", " ")
@ -870,39 +834,14 @@ func (sm *SessionManager) writeSessionSnapshot(snapshot Session) error {
} }
sessionPath := filepath.Join(sm.storage, filename+".json") sessionPath := filepath.Join(sm.storage, filename+".json")
tmpFile, err := os.CreateTemp(sm.storage, "session-*.tmp") return fileutil.WriteFileAtomic(sessionPath, data, 0o644)
if err != nil { }
return err
}
tmpPath := tmpFile.Name() // validateSessionFilename rejects filenames that would escape sm.storage.
cleanup := true func validateSessionFilename(filename string) error {
defer func() { if filename == "." || !filepath.IsLocal(filename) || strings.ContainsAny(filename, `/\`) {
if cleanup { return os.ErrInvalid
_ = os.Remove(tmpPath)
} }
}()
if _, err := tmpFile.Write(data); err != nil {
_ = tmpFile.Close()
return err
}
if err := tmpFile.Chmod(0o644); err != nil {
_ = tmpFile.Close()
return err
}
if err := tmpFile.Sync(); err != nil {
_ = tmpFile.Close()
return err
}
if err := tmpFile.Close(); err != nil {
return err
}
if err := os.Rename(tmpPath, sessionPath); err != nil {
return err
}
cleanup = false
return nil return nil
} }
@ -912,8 +851,8 @@ func (sm *SessionManager) deleteSessionFile(sessionKey string) error {
} }
filename := sanitizeFilename(sessionKey) filename := sanitizeFilename(sessionKey)
if filename == "." || !filepath.IsLocal(filename) || strings.ContainsAny(filename, `/\`) { if err := validateSessionFilename(filename); err != nil {
return os.ErrInvalid return err
} }
sessionPath := filepath.Join(sm.storage, filename+".json") sessionPath := filepath.Join(sm.storage, filename+".json")