feat: model state in /v1/models, lifecycle API, GET /api/ps and GET /api/models/:model - #755

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@proxy/proxymanager_api.go`:
- Around line 36-37: The GET route for models is brittle with IDs that contain
slashes; change the route patterns from "/models/:model" to a wildcard like
"/models/*model" for both places where you register it (the one using
pm.apiGetModelHandler and the other occurrence at lines ~379-380) and update the
handler to extract and sanitize the wildcard param (e.g., use c.Param("model"),
strip any leading "/" and URL-decode) so IDs like "author/model" are handled
correctly.
- Around line 361-367: apiLoadSingleModelHandler currently calls
findGroupByModelName directly which bypasses swapProcessGroup and therefore
skips exclusive-group eviction and swap semantics; update this handler to use
pm.swapProcessGroup(requestedModel) (or the appropriate swap entrypoint used by
normal routing) to obtain the active processGroup before calling
processGroup.ProxyRequest(realModelName, dw, req), and preserve error handling
via pm.sendErrorResponse if swapProcessGroup returns nil or an error; ensure the
same swap-based path is used as in normal inference routing so exclusive-group
behavior is honored.

In `@proxy/proxymanager.go`:
- Around line 621-622: Alias rows are calling pm.modelProcessState(modelId) with
the alias ID, causing incorrect state/loaded values; change the alias-row logic
(the spots that build alias entries around the pm.modelProcessState call,
including the other occurrence at the block around lines 655–659) to resolve the
alias to its canonical model ID first and then call
pm.modelProcessState(canonicalID) so runtime state and loaded are computed for
the canonical model instead of the alias.
- Around line 602-613: The switch over process.CurrentState() currently maps
known states but returns "stopped" in the default, which masks unexpected
values; change the default branch in the switch (around process.CurrentState())
to return "unknown", false instead of "stopped", false so unmapped/new process
states are explicitly reported as "unknown".

---

Nitpick comments:
In `@proxy/proxymanager_test.go`:
- Around line 1947-2061: Add test coverage for alias resolution and
slash-delimited model IDs for the load and detail endpoints: extend or add to
TestProxyManager_LoadSingleModel (and its NotFound variant) and
TestProxyManager_ModelDetail to include models defined with an "alias" (or "id"
mapping) and a model whose key contains a slash; send POST requests to
/api/models/load/<alias> and /api/models/load/<slash/id> and GET requests to
/api/models/<alias> and /api/models/<slash/id>, then assert the same behaviors
as existing checks (HTTP 200 + state transitions for load, 200 with correct
metadata for detail, and 404 for unknown aliases/paths). Reference the existing
test functions (TestProxyManager_LoadSingleModel,
TestProxyManager_LoadSingleModel_NotFound, TestProxyManager_ModelDetail) and the
POST /api/models/load/:model and GET /api/models/:model routes when locating
where to add these assertions.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@proxy/proxymanager_models_test.go`:
- Around line 16-33: Rename the test functions so they follow the project's
TestProxyManager_<name> convention: change TestParseModelPath to
TestProxyManager_parseModelPath and likewise rename TestResolveHFSource,
TestAPIGetStorage, TestAPIGetStorageInferred, TestAPIPullModel,
TestAPIPullModelGated, TestAPIDeleteModel, and TestAPIDeleteModelNotFound to
TestProxyManager_resolveHFSource, TestProxyManager_apiGetStorage,
TestProxyManager_apiGetStorageInferred, TestProxyManager_apiPullModel,
TestProxyManager_apiPullModelGated, TestProxyManager_apiDeleteModel, and
TestProxyManager_apiDeleteModelNotFound respectively so they group with other
ProxyManager tests (update any references/imports if needed).

In `@proxy/proxymanager_models.go`:
- Around line 178-193: The stream handler currently locks HTTP 200 on the first
ndjson write and ignores write/flush errors; update the logic so when
upstream/gated response status is known (before sendJSON is first called) you
call c.Writer.WriteHeader(upstreamStatusCode) (or set the appropriate Gin
response status) before any c.Writer.Write/Flush to ensure the real HTTP status
is sent, and for all c.Writer.Write and c.Writer.Flush calls check their
returned error and abort/return (or close the stream) on error or if ctx.Done is
triggered; modify sendJSON (and any callers) to accept/return an error so
callers can stop on write failures and ensure non-stream paths also set
c.Status(...) before writing body.
- Around line 238-247: The current path-traversal check using strings.HasPrefix
on filepath.Clean(dest) vs filepath.Clean(dir) is unsafe; replace it with a
robust boundary check using filepath.Rel: compute rel, err :=
filepath.Rel(filepath.Clean(dir), filepath.Clean(dest)) and treat any err or any
rel that equals "."? no — treat any err or where rel starts with ".." or
contains ".." (path traversal) as invalid; keep the existing error handling
(sendJSON(pullProgress{...}) and c.JSON(...) when !stream) and reference the
variables dest, dir, filename and functions sendJSON, pullProgress, stream,
c.JSON when applying the change.
- Line 11: The file currently imports and uses Unix-only syscall types
(syscall.Statfs, syscall.Statfs_t) in apiGetStorage which breaks Windows builds;
split the platform-specific implementation: move the Statfs-based logic into a
new proxymanager_models_unix.go with a //go:build unix tag and implement a
helper used by apiGetStorage (e.g., getFilesystemStats or similar) that uses
syscall.Statfs/Statfs_t, and add proxymanager_models_windows.go with //go:build
windows that implements the same helper but returns an error or a
501-not-implemented result; update apiGetStorage to call this helper so builds
on both platforms succeed without direct syscall imports on Windows.
- Around line 62-74: modelsDir() currently iterates pm.config.Models (a map)
which is order-dependent; change it to iterate model IDs in a stable, sorted
order (same sorting used in config.go at load time): collect keys from
pm.config.Models, sort them (e.g. sort.Strings), then iterate those keys and
call parseModelPath(mc.Cmd) on pm.config.Models[id] to infer the directory;
update ProxyManager.modelsDir to return filepath.Dir(p) from the first
resolvable model in that sorted order so the result is deterministic.
- Around line 35-60: Replace the local sanitizeCommand implementation with a
call to the shared config.SanitizeCommand: remove the local sanitizeCommand
function and in parseModelPath (and any callers) call
config.SanitizeCommand(cmd) instead, handling the returned ([]string, error)
properly (i.e., check and return/log the error from config.SanitizeCommand
rather than assuming nil). Also update imports to reference the config package
and remove any now-dead error-free assumptions (delete the existing dead err !=
nil branch that expected a local error behavior).

---

Duplicate comments:
In `@proxy/proxymanager_api.go`:
- Around line 36-37: Routes for GET and DELETE use :model which breaks on model
IDs containing slashes; change the route definitions
apiGroup.GET("/models/:model", pm.apiGetModelHandler) and
apiGroup.DELETE("/models/:model", pm.apiDeleteModel) to use wildcards
("/models/*model") to match slash-containing IDs, and update the handlers
apiGetModelHandler and apiDeleteModel to normalize the param by trimming any
leading '/' from c.Param("model") (or equivalent) before lookup so the rest of
the code receives the same model ID format as load/unload handlers.
- Around line 361-369: The load handler currently bypasses the
group-swap/eviction logic by calling findGroupByModelName; replace that lookup
with the same swap path used by normal routing: call swapProcessGroup(pm,
realModelName, c.Request.Context() or existing ctx) to obtain the processGroup
(fall back to pm.matrix.ProxyRequest when pm.matrix != nil as before), check for
nil and return the same error via pm.sendErrorResponse, then call
processGroup.ProxyRequest(realModelName, dw, req); ensure you preserve existing
error handling and semantics of swapProcessGroup (exclusive-group eviction)
rather than using findGroupByModelName.

---

Nitpick comments:
In `@proxy/proxymanager_models_test.go`:
- Around line 191-216: Update TestAPIPullModelGated to decode and assert the
JSON error envelope from the response body after pm.ServeHTTP instead of only
checking status codes: read w.Body, unmarshal into a struct with an Error string
(matching the apiPullModel response shape), and assert that the error message
contains the gated-token hint (e.g., contains "gated" or "token" / "provide a
token"); keep the existing status code check but add this body assertion in
TestAPIPullModelGated to catch regressions that change or swallow the gated
hint.
- Around line 142-188: Add a streaming test variant to TestAPIPullModel that
omits the "stream" field (so pullRequest.Stream defaults to true) or explicitly
sets it true; call pm.ServeHTTP with the same model URL, read the response body
as newline-delimited JSON (ndjson) and parse each JSON line emitted by sendJSON,
asserting you see at least one "downloading" (progress) event and a final
"success" event (and that the file was written). Use the same helpers
(CreateTestResponseRecorder, New config with ModelsDir, pm.StopProcesses) and
verify the periodic progress/framing behavior and locked-200 terminal behavior
by checking event types/fields rather than a single JSON object.

In `@proxy/proxymanager_models.go`:
- Around line 299-308: The code currently ignores f.Close() before promoting the
temp file; capture the result of f.Sync() (optional for durability) and
f.Close(), check for errors, and treat them the same way you handle the
os.Rename failure: remove tmp, call sendJSON(pullProgress{Status: "error",
Error: err.Error()}), respond with c.JSON(...) when !stream, and return; update
the block around f.Close(), os.Rename, tmp, dest and use the same error handling
path so a failed flush/close prevents renaming the truncated file.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@proxy/proxymanager_models.go`:
- Around line 355-360: The current delete path swallows errors from StopProcess,
which can leave the model serving from an unlinked inode; update the handler so
that after calling pm.matrix.StopProcess(realModelName, StopImmediately) or
pg.StopProcess(realModelName, StopImmediately) you check the returned error and
only ignore it if it represents a “not running”/already-stopped condition, but
for any other error propagate/return that error and abort the delete; use the
same error inspection logic for both pm.matrix.StopProcess and pg.StopProcess
and reference pm.findGroupByModelName to locate the pg branch.
- Around line 121-129: The current branch accepts any http(s) URL (variable
model) which permits SSRF and leaking HF_TOKEN; update the logic in the
URL-handling block (where model is parsed and filename/downloadURL are set) to
parse the URL using net/url, require scheme == "https", and validate the host
against a whitelist of HuggingFace hosts (e.g., exact matches and allowed
subdomains such as *.huggingface.co and huggingface.co); if the host is not
allowed, reject the URL path instead of setting downloadURL/filename. Also
ensure code that forwards the bearer token (where token / HF_TOKEN is applied)
only attaches Authorization for validated HuggingFace hosts (and do the same fix
for the other similar URL-handling location referenced).
- Around line 317-320: The code currently calls f.Close() but ignores its error
before os.Rename(tmp, dest), which can publish a truncated file; capture and
check the error returned by f.Close() (e.g., err = f.Close()), and if non-nil
remove the tmp file (os.Remove(tmp)) and return/propagate the error instead of
proceeding to os.Rename; only call os.Rename(tmp, dest) when Close succeeded,
preserving existing cleanup on failure.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@proxy/proxymanager_api.go`:
- Line 43: Change the DELETE route to accept wildcard IDs with slashes by
updating the route registration (apiGroup.DELETE "/config/models/:id" -> use a
wildcard form like "/config/models/*id") and modify the handler
pm.apiConfigRemoveModel to read the parameter with
strings.TrimPrefix(c.Param("id"), "/") so IDs like "author/model" are extracted
correctly; ensure any downstream uses of the id variable expect the decoded
value without a leading slash.

In `@proxy/proxymanager_config.go`:
- Around line 14-27: The model ID in configModelRequest is only checked for
non-empty but can contain problematic chars; in apiConfigAddModel validate
req.ID against a safe regex (e.g. ^[A-Za-z0-9._:-]+$) before persisting to the
models map, normalize/truncate if needed, and return a 400 error when validation
fails so invalid IDs never get written; reference the configModelRequest struct
and the apiConfigAddModel handler to locate where to add this validation logic
around req.ID.
- Around line 93-109: In apiConfigRemoveModel, RealModelName(id) currently
discards the resolved name and you pass the original alias to
removeModelFromConfig, so deletion is a no-op; capture the resolved real model
name (e.g., realName, found := pm.config.RealModelName(id)), use that realName
when calling pm.removeModelFromConfig(realName), and return the realName in the
success JSON (and keep pm.triggerReload() as-is) so the YAML models map is
deleted by its actual key.
- Around line 208-247: The buildCmd function currently iterates pm.config.Models
nondeterministically and calls the local sanitizeCommand; update buildCmd to
first collect the model IDs/keys from pm.config.Models, sort them
(lexicographically) and then iterate in that deterministic order to pick the
template, and replace calls to sanitizeCommand with the centralized
pm.config.SanitizeCommand (or config.SanitizeCommand) to address the
sanitizeCommand duplication; keep the rest of the logic (skipping/replacing
model args and fallback fmt.Sprintf) unchanged so rebuilt commands are stable
across runs.

In `@proxy/proxymanager_models.go`:
- Around line 224-231: sendJSON currently swallows errors from json.Marshal,
c.Writer.Write and c.Writer.Flush which lets downloads continue after clients
disconnect; change sendJSON (the closure taking pullProgress and using stream,
pullProgress and c.Writer) to return error, check and return any json.Marshal,
Write or Flush error, and in the outer read loop check c.Request.Context().Err()
and the error returned from sendJSON and break the loop on first non-nil error
so the upstream read stops promptly; propagate that error back to the caller so
the read loop can stop immediately on client disconnect.
- Around line 147-150: After computing filename := filepath.Base(clean) (and
before assigning downloadURL/model return), validate the derived filename and
reject invalid values: if filename == "" || filename == "." || filename == ".."
|| strings.ContainsRune(filename, os.PathSeparator) then return an error (or nil
plus a descriptive error) instead of proceeding. Use the filename variable,
filepath.Base(clean) and os.PathSeparator/strings.ContainsRune to implement the
guard so we never create files named "."/".." or with path separators.
- Around line 350-379: The code currently ignores req.Register when
pm.configFile == "" and silently drops the registration; change the branch that
checks req.Register to detect when pm.configFile is empty and surface a failure:
if req.Register != nil && pm.configFile == "" then call sendJSON with
pullProgress{Status: "register_failed", Error: "no config file set"} (and in the
non-stream response path return a 422 error) instead of proceeding silently;
keep existing behavior for the normal write path using pm.writeModelToConfig,
sendJSON, and pm.triggerReload.

---

Nitpick comments:
In `@proxy/proxymanager_config.go`:
- Around line 113-120: The handler currently replies 200 immediately and
launches pm.reloadFn() in a goroutine so callers can’t learn if the reload
actually failed; change apiConfigReload to run the reload synchronously (call
pm.reloadFn() directly), adjust reloadFn to return an error (or wrap its result)
and if it returns non-nil use pm.sendErrorResponse to return an appropriate HTTP
error, only returning the 200 {"status":"reloading"} (or {"status":"reloaded"})
after the reload succeeded; keep apiConfigReload, pm.reloadFn, and
pm.sendErrorResponse as the focal symbols to update.

In `@proxy/proxymanager_models.go`:
- Around line 356-361: The code that strips the ".gguf" extension (using base :=
filename; if i := len(base) - len(".gguf"); i > 0 && strings.HasSuffix(base,
".gguf") { base = base[:i] }) should be replaced with strings.TrimSuffix to
simplify and handle edge cases (like the filename ".gguf"); change that block to
set base = strings.TrimSuffix(filename, ".gguf") and then set id =
strings.ToLower(base).