fix: stop a failed PM2 read from looking like an externally-started llama-server - #4804
Merged
Conversation
…lama-server (#4780) `getLlamaServerStatus` set `managed: null` for a PM2 read that FAILED — but nulled `config` on the same read, so every caller guarding on `!managed || !config?.model` got the same answer as for a daemon somebody else started. The tuning relaunch then refused with "llama-server was started outside PortOS", pointing a user who owns the daemon at a process that does not exist. Since #4759 an UNTUNED assessment relaunches too, so this cost real data: the refusal is filed `applied: false`, which drops the reading from `scorable`, which removes the model's backend-defaults BASELINE — the row every tuned reading is ranked against — from the comparison table. The `!running` exit also cleared the pre-tuning launch line, so the baseline could not be restored even after PM2 answered again. - keep the last known launch line on a failed read, so `managed: null` is actionable separately from `managed: false` - re-read PM2 a bounded number of times before letting "could not tell" decide anything, absorbing the transient hiccup that caused this - refuse an unreadable PM2 as itself (`retryable`, "could not read PM2") in the tuning relaunch, the capture and the restore, ahead of anything that discards state — the refusal is still the safe direction, it just stops lying about why - capture the launch line on an unreadable read rather than nothing, so a sweep that clears the user's flags still has a record of what they were - render the three states distinctly on the LLMs page instead of calling an unreadable server external
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
getLlamaServerStatusreturnsmanaged: nullwhen the PM2 read failed — deliberately not the same asfalse("somebody else started this daemon") — but it nulledconfigon the same failed read. Every caller guarding on!status.managed || !status.config?.modeltherefore treated "could not tell" exactly like an external process, and the tuning relaunch refused with "llama-server was started outside PortOS", pointing a user who owns the daemon at a process that does not exist.Since #4759 an untuned assessment relaunches too, so the misdiagnosis costs data: the refusal is filed
applied: false, whichgetAssessmentReportdrops fromscorable, which removes the model's backend-defaults baseline — the rowcompareTuningsranks every tuned reading against — from the comparison table. The!runningexit also clearedpreTuningConfig, so the baseline could not be restored even once PM2 answered again.managed: nullis actionable separately frommanaged: false.readLlamaServerStatusRetrying, 2 retries) before letting "could not tell" decide anything —fetchJlistcaches successes only, so each attempt is a genuine re-read.retryable: true, "could not read PM2") inrelaunchLlamaServerWithTuning,captureLlamaServerConfigandrestoreLlamaServerConfig, checked ahead of any branch that discards state. The refusal stays — PortOS must not restart a process it cannot prove it owns — it just stops blaming an external process for it.relaunchLlamaServerWithAliasalready drew this distinction; the tuning path now matches it, andmtplxServerManagerremains the reference for the split.Test plan
cd server && npm test— full suite green (32,714 passed). The tworoutes/imageGen.*suites that time out under parallel load pass in isolation and are unrelated to this change.cd client && npx vitest run src/components/settings/LocalLlmTab.test.jsx— 41 passed.origin/mainfirst and fails there:retryablewith a "could not read PM2" reason and never mentions an external process, having re-read more than once (bypass probe on the retry);Closes #4780