stacklok / stacklok/toolhive

Delete the unreachable legacy optimizer decorator path in sessionmanager

Open Beginner friendly
#6,103 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

needs-triage
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

Context

#6033 (review follow-through) adds a constructor guard in sessionmanager.New: configuring an optimizer (FactoryConfig.OptimizerFactory or OptimizerConfig) without AdvertiseFromCore is now rejected at startup. Previously that configuration installed the optimizer decorator (factory.go, the optimizerFactory != nil && !cfg.AdvertiseFromCore branch) but the Serve layer discarded its advertised tools (serve_optimizer.go returns raw core tools when s.optimizerFactory == nil) — the decorator indexed the FTS5 store (including an embedding round-trip per tool when configured) and served no one, and the Modern capability gate failed open.

What to do

With the guard in place, the !cfg.AdvertiseFromCore optimizer branch in pkg/vmcp/server/sessionmanager/factory.go is unreachable. Delete:

  • optimizerDecoratorFn (factory.go:160-178)
  • the && !cfg.AdvertiseFromCore condition and the branch it guards (factory.go:151)
  • adaptToolsForFactory and the optimizerdec import, if nothing else uses them

and simplify any comments that still describe the "legacy decorator" composition path. Kept out of #6033 to avoid growing an already-XL diff with a cascading deletion.

Blocked on #6033 landing. Refs #6089.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in pkg/vmcp/server/sessionmanager/factory.go and inspect optimizerDecoratorFn, the !cfg.AdvertiseFromCore branch, adaptToolsForFactory, and their optimizerdec references. Confirm that #6033 has landed, remove the unreachable legacy decorator path and stale comments, then verify the sessionmanager package still builds and its existing tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.