Comfy-Org/comfy-qa-tools

Fork 0

mirror of https://github.com/Comfy-Org/comfy-qa-tools.git synced 2026-11-04 23:47:20 +00:00

[PR #31] [MERGED] Fix what the first hardware pass found #34

New issue

Closed

opened 2026-10-31 22:02:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:02:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-qa-tools/pull/31
Author: @aliranjah
Created: 9/9/2026
Status: ✅ Merged
Merged: 9/9/2026
Merged by: @aliranjah

Base: main ← Head: qa/first-hardware-pass

📝 Commits (2)

b997db2 Fix what the first hardware pass found
a59d7fe Fix three lint errors ruff caught

📊 Changes

51 files changed (+8492 additions, -465 deletions)

View changed files

📝 CHANGELOG.md (+177 -3)
📝 README.md (+30 -6)
📝 comfy_qa/__init__.py (+30 -0)
📝 comfy_qa/auth.py (+102 -10)
📝 comfy_qa/config.py (+4 -4)
📝 comfy_qa/create.py (+186 -24)
📝 comfy_qa/gcloud.py (+232 -8)
📝 comfy_qa/host.py (+787 -122)
📝 comfy_qa/hostfile.py (+121 -16)
📝 comfy_qa/inflight.py (+23 -1)
📝 comfy_qa/lifecycle.py (+562 -29)
📝 comfy_qa/provision.py (+63 -2)
📝 comfy_qa/relocate.py (+253 -29)
📝 comfy_qa/remove.py (+20 -7)
📝 comfy_qa/say.py (+185 -5)
📝 comfy_qa/setup.py (+35 -6)
📝 comfy_qa/stamp.py (+4 -4)
📝 comfy_qa/tunnel.py (+216 -10)
📝 docs/machines.md (+9 -1)
📝 docs/test-criteria.md (+506 -38)

...and 31 more files

📄 Description

The first hardware pass, and what it found

This is the work from driving comfy-qat 1.1.0 against real GCE hardware for the
first time, plus the fixes that came out of it. Until now everything this tool
believed about itself came from tests against a fake cloud — one that answers
instantly, always has capacity, and never sends a bill. Four GPU boxes were
created and destroyed on a live project, ComfyUI was installed and served on two
of them, and a move was run end to end for the first time in the tool's
existence. The pass found one defect that makes the tool unusable on four of the
cards it offers, nine more that cost money, and a dozen that hand the user a
wrong answer at the moment they most need a right one. All of them are fixed
here. 45 files changed plus four new test files, around 6,700 insertions, and the
suite goes from 8,160 passing with 28 skipped to 8,519 with 11 — the skips having
become assertions rather than having been deleted.

The headline: four of the six cards could never have worked

V100, P100, P4 and K80 cannot be brought up by this tool, and never could.

Provisioning a Linux box installs the driver exactly one way — Google's own
cuda_installer.pyz install_driver, as a startup script — and that installer
lays down the open NVIDIA kernel module. The open module only works on a GPU
with a GSP, the on-board processor NVIDIA moved most of the driver onto with
Turing. Kepler, Pascal and Volta predate it and never got one.

Every visible sign was success. Read off a V100 box created on 2026-09-09:
create returned 0, the startup script finished 0 with the packages installed
and nvidia-open set on hold, the instance booted, and lspci showed the card.
No nvidia module loaded, nvidia-smi failed, and a clean reboot did not change
it. The kernel says why, and nothing in the tool's own output ever did:

NVRM: The NVIDIA GPU 0000:00:04.0 (PCI ID: 10de:1db1)
NVRM: installed in this system is not supported by open
NVRM: nvidia.ko because it does not include the required GPU
NVRM: System Processor (GSP).
NVRM: The NVIDIA probe routine failed for 1 device(s).
NVRM: None of the NVIDIA devices were initialized.

The project genuinely holds quota for all four, and quota list genuinely
reported them as ready. So the tool advertised a card, took the order, built
the box, billed for it, and the only symptom was a ComfyUI that would not start —
a night of debugging for something no amount of debugging on the box can fix.
Four of the six cards a tester could actually reach for were in that state.

What happens now:

create refuses all four before anything is created. The refusal is raised
in plan(), which runs offline and before the minute-long quota read, so it
costs a second and no money. It names the architecture, says no module loads at
all, lists the cards that do work, and offers --gpu t4 — the same
n1-standard-8 machine and the cheapest card that works.
quota list marks them, because a grant and a driver are two different
questions and the table used to answer only the first. A card the project holds
quota for now reads ready — this tool cannot drive it, and --json carries a
drivable field. status says granted but not drivable in its quota line.
The line is drawn at architecture, not at a list of card names. GSP is
silicon; a Pascal card will not grow one, so this is not a version skew that
waiting out will fix, and a name list is something somebody has to remember to
extend.

This is a refusal rather than a second driver flavour on purpose. The obvious
repair is to install the proprietary module on older cards, and there is no way
to ask cuda_installer.pyz for that — it takes an installation branch and no
flavour switch — so it would mean abandoning Google's documented installer for a
hand-rolled one and pinning a driver branch old enough to still carry the card.
That cannot be verified from a laptop. If someone builds it and proves it on
hardware, GSP_ARCHITECTURES in comfy_qa/create.py is the one place to change:
the refusal, the card lists and the help all read from it.

Related, and fixed with it: the startup script exited 0 and reported the driver
installed on a box where the module could not load at all.

Fixed — the defects that cost money

discover --prune deleted the host-list entry of a running box. Absence was
inferred from a bulk instances list keyed on (name, zone), so an entry whose
declared zone was wrong — a hand-typed field, a box recreated from the console, a
move that did not finish — read as "not on the project any more", was removed,
and exited 0. The L4 it named went on billing with nothing left in the host list
to reach it, so neither down nor list could see it again. Absence is now
decided on the name across the whole project; a name found in a different zone is
reported as a zone mismatch and kept, because the entry is wrong and the box
is not gone.

And an inference is no longer enough to remove anything. Every candidate is
now put to gcloud compute instances describe, by name, in its own zone, and
pruned only on a flat not-found — Google saying so about that machine, rather
than a listing it failed to appear in. That is one gcloud call per candidate: a
prune with five stale entries makes five and says how many it is about to check
before it checks them; a host list with nothing stale in it pays nothing. Three
details carry the weight of it:

DENIED is classified before NOT_FOUND, because Google answers a resource
you may not see with a 403 and not a 404. A credential that cannot see an
instance now raises rather than confirming an absence.
A not-found about the zone or the project raises too. One mistyped
gce_project would otherwise be answered "not found" for every entry naming
it, every one of those would read as a confirmed absent instance, and a single
y would delete the only record of machines still running on the project that
was meant.
gcloud compute instances list printing nothing at all is now an error rather
than an empty project. It used to read as "you have no machines", which pruned
every entry in a hand-maintained file that has no other copy on the machine.

move --clean deleted the previous run's work before deciding it would
refuse. The cleanup ran at the top of the command and the guards fifteen lines
below it, so a move refused by the GPU ceiling had already destroyed the snapshot
and the 200-300 GB disk that make a resumed move cheap — to carry out a move it
had already decided not to do. Every refusal a delete cannot change is now
computed first.

move's host-list rewrite could only fail after the box existed and was
billing. That rewrite is the move's seventh action. A host list that parses,
loads, and is spelled in a way the rewrite does not recognise — [hosts."comfy-win"]
and [ hosts.comfy-win ] are both valid TOML — failed at the end, with a
snapshot, a disk and a running GPU already paid for and comfy-qat down still
pointing at the old box. Every one of those refusals is computable from the local
file for free, so it is now computed before anything is created, through the same
code that does the real write.

move's GPU-ceiling guard failed open. It counted the source's cards out of
a bulk listing, so a source the listing did not carry, or a row without
guestAccelerators, both read as "no cards needed" — and zero skipped the quota
read and the whole check with it, leaving Google to refuse the create after the
snapshot and the disk existed. The requirement now comes off the describe
payload move already holds, and a host declared with a card whose payload shows
none counts as one rather than none. The quota read is also no longer skipped
when nothing currently holds a card: held + needed > ceiling is true at
held == 0 whenever the ceiling is below what the box needs, which is a lapsed
quota under an existing box, and the check that would have caught it could not be
reached.

move started a box you had deliberately stopped, and left it running.
Finding out whether a zone has capacity means trying to start the machine, and on
the path where the answer is "there is capacity here, no move needed" the probe
was never put back. The state is read before the probe now, and a box found
stopped is stopped again; a box found running is left alone and told so.

move --clean on that same path cleaned nothing. It decided there was
nothing to move and returned before looking at a single resource, so someone
running it to tidy up was told everything was fine while a snapshot and a disk
from a failed move went on billing. What earlier moves of that box left behind is
now found from the resources themselves — in whatever zone they are in — reported
whether or not --clean was passed, and deleted on --clean or an answered
prompt.

The snapshot ran under a clock written for starting an instance. Copying a
disk is not that operation: 200 GB is the default boot disk this tool creates, a
first snapshot of one is a full copy at Google's pace, and a real move went past
the 300-second limit — gcloud killed, Google still holding the snapshot in
UPLOADING, and the tool reporting a failure about a resource that was being
created and would bill. It has its own hour-long timeout now.

create billed a box against a host list it could not read. The read failure
was swallowed, the host list was treated as empty, the instance was created — and
the entry that would have made it reachable could not be written, so down could
not stop it. It refuses now, before anything exists.

down --all gave an all-clear while a machine was running. The command exists
to answer one question, am I still paying for anything, and it printed
was billing: comfy-win. Stopped. Nothing is now. directly above a warning that
an undeclared box was running. An all-clear is the last thing read; the reader
stops there and the orphan bills all night. The guard already existed on the
sibling branch and had never been applied to this one. What this stopped and
whether anything is left are now two separate claims, computed once and read by
both branches so they cannot drift apart again.

Fixed — wrong answers at the worst moment

go straight after create died on the tunnel and blamed your credentials.
This is the first-run path. RUNNING is Google's word for "powered on"; it is not
"sshd is listening", and on a box created moments ago it is not even "this machine
will stay up", because the startup script installs the driver and reboots it.
create's closing line promises go waits that out. It did not: go reported a
tunnel that closed for unknown reasons with gcloud auth login as the fix, on a
session that was fine, and the identical command four minutes later installed
ComfyUI and served it. A still-booting machine is now a shape the tunnel
recognises and waits on for up to five minutes, saying so while it waits. A box
that is genuinely ready never enters the wait, and only that shape is retried —
an expired credential, a taken port and a tunnel pointing at another machine are
raised on the first try, because none of them improves by being asked again and
every retry is GPU time.

up spent three minutes to say something that named two states and committed to
neither. "ComfyUI is not installed or not started", followed by advice to get
onto the box by hand — on a machine that has been through down, which is every
machine that has ever been stopped, since nothing on a box starts ComfyUI at
boot. It now asks the box what is on it and says which one it is, and hands over
comfy-qat go either way. Measured on hardware: go had ComfyUI answering in 52
seconds, immediately after up had spent 180 concluding the opposite.

logs exited 0 when the read failed, so a phase that grades on exit codes
graded nothing. A read that comes back non-zero now says that what is above it is
not the whole log. Ctrl-C on a follow exits 130 rather than 0 — it is "ended by
SIGINT", not a failure, and it is what stops a script reading $? from taking an
interrupted follow for a log that ended by itself. --tail 0 means zero lines
instead of one, and a negative --tail is refused.

rdp announced an irreversible password reset on a machine it had not
reached. It now checks gcloud is there and the box is running first, says the
password in use is unchanged, and hands over comfy-qat up. A Ctrl-C during the
reset is reported the way every other mutating call reports one, and can no
longer be overwritten by the command's own progress ticker.

A dead tunnel was explained with the previous session's error. The log was
opened in append mode, so a tunnel that died writing nothing was diagnosed from
the tunnel before it. It is written fresh per tunnel now. down and disconnect
remove the .pid and the .json — the two records that claim a tunnel is there —
and keep the .log, which is the only place the reason exists and is exactly
what somebody goes looking for after a tunnel dies. The diagnosis also reads the
whole of that log rather than its last six lines: gcloud's final line is its own
exit status restated, and the line saying why had scrolled off the top of the
tail.

The tunnel bound IPv4 only, so a browser preferring IPv6 for localhost got
an error page for a tunnel that was working — curl http://127.0.0.1:8190/
returned 200 while curl -6 'http://[::1]:8190/' could not connect. Both
families are forwarded now, with ExitOnForwardFailure=no pinned so a user's own
~/.ssh/config cannot let a failed IPv6 bind take the working IPv4 forward down
with it.

create's stockout advice printed a command the tool then refuses —
--os "Ubuntu 22.04", which is the image's name and not something the flag
accepts. It offers --os linux, and everything it prints is shell-quoted.

Two errors computed a remedy and threw it away. say.error falls back to the
exception's .fix, but the exception had been interpolated into an f-string
first, so what it inspected was a str. The advice gcloud's classifier had
already worked out — which region to use, which quota id is the real one — was
discarded, on quota request, where a refusal is the ordinary outcome.

hosts.toml.bak held a state that existed for milliseconds. A prune that also
adds writes twice, and each write kept its own copy, so the backup held the file
with the additions already in it. One copy per command now.

disconnect --help stated a rationale that can no longer occur — that killing
the ssh by hand leaves records that make list report a tunnel that is not
there. The pid-identity work fixed that; the docstring outlived it. It now says
what the command really does that kill does not: report whether the machine it
left behind is still running and billing.

discover also now reports what it did not do as carefully as what it did —
entries whose box turned up in another zone, and entries whose absence could not
be confirmed, each named with the reason and left alone. delete no longer loses
its interrupt report when the call it is making raises something unusual.

Fixed — the terminal output

A separate pass over what the tool actually looks like on a terminal. Almost none
of it was rewording: the sentences were right and the page they sat on was flat.

NO_COLOR is honoured. Nothing this tool writes itself has ever been
coloured and a test holds it to that, but --help is rendered by Typer through
Rich and came out in 348 SGR escapes on a pty with the variable set. Rich reads
it; Typer builds its own Console and does not pass it on. Measured: 348 escapes
to 0, with the box, the columns and every option name still in place. An empty
NO_COLOR= is not a request, per the standard.
A to fix: block with more than one thing in it now shows its groups. Four
runnable commands, a sentence introducing two of them, and a remark about what
a snapshot costs used to sit at one indent, all reading as things to paste.
# notes line up. The gap was typed at each call site — counted by eye, in
about twenty places — and one block had its third note two columns out of line.
Alignment is a property of the whole block, so it moved to the printer.
warning: stays welded to its own sentence rather than being stranded on a
line by itself when a caller asks for air above it.
The Ctrl-C report separates its claims. "This may exist and be billing" and
"this may still be running" were flush against each other, so the GPU box read
as the fourth line of a list about a snapshot.
discover gets a heading and the same column padding list uses, create --dry-run's plan block gets a heading to match the two named blocks around it,
and list's 168-character footnote is two authored lines.

Fixed — retired spellings the tool still taught

host and auth were removed as command groups at 1.1.0, and two places still
printed them at users:

The starter host list that init and setup write explained its last rule with
`host down` would leave a real instance running and billing — line 13
of the first file every new user owns, and the first explanation they read of
why the rule exists. comfy-qat host down exits 2.
comfy-qat go --help printed `host logs` reads its log, in the help of
the most-used verb.

Both now name the spelling that runs. The guard that was supposed to catch this
required the literal prefix comfy-qat host , so a bare host down in a
template string passed straight through; it now matches the bare verb phrase
across every line of the package, comments and docstrings included, with the verb
set derived from the live command tree rather than typed. 26 internal references
in comments and docstrings were corrected with it — prose only, no logic touched.

What was verified on real hardware

Kept separate because it is the unusual part. Everything in this section was
observed on live GCE instances on 2026-09-09, not in a fake.

card	zone	ComfyUI	generation	verdict
L4 (22 GB)	europe-west4-c	0.35.0, cuda:0	6.3s, image verified	pass
T4 (15 GB)	europe-central2-c	0.35.0, cuda:0	8.6s, image verified	pass, 3.6 TFLOPS fp32
V100 (Volta)	europe-west4-c	never started	impossible	fail — no GSP
P4 (Pascal)	europe-west4-c	never started	impossible	fail — 15 open-module rejections logged

Four GPU boxes created and every one destroyed. delete was exercised on real
machines, removing the instance, its disk and its host-list entry each time and
leaving backups. The project finished the night at zero instances and zero
disks: nothing is billing.
The first end-to-end move this tool has ever done. It was the largest
untested surface in the codebase — everything believed about it came from
tests. Verified after the fixes: --clean on a refused move left a 14.3 GB
snapshot and a 200 GB disk intact, where before the fix it destroyed both; a
200 GB snapshot completed in about three and a half minutes, inside the new
hour clock and past the old 300-second one; the ceiling guard refused for free
and created nothing.
go after create waits out the driver reboot — verified on the first-run
path that originally failed.
create's stockout advice prints a runnable --os linux.
down --all's all-clear was tested with a genuine undeclared orphan
running on the project, which is the case the bug needed and the case a fake
cloud had never produced.
Phases K (create), E/F (go, up), L (logs), S (ssh, rdp,
disconnect), R (move), G (down) and N (delete) ran on live hardware for
the first time.

Where a finding was not verified on hardware it was verified by mutation:
breaking the code deliberately and checking the suite goes red. That is how the
test gaps below were found, and how the down --all all-clear, the money
sentences and the column alignment were each pinned.

Test suite

8,160 passing with 28 skipped, to 8,519 with 11.

The eleven that remain are two Windows-only PowerShell builders and nine
message exemptions in test_docs.py, each of which is held by hygiene tests that
eject an entry the moment it stops excusing anything real.

The suite no longer touches the network. A subprocess trap over a full run
confirms zero real gcloud calls. Until this pass four unit tests were shelling
out to gcloud compute instances describe against a live project — and were
vacuous while doing it: a missing fixture meant they exited through the refusal
path without ever entering the branch they claimed to test, so inverting the rule
that decides whether rdp tells a user their password may have been reset left
the entire suite green. That file now runs in 1.4s rather than 8.3s.

The skips did not go away by being deleted. Several were tests that could not
fail: the inclusion criterion was also the pass criterion, so a case that had
nothing to check skipped itself and reported as a skip rather than as anything
anyone reads. Fifteen docstring-path cases and one option-help case were in that
shape. Each now asserts on the path it finds, with a floor test underneath saying
that at least one such path exists at all — so the suite can no longer go quiet by
finding nothing.

Ten allowlist skips had no staleness guard, which meant a dead entry excused
nothing and said so to no one. Those now fail when the thing they excuse has
left, which is what turned one exemption list into a shrinking one.

The 11 skips that remain are all declared exemptions rather than tests that gave
up. Two are test_provision.py's Windows-only PowerShell builders, which emit
nothing on Windows and have nothing to assert. The other nine are
troubleshooting-page entries excused for a written reason: seven quote somebody
else's words — click's No such command 'host', torch's "not compiled with CUDA
enabled" — which no change to this package will ever make ours, and two are
messages built from runs too short to identify themselves. Each list has a
hygiene test that ejects an entry the moment its excuse stops being true.

Every fix above arrived with the test that would have caught it.

Documentation

CHANGELOG.md carries all of this under 1.1.0 — the version in pyproject.toml
is unchanged, and tests/test_version.py requires the newest heading to name the
version that builds, so a separate release number is a decision to take
deliberately rather than a side effect of this PR.

README.md was re-checked claim by claim rather than trusted. Four statements
were stale: down was said to remove all three tunnel records when it removes
two, up was described as waiting until ComfyUI answers when it never starts
ComfyUI and now refuses fast, discover was said never to touch entries you
already have with no mention of --prune, and logs was described by a flag
rather than by what it does without one. The list of what the tool writes under
~/.config/comfy-qa-tools/ claimed to be complete and never mentioned
hosts.toml.bak or backups/.

docs/troubleshooting.md gained entries for every new message, which a test
enforces. docs/test-criteria.md gained the phases the hardware run covered and
two new criteria — K2b for the driver refusal and C4d for the quota list
marking, which have to agree with each other.

What this does not cover

Phase M — two GPU boxes up at once — is unreachable on this project. The
GPUS_ALL_REGIONS ceiling is 1, which is the quota that actually bites, so a
second box cannot be started to test against. It is not skipped because it is
unimportant; it is skipped because the project cannot hold the hardware.

A ComfyUI defect that is not in this tool, found while testing through it. A
ComfyUI server started with --listen returns file responses with no HTTP status
line and no headers over a non-loopback interface. Proven at the raw socket: the
same request to loopback returns HTTP/1.1 200 OK, while the LAN interface
returns the raw HTML body with no status line at all, which no browser will
render. Confirmed present in both 0.28.3 and 0.35.0, and it is the socket rather
than the Host header — connecting to loopback returns correct HTTP whatever Host
is sent. This is upstream in ComfyUI and wants filing there. The browser pass was
completed by forwarding to the box's own loopback instead, so it did not block
this work, but anyone running ComfyUI on a LAN interface will hit it.

Two cards were proven dead; two were not created. V100 and P4 were each built
on real hardware and each failed to initialise, the P4 logging fifteen kernel
rejections. P100 and K80 were not built — they share the architecture and the
mechanism is settled — and by the end of the pass no zone tried had P4 capacity
at all, which is consistent with the card being retired.

One path the refusal now protects is untested on metal. A pre-Turing box that
arrives some other way — created in the console, then adopted by discover —
is now refused by go in seconds rather than after a fifteen-minute wait. That
change is covered by tests and by mutation, but no P4 could be created to prove
it on hardware, because none was available anywhere by then.

Whether the older cards could be made to work at all is unanswered. Deciding
it needs one hand-built pre-Turing box and an attempt to install a proprietary
driver branch that still carries Pascal, against a torch build ComfyUI accepts.
If that succeeds, GSP_ARCHITECTURES is the only place to change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01YQnfQ2fschFSbpwkTLHCim

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-qa-tools/pull/31 **Author:** [@aliranjah](https://github.com/aliranjah) **Created:** 9/9/2026 **Status:** ✅ Merged **Merged:** 9/9/2026 **Merged by:** [@aliranjah](https://github.com/aliranjah) **Base:** `main` ← **Head:** `qa/first-hardware-pass` --- ### 📝 Commits (2) - [`b997db2`](https://github.com/Comfy-Org/comfy-qa-tools/commit/b997db2dae6f0ff50c3b39d88f5be0c988378ccc) Fix what the first hardware pass found - [`a59d7fe`](https://github.com/Comfy-Org/comfy-qa-tools/commit/a59d7fe3e518de47f23ef8b06688cc9d8ec4987b) Fix three lint errors ruff caught ### 📊 Changes **51 files changed** (+8492 additions, -465 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+177 -3) 📝 `README.md` (+30 -6) 📝 `comfy_qa/__init__.py` (+30 -0) 📝 `comfy_qa/auth.py` (+102 -10) 📝 `comfy_qa/config.py` (+4 -4) 📝 `comfy_qa/create.py` (+186 -24) 📝 `comfy_qa/gcloud.py` (+232 -8) 📝 `comfy_qa/host.py` (+787 -122) 📝 `comfy_qa/hostfile.py` (+121 -16) 📝 `comfy_qa/inflight.py` (+23 -1) 📝 `comfy_qa/lifecycle.py` (+562 -29) 📝 `comfy_qa/provision.py` (+63 -2) 📝 `comfy_qa/relocate.py` (+253 -29) 📝 `comfy_qa/remove.py` (+20 -7) 📝 `comfy_qa/say.py` (+185 -5) 📝 `comfy_qa/setup.py` (+35 -6) 📝 `comfy_qa/stamp.py` (+4 -4) 📝 `comfy_qa/tunnel.py` (+216 -10) 📝 `docs/machines.md` (+9 -1) 📝 `docs/test-criteria.md` (+506 -38) _...and 31 more files_ </details> ### 📄 Description # The first hardware pass, and what it found This is the work from driving `comfy-qat 1.1.0` against real GCE hardware for the first time, plus the fixes that came out of it. Until now everything this tool believed about itself came from tests against a fake cloud — one that answers instantly, always has capacity, and never sends a bill. Four GPU boxes were created and destroyed on a live project, ComfyUI was installed and served on two of them, and a `move` was run end to end for the first time in the tool's existence. The pass found one defect that makes the tool unusable on four of the cards it offers, nine more that cost money, and a dozen that hand the user a wrong answer at the moment they most need a right one. All of them are fixed here. 45 files changed plus four new test files, around 6,700 insertions, and the suite goes from 8,160 passing with 28 skipped to 8,519 with 11 — the skips having become assertions rather than having been deleted. --- ## The headline: four of the six cards could never have worked **V100, P100, P4 and K80 cannot be brought up by this tool, and never could.** Provisioning a Linux box installs the driver exactly one way — Google's own `cuda_installer.pyz install_driver`, as a startup script — and that installer lays down the **open** NVIDIA kernel module. The open module only works on a GPU with a GSP, the on-board processor NVIDIA moved most of the driver onto with Turing. Kepler, Pascal and Volta predate it and never got one. Every visible sign was success. Read off a V100 box created on 2026-09-09: `create` returned 0, the startup script finished 0 with the packages installed and `nvidia-open set on hold`, the instance booted, and `lspci` showed the card. No nvidia module loaded, `nvidia-smi` failed, and a clean reboot did not change it. The kernel says why, and nothing in the tool's own output ever did: ``` NVRM: The NVIDIA GPU 0000:00:04.0 (PCI ID: 10de:1db1) NVRM: installed in this system is not supported by open NVRM: nvidia.ko because it does not include the required GPU NVRM: System Processor (GSP). NVRM: The NVIDIA probe routine failed for 1 device(s). NVRM: None of the NVIDIA devices were initialized. ``` The project genuinely holds quota for all four, and `quota list` genuinely reported them as `ready`. So the tool advertised a card, took the order, built the box, billed for it, and the only symptom was a ComfyUI that would not start — a night of debugging for something no amount of debugging on the box can fix. Four of the six cards a tester could actually reach for were in that state. What happens now: - `create` refuses all four **before anything is created**. The refusal is raised in `plan()`, which runs offline and before the minute-long quota read, so it costs a second and no money. It names the architecture, says no module loads at all, lists the cards that do work, and offers `--gpu t4` — the same `n1-standard-8` machine and the cheapest card that works. - `quota list` marks them, because a grant and a driver are two different questions and the table used to answer only the first. A card the project holds quota for now reads `ready — this tool cannot drive it`, and `--json` carries a `drivable` field. `status` says `granted but not drivable` in its quota line. - The line is drawn at **architecture**, not at a list of card names. GSP is silicon; a Pascal card will not grow one, so this is not a version skew that waiting out will fix, and a name list is something somebody has to remember to extend. This is a refusal rather than a second driver flavour on purpose. The obvious repair is to install the proprietary module on older cards, and there is no way to ask `cuda_installer.pyz` for that — it takes an installation branch and no flavour switch — so it would mean abandoning Google's documented installer for a hand-rolled one and pinning a driver branch old enough to still carry the card. That cannot be verified from a laptop. If someone builds it and proves it on hardware, `GSP_ARCHITECTURES` in `comfy_qa/create.py` is the one place to change: the refusal, the card lists and the help all read from it. Related, and fixed with it: the startup script exited 0 and reported the driver installed on a box where the module could not load at all. --- ## Fixed — the defects that cost money **`discover --prune` deleted the host-list entry of a running box.** Absence was inferred from a bulk `instances list` keyed on `(name, zone)`, so an entry whose declared zone was wrong — a hand-typed field, a box recreated from the console, a `move` that did not finish — read as "not on the project any more", was removed, and exited 0. The L4 it named went on billing with nothing left in the host list to reach it, so neither `down` nor `list` could see it again. Absence is now decided on the name across the whole project; a name found in a different zone is reported as a zone mismatch and **kept**, because the entry is wrong and the box is not gone. **And an inference is no longer enough to remove anything.** Every candidate is now put to `gcloud compute instances describe`, by name, in its own zone, and pruned only on a flat not-found — Google saying so about that machine, rather than a listing it failed to appear in. That is one gcloud call per candidate: a prune with five stale entries makes five and says how many it is about to check before it checks them; a host list with nothing stale in it pays nothing. Three details carry the weight of it: - `DENIED` is classified before `NOT_FOUND`, because Google answers a resource you may not see with a 403 and not a 404. A credential that cannot see an instance now raises rather than confirming an absence. - A not-found about the **zone** or the **project** raises too. One mistyped `gce_project` would otherwise be answered "not found" for every entry naming it, every one of those would read as a confirmed absent instance, and a single `y` would delete the only record of machines still running on the project that was meant. - `gcloud compute instances list` printing nothing at all is now an error rather than an empty project. It used to read as "you have no machines", which pruned every entry in a hand-maintained file that has no other copy on the machine. **`move --clean` deleted the previous run's work before deciding it would refuse.** The cleanup ran at the top of the command and the guards fifteen lines below it, so a move refused by the GPU ceiling had already destroyed the snapshot and the 200-300 GB disk that make a resumed move cheap — to carry out a move it had already decided not to do. Every refusal a delete cannot change is now computed first. **`move`'s host-list rewrite could only fail after the box existed and was billing.** That rewrite is the move's seventh action. A host list that parses, loads, and is spelled in a way the rewrite does not recognise — `[hosts."comfy-win"]` and `[ hosts.comfy-win ]` are both valid TOML — failed at the end, with a snapshot, a disk and a running GPU already paid for and `comfy-qat down` still pointing at the old box. Every one of those refusals is computable from the local file for free, so it is now computed before anything is created, through the same code that does the real write. **`move`'s GPU-ceiling guard failed open.** It counted the source's cards out of a bulk listing, so a source the listing did not carry, or a row without `guestAccelerators`, both read as "no cards needed" — and zero skipped the quota read and the whole check with it, leaving Google to refuse the create after the snapshot and the disk existed. The requirement now comes off the `describe` payload `move` already holds, and a host declared with a card whose payload shows none counts as one rather than none. The quota read is also no longer skipped when nothing currently holds a card: `held + needed > ceiling` is true at `held == 0` whenever the ceiling is below what the box needs, which is a lapsed quota under an existing box, and the check that would have caught it could not be reached. **`move` started a box you had deliberately stopped, and left it running.** Finding out whether a zone has capacity means trying to start the machine, and on the path where the answer is "there is capacity here, no move needed" the probe was never put back. The state is read before the probe now, and a box found stopped is stopped again; a box found running is left alone and told so. **`move --clean` on that same path cleaned nothing.** It decided there was nothing to move and returned before looking at a single resource, so someone running it to tidy up was told everything was fine while a snapshot and a disk from a failed move went on billing. What earlier moves of that box left behind is now found from the resources themselves — in whatever zone they are in — reported whether or not `--clean` was passed, and deleted on `--clean` or an answered prompt. **The snapshot ran under a clock written for starting an instance.** Copying a disk is not that operation: 200 GB is the default boot disk this tool creates, a first snapshot of one is a full copy at Google's pace, and a real move went past the 300-second limit — gcloud killed, Google still holding the snapshot in `UPLOADING`, and the tool reporting a failure about a resource that was being created and would bill. It has its own hour-long timeout now. **`create` billed a box against a host list it could not read.** The read failure was swallowed, the host list was treated as empty, the instance was created — and the entry that would have made it reachable could not be written, so `down` could not stop it. It refuses now, before anything exists. **`down --all` gave an all-clear while a machine was running.** The command exists to answer one question, am I still paying for anything, and it printed `was billing: comfy-win. Stopped. Nothing is now.` directly above a warning that an undeclared box was running. An all-clear is the last thing read; the reader stops there and the orphan bills all night. The guard already existed on the sibling branch and had never been applied to this one. What this stopped and whether anything is left are now two separate claims, computed once and read by both branches so they cannot drift apart again. --- ## Fixed — wrong answers at the worst moment **`go` straight after `create` died on the tunnel and blamed your credentials.** This is the first-run path. RUNNING is Google's word for "powered on"; it is not "sshd is listening", and on a box created moments ago it is not even "this machine will stay up", because the startup script installs the driver and reboots it. `create`'s closing line promises `go` waits that out. It did not: `go` reported a tunnel that closed for unknown reasons with `gcloud auth login` as the fix, on a session that was fine, and the identical command four minutes later installed ComfyUI and served it. A still-booting machine is now a shape the tunnel recognises and waits on for up to five minutes, saying so while it waits. A box that is genuinely ready never enters the wait, and only that shape is retried — an expired credential, a taken port and a tunnel pointing at another machine are raised on the first try, because none of them improves by being asked again and every retry is GPU time. **`up` spent three minutes to say something that named two states and committed to neither.** "ComfyUI is not installed or not started", followed by advice to get onto the box by hand — on a machine that has been through `down`, which is every machine that has ever been stopped, since nothing on a box starts ComfyUI at boot. It now asks the box what is on it and says which one it is, and hands over `comfy-qat go` either way. Measured on hardware: `go` had ComfyUI answering in 52 seconds, immediately after `up` had spent 180 concluding the opposite. **`logs` exited 0 when the read failed**, so a phase that grades on exit codes graded nothing. A read that comes back non-zero now says that what is above it is not the whole log. Ctrl-C on a follow exits 130 rather than 0 — it is "ended by SIGINT", not a failure, and it is what stops a script reading `$?` from taking an interrupted follow for a log that ended by itself. `--tail 0` means zero lines instead of one, and a negative `--tail` is refused. **`rdp` announced an irreversible password reset on a machine it had not reached.** It now checks gcloud is there and the box is running first, says the password in use is unchanged, and hands over `comfy-qat up`. A Ctrl-C during the reset is reported the way every other mutating call reports one, and can no longer be overwritten by the command's own progress ticker. **A dead tunnel was explained with the previous session's error.** The log was opened in append mode, so a tunnel that died writing nothing was diagnosed from the tunnel before it. It is written fresh per tunnel now. `down` and `disconnect` remove the `.pid` and the `.json` — the two records that claim a tunnel is there — and keep the `.log`, which is the only place the reason exists and is exactly what somebody goes looking for after a tunnel dies. The diagnosis also reads the whole of that log rather than its last six lines: gcloud's final line is its own exit status restated, and the line saying why had scrolled off the top of the tail. **The tunnel bound IPv4 only**, so a browser preferring IPv6 for `localhost` got an error page for a tunnel that was working — `curl http://127.0.0.1:8190/` returned 200 while `curl -6 'http://[::1]:8190/'` could not connect. Both families are forwarded now, with `ExitOnForwardFailure=no` pinned so a user's own `~/.ssh/config` cannot let a failed IPv6 bind take the working IPv4 forward down with it. **`create`'s stockout advice printed a command the tool then refuses** — `--os "Ubuntu 22.04"`, which is the image's name and not something the flag accepts. It offers `--os linux`, and everything it prints is shell-quoted. **Two errors computed a remedy and threw it away.** `say.error` falls back to the exception's `.fix`, but the exception had been interpolated into an f-string first, so what it inspected was a `str`. The advice gcloud's classifier had already worked out — which region to use, which quota id is the real one — was discarded, on `quota request`, where a refusal is the ordinary outcome. **`hosts.toml.bak` held a state that existed for milliseconds.** A prune that also adds writes twice, and each write kept its own copy, so the backup held the file with the additions already in it. One copy per command now. **`disconnect --help` stated a rationale that can no longer occur** — that killing the ssh by hand leaves records that make `list` report a tunnel that is not there. The pid-identity work fixed that; the docstring outlived it. It now says what the command really does that `kill` does not: report whether the machine it left behind is still running and billing. `discover` also now reports what it did **not** do as carefully as what it did — entries whose box turned up in another zone, and entries whose absence could not be confirmed, each named with the reason and left alone. `delete` no longer loses its interrupt report when the call it is making raises something unusual. --- ## Fixed — the terminal output A separate pass over what the tool actually looks like on a terminal. Almost none of it was rewording: the sentences were right and the page they sat on was flat. - **`NO_COLOR` is honoured.** Nothing this tool writes itself has ever been coloured and a test holds it to that, but `--help` is rendered by Typer through Rich and came out in 348 SGR escapes on a pty with the variable set. Rich reads it; Typer builds its own Console and does not pass it on. Measured: 348 escapes to 0, with the box, the columns and every option name still in place. An empty `NO_COLOR=` is not a request, per the standard. - **A `to fix:` block with more than one thing in it now shows its groups.** Four runnable commands, a sentence introducing two of them, and a remark about what a snapshot costs used to sit at one indent, all reading as things to paste. - **`#` notes line up.** The gap was typed at each call site — counted by eye, in about twenty places — and one block had its third note two columns out of line. Alignment is a property of the whole block, so it moved to the printer. - **`warning:` stays welded to its own sentence** rather than being stranded on a line by itself when a caller asks for air above it. - **The Ctrl-C report separates its claims.** "This may exist and be billing" and "this may still be running" were flush against each other, so the GPU box read as the fourth line of a list about a snapshot. - `discover` gets a heading and the same column padding `list` uses, `create --dry-run`'s plan block gets a heading to match the two named blocks around it, and `list`'s 168-character footnote is two authored lines. --- ## Fixed — retired spellings the tool still taught `host` and `auth` were removed as command groups at 1.1.0, and two places still printed them at users: - The starter host list that `init` and `setup` write explained its last rule with ``` `host down` would leave a real instance running and billing ``` — line 13 of the first file every new user owns, and the first explanation they read of why the rule exists. `comfy-qat host down` exits 2. - `comfy-qat go --help` printed ``` `host logs` reads its log ```, in the help of the most-used verb. Both now name the spelling that runs. The guard that was supposed to catch this required the literal prefix `comfy-qat host `, so a bare `host down` in a template string passed straight through; it now matches the bare verb phrase across every line of the package, comments and docstrings included, with the verb set derived from the live command tree rather than typed. 26 internal references in comments and docstrings were corrected with it — prose only, no logic touched. --- ## What was verified on real hardware Kept separate because it is the unusual part. Everything in this section was observed on live GCE instances on 2026-09-09, not in a fake. | card | zone | ComfyUI | generation | verdict | |---|---|---|---|---| | L4 (22 GB) | europe-west4-c | 0.35.0, cuda:0 | **6.3s**, image verified | pass | | T4 (15 GB) | europe-central2-c | 0.35.0, cuda:0 | **8.6s**, image verified | pass, 3.6 TFLOPS fp32 | | V100 (Volta) | europe-west4-c | never started | impossible | **fail — no GSP** | | P4 (Pascal) | europe-west4-c | never started | impossible | **fail — 15 open-module rejections logged** | - Four GPU boxes created and every one destroyed. `delete` was exercised on real machines, removing the instance, its disk and its host-list entry each time and leaving backups. The project finished the night at zero instances and zero disks: nothing is billing. - **The first end-to-end `move` this tool has ever done.** It was the largest untested surface in the codebase — everything believed about it came from tests. Verified after the fixes: `--clean` on a refused move left a 14.3 GB snapshot and a 200 GB disk intact, where before the fix it destroyed both; a 200 GB snapshot completed in about three and a half minutes, inside the new hour clock and past the old 300-second one; the ceiling guard refused for free and created nothing. - `go` after `create` waits out the driver reboot — verified on the first-run path that originally failed. - `create`'s stockout advice prints a runnable `--os linux`. - `down --all`'s all-clear was tested with a **genuine undeclared orphan running** on the project, which is the case the bug needed and the case a fake cloud had never produced. - Phases K (`create`), E/F (`go`, `up`), L (`logs`), S (`ssh`, `rdp`, `disconnect`), R (`move`), G (`down`) and N (`delete`) ran on live hardware for the first time. Where a finding was **not** verified on hardware it was verified by mutation: breaking the code deliberately and checking the suite goes red. That is how the test gaps below were found, and how the `down --all` all-clear, the money sentences and the column alignment were each pinned. --- ## Test suite **8,160 passing with 28 skipped, to 8,519 with 11.** The eleven that remain are two Windows-only PowerShell builders and nine message exemptions in `test_docs.py`, each of which is held by hygiene tests that eject an entry the moment it stops excusing anything real. **The suite no longer touches the network.** A `subprocess` trap over a full run confirms zero real `gcloud` calls. Until this pass four *unit* tests were shelling out to `gcloud compute instances describe` against a live project — and were vacuous while doing it: a missing fixture meant they exited through the refusal path without ever entering the branch they claimed to test, so inverting the rule that decides whether `rdp` tells a user their password may have been reset left the entire suite green. That file now runs in 1.4s rather than 8.3s. The skips did not go away by being deleted. Several were tests that could not fail: the inclusion criterion was also the pass criterion, so a case that had nothing to check skipped itself and reported as a skip rather than as anything anyone reads. Fifteen docstring-path cases and one option-help case were in that shape. Each now asserts on the path it finds, with a floor test underneath saying that at least one such path exists at all — so the suite can no longer go quiet by finding nothing. Ten allowlist skips had no staleness guard, which meant a dead entry excused nothing and said so to no one. Those now fail when the thing they excuse has left, which is what turned one exemption list into a shrinking one. The 11 skips that remain are all declared exemptions rather than tests that gave up. Two are `test_provision.py`'s Windows-only PowerShell builders, which emit nothing on Windows and have nothing to assert. The other nine are troubleshooting-page entries excused for a written reason: seven quote somebody else's words — click's `No such command 'host'`, torch's "not compiled with CUDA enabled" — which no change to this package will ever make ours, and two are messages built from runs too short to identify themselves. Each list has a hygiene test that ejects an entry the moment its excuse stops being true. Every fix above arrived with the test that would have caught it. --- ## Documentation `CHANGELOG.md` carries all of this under 1.1.0 — the version in `pyproject.toml` is unchanged, and `tests/test_version.py` requires the newest heading to name the version that builds, so a separate release number is a decision to take deliberately rather than a side effect of this PR. `README.md` was re-checked claim by claim rather than trusted. Four statements were stale: `down` was said to remove all three tunnel records when it removes two, `up` was described as waiting until ComfyUI answers when it never starts ComfyUI and now refuses fast, `discover` was said never to touch entries you already have with no mention of `--prune`, and `logs` was described by a flag rather than by what it does without one. The list of what the tool writes under `~/.config/comfy-qa-tools/` claimed to be complete and never mentioned `hosts.toml.bak` or `backups/`. `docs/troubleshooting.md` gained entries for every new message, which a test enforces. `docs/test-criteria.md` gained the phases the hardware run covered and two new criteria — K2b for the driver refusal and C4d for the `quota list` marking, which have to agree with each other. --- ## What this does not cover **Phase M — two GPU boxes up at once — is unreachable on this project.** The `GPUS_ALL_REGIONS` ceiling is 1, which is the quota that actually bites, so a second box cannot be started to test against. It is not skipped because it is unimportant; it is skipped because the project cannot hold the hardware. **A ComfyUI defect that is not in this tool, found while testing through it.** A ComfyUI server started with `--listen` returns file responses with no HTTP status line and no headers over a non-loopback interface. Proven at the raw socket: the same request to loopback returns `HTTP/1.1 200 OK`, while the LAN interface returns the raw HTML body with no status line at all, which no browser will render. Confirmed present in both 0.28.3 and 0.35.0, and it is the socket rather than the Host header — connecting to loopback returns correct HTTP whatever Host is sent. This is upstream in ComfyUI and wants filing there. The browser pass was completed by forwarding to the box's own loopback instead, so it did not block this work, but anyone running ComfyUI on a LAN interface will hit it. **Two cards were proven dead; two were not created.** V100 and P4 were each built on real hardware and each failed to initialise, the P4 logging fifteen kernel rejections. P100 and K80 were not built — they share the architecture and the mechanism is settled — and by the end of the pass no zone tried had P4 capacity at all, which is consistent with the card being retired. **One path the refusal now protects is untested on metal.** A pre-Turing box that arrives some other way — created in the console, then adopted by `discover` — is now refused by `go` in seconds rather than after a fifteen-minute wait. That change is covered by tests and by mutation, but no P4 could be created to prove it on hardware, because none was available anywhere by then. **Whether the older cards could be made to work at all is unanswered.** Deciding it needs one hand-built pre-Turing box and an attempt to install a proprietary driver branch that still carries Pascal, against a torch build ComfyUI accepts. If that succeeds, `GSP_ARCHITECTURES` is the only place to change. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01YQnfQ2fschFSbpwkTLHCim --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 22:02:49 +00:00

closed this issue
added the
pull-request
label

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/comfy-qa-tools#34

No description provided.

Rows
Columns