docs(ops): B850 bring-back + cross-node JuiceFS runbooks; 5090 MiniMax-H3 installer - #2502
Conversation
|
Warning Review limit reached
Next review available in: 54 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
POWERFULMOVES
left a comment
POWERFULMOVES
left a comment
There was a problem hiding this comment.
Pair-review pass from 4090-CLAUDE (field) — one P1, then it's ready
The runbooks are the good kind: they lead with the failure that actually
bit (file- vs minio-backed volume), they resolve hostnames live instead
of hardcoding, they carry the don't-untag regression as a measured warning,
and the H3 section corrects two of its own briefing premises rather than
shipping them. No IPs, no inline secrets, DB_PASS/SUPABASE_DB_PASSWORD
routed through the CHIT pipeline with an explicit "don't paste on a shared
CLI" note. That's the standard.
One finding is load-bearing, because it inverts this PR's own acceptance test.
P1 — the T3 cache verification passes only when the cache is UNBOUNDED
JUICEFS_CROSS_NODE_MOUNT_RUNBOOK.md § 3 verifies with:
docker exec juicefs-mount sh -c 'du -sh /data/jfsCache 2>/dev/null || du -sh /var/jfsCache'But /data/jfsCache is not what the documented path produces. On #2501's branch:
| site | emits |
|---|---|
juicefs-cross-node-setup.sh:93 (this runbook's target) |
JFS_CACHE_DIR=/data → --cache-dir /data |
egress.mk:363 |
JFS_CACHE_DIR=/data → --cache-dir /data |
compose juicefs-gateway:1533 |
--cache-dir "$D/jfsCache" |
helper's own auto-survey juicefs-cache-bounds.sh:78 |
"${best_mount%/}/jfsCache" |
Three of four use the .../jfsCache convention; the two mount call sites
explicitly override it to bare /data. JuiceFS then stores blocks at
/data/<volume-uuid>/raw/…, so /data/jfsCache never exists.
Follow the ||:
- bounds applied correctly →
/data/jfsCachemissing → falls through to
/var/jfsCache, which is the default location and is therefore also
absent → operator reads zero and, per the stated T3 criterion
("jfsCachegrows to a non-trivial size"), fails a correct mount. - bounds not applied (exactly #2501's
egress.mkempty-flags path) →
JuiceFS reverts to the 100 GiB default in/var/jfsCache→ non-trivial
size → T3 passes.
So the acceptance test for "cache is bounded" is satisfied by the unbounded
case and rejected by the bounded one. Worth stressing this is a runbook bug,
not a doc nit: T3 is the gate the whole Fleet-Pass leans on.
One-token fix that makes all four sites agree — set
JFS_CACHE_DIR=/data/jfsCache at juicefs-cross-node-setup.sh:93 and
egress.mk:363. The runbook command then works exactly as written, the
convention matches the gateway and the helper's own survey branch, and the
|| /var/jfsCache fallback becomes what it should be: a tell that the bounds
did not take. That belongs in #2501 alongside the two P1s already flagged
there, with this runbook unchanged.
P2 — this PR should land after #2501, and #2501 is currently REQUEST-CHANGES
Not a dependency of file references — I checked, all five mentions of
juicefs-cache-bounds.sh are path references with no line numbers, so
nothing dangles (correcting my own nit on #2501). The dependency is semantic:
§ 2 documents "computes per-host bounded cache flags … so the 4090/5090 do
not inherit the 100 GiB default" as present-tense behaviour, and on the
egress.mk path today that sentence is false — empty flags chain straight
into docker run. Sequencing #2501 first keeps the runbook honest on the day
it merges.
Nit — the Windows/5090 note is the most likely thing to be tried first
The 5090 is where H3 renders, so it is also where someone will want the
shared volume soonest, and it is the one path this PR leaves as
"pick whichever runtime matches." Fine to defer, but worth an explicit
(unverified — no 5090 mount has been performed) marker so it does not read
as a tested procedure a year from now.
Confirming the security follow-ups rather than restating them
Both B850 items are real and correctly rated HIGH — recreate juicefs-mount
with META_PASSWORD (the admin password was inline, i.e. visible in
docker inspect and ps, for days) and rotate it. Rotation is the
operator lane per feedback_env_tier_media_secret_leak; flagging here only so
it is not lost in a docs PR. Exposure was via process/inspect surface, not a
commit — this diff is clean.
Disposition
Approve-after-#2501. The P1 is one token in the sibling PR, not a rewrite
here. Nothing in these runbooks needs to change except the optional
unverified-marker nit.
Not reached: byte-verification of the H3 model repos (taking the hf_fs
receipts at face value), and the .bat under a real Windows ComfyUI root.
agent_signature (advisory, unsigned-local): ACK::4090-CLAUDE::PR2502-FLEET-PASS-RUNBOOKS-REVIEW-2026-08-09
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfd070106e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Verification pass on the 5 new threads — one P1 confirmed, one partially refutedCONFIRMED P1 —
|
Implementation pass — all 5 threads addressed, plus one defect the review missedTwo commits. Everything below was verified by executing the scripts and commands, not by reading them. Installer (
|
…MiniMax-H3 installer
Operator-run deliverables for the Fleet-Pass cluster (code guard ships separately
as the JuiceFS cache-bounds PR, referenced from these runbooks).
B850_BRINGBACK_RUNBOOK.md — closes the "not documented" gap: hostname-resolve-first
(3 drifted names), CHIT enroll sequence, `tailscale up` with the correct
tag:pmoves,gpu,lab set, the don't-untag regression warning, Tailscale-SSH-by-tag
note, and the two HIGH security follow-ups (re-create juicefs-mount with
META_PASSWORD; rotate the exposed Supabase admin password).
JUICEFS_CROSS_NODE_MOUNT_RUNBOOK.md — 4090/5090 mounts of the canonical
minio-backed `pmoves` volume (NOT file-backed `pmoves-media`), storage-check
preflight, per-node sequence with Thread-1 cache bounds applied, Windows/5090 note.
creator/installers/MINIMAX-H3-* — .bat + .sh (RunPod) NVFP4 installer, parametric
on COMFY_ROOT (in-tree PMOVES-Creator fork OR Pinokio fork), clones the 3 aux node
packs (H3 nodes are native in the fork), downloads NVFP4 models via hf. Sources are
byte-exact verified (hf_fs, 2026-08-09):
- DmitryDB/MiniMax-H3-ComfyUI-Quants: FL2VA/Ref2VA NVFP4 (10.86 GiB each),
VideoVAE-FP16, AudioVAE-FP32
- OTMFLY/Qwen3-VL-32B-Ultra-Heretic-MiniMax-H3-ComfyUI-INT8-ConvRot encoder
Corrects two brief premises: filenames are FREE (loaders are dropdowns — the
*_pruned_* name was a different repo), and the layers-50-63 tail is NOT the
conditioning encoder (the 24.55 GiB layers-0-49 file is REQUIRED). Flags the
abliterated/uncensored encoder as an explicit operator decision for client-facing
work, and that NVFP4 has not been run end-to-end on a 5090.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… honour INSTALL_TAIL
Three defects, all reproduced by running the script against a fake ComfyUI
portable layout on Windows.
1. `hf` was never reachable in the installer's PRIMARY scenario. pip drops console
scripts into the target interpreter's Scripts\ dir, and a stock ComfyUI portable
does not put python_embeded\Scripts on PATH — so every later bare `hf download`
died with "'hf' is not recognized" even though the pip install had just
succeeded. The CLI is now resolved to an absolute path next to the interpreter
we install into, falling back to PATH for venv/system installs, and the script
aborts with a clear message if it cannot be found at all.
2. A failed download only warned and continued, and the script still printed
"MiniMax-H3 models + support nodes ready." A false success on a 55-63 GB install
is expensive to discover — usually at generation time. `:hf` now returns non-zero,
every call site checks it, and a failure routes to a `:download_failed` banner
that names the likely causes and exits 1.
3. Found while running (2): the `INSTALL_TAIL` guard did not work. Unescaped parens
in `echo [OPTIONAL] ... (layers 50-63, ~7.1 GiB)` closed the if-body early, so
the OPTIONAL 7.1 GiB tail downloaded unconditionally and "[SKIP] optional tail"
printed immediately after it. Isolated repro, INSTALL_TAIL=0:
before: ***DOWNLOADING THE 7.1 GiB TAIL*** + [SKIP] optional tail
after: [SKIP] optional tail
Also switches the failure marker from `[!]` to `[ERROR]`: under
EnableDelayedExpansion the `!` was eaten, so the old message rendered as a bare
`[ ` — the one line an operator most needs to read was the one being mangled.
Verified end to end: with a deliberately non-executable hf stub in
python_embeded\Scripts, the script resolves it by absolute path, fails the first
download, prints the abort banner, never prints "ready", and exits 1. The same
harness on the pre-fix script prints "'hf' is not recognized" for every model and
then "MiniMax-H3 models + support nodes ready."
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cks the Known Road Two corrections to the B850 runbook, one to the cross-node runbook. Section 6a claimed closure it does not deliver. `docker run -e META_PASSWORD` makes Docker persist the EXPANDED value into the container's Config Env block, so the step moves the credential out of Args — visible in both `ps` and `inspect` — into the Env block, still visible to `docker inspect`. The section's own verification command greps that same block and expects a hit, which is the disproof sitting next to the claim. It now says plainly that this is reduction, not closure, names who can still read it (anyone in the docker group), and points at #2492 section 8 — file-mounted Docker secrets per the #1901 precedent — as the actual remediation. A runbook that overstates a fix is worse than one that discloses the gap, because the operator stops looking. No secrets mechanism is changed here; this is a docs PR and the file-mount belongs with #2492. Fleet checks now go through `make -C pmoves fleet-status`, per BOOTSTRAP.md ("Fleet view | never raw `tailscale status` for public IPs"). To be precise about why: raw `tailscale status` is NOT blocked by the damage-control hooks — bare `tailscale status` matches no pattern in patterns.yaml and the step as written completes. It is a convention the runbook should follow, not a guard it trips over, and following it keeps fleet IPs out of terminals and pasted output. The T2 tag criterion was unachievable as written, independent of which command is used: `tailscale status` does not print per-node tags at all. Its third column is the OWNER, which for a tagged node reads `tagged-devices`; verified on the live tailnet that `tag:` appears zero times in the human-readable output. Tags exist only in `tailscale status --json` under the peer Tags field. The runbook now uses a jq query that prints hostname + tags and nothing else, so the no-IPs convention still holds. Verified against a real tagged node: $ tailscale status --json | jq -r '...' pmoves-z890: tag:gpu, tag:lab, tag:pmoves `make -C pmoves fleet-status | grep -iE 'b850|knuckles|9850|rdna4'` also verified — it prints the hostname column with IPs redacted, which is all section 0 needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
7d66961 to
c25d1ed
Compare
The bottleneck stopped being discovery and became the ability to act. This is the artifact for acting: order and dependency as facts, checked against the tree, with no recommendation about what should land. "Ready" means nothing blocks it mechanically, not that it should merge. Every dependency was verified by tree lookup against origin/main and each PR head. No PR body was treated as evidence. The queue is 26 open PRs, not 13. MUST NOT MERGE AS-IS, at the top rather than buried: #2524 — base is feat/dockerfile-copy-anchor, not main. It deletes two entries from a baseline file that does not exist on main. Needs #2521, then retarget, then merge. #2526 — real failures on merge-decision and python-tests. Its purpose is to make python-tests capable of failing, so this may be the ratchet working; that has to be settled by its author, because merging it red turns a required check into permanent noise. #2517 — check-suit-release-notes fails. Not diagnosed. VERIFIED DEPENDENCIES (4): #2521 -> #2524 _known_copy_gaps.yaml absent on main #2514 -> #2515 JUICEFS_MEDIA_MINIO_REFORMAT_RUNBOOK.md absent on main, cited at jetson-combiner-archon-assignment:56 #2501 -> #2502 juicefs-cache-bounds.sh absent on main, cited twice in B850_BRINGBACK_RUNBOOK (:162, :213) #2519 -> future branch-protection-sync trigger work; #2519 adds the 5 missing `branch =` keys, without which the workflow's branch="main" fallback targets a branch 4 of those repos lack One assumed dependency corrected: #2502 -> #2501 is narrower than stated. juicefs-cross-node-setup.sh AND its make target already exist on main (mk/egress.mk:318); only juicefs-cache-bounds.sh is missing. The anchor ratchet would not have flagged it either way. COLLISION MAP: 7 files. Only one is a hard conflict — pmoves/Makefile, edited by #2521, #2523 and #2526 in the same ~20 lines around the validate-* block. #2523 should land first of the three: it is a 9-line deletion with no dependants, and rebasing two insertions onto a deletion is cheap while the reverse is not. Correction to the brief as requested: among currently open PRs only #2526 touches merge-gate.yml. The expected trim-2511 change is not open yet. Two facts that make red checks unreadable right now, stated up front: - `emit lifecycle trail` now fails on pull_request as well as push. Root cause pinned: the Archon commit our gitlink points at (1e02907ac) has seven nested gitlinks and no .gitmodules at all, so any recursive checkout dies at the first one. Archon's defect surfacing through our gitlink, not ours. Unowned. - merging needs an admin override: required_approving_review_count is 1 and the operator authors every PR, so self-approval is impossible. Six operator decisions listed separately from PR dependencies. Also disclosed: a gap in the COPY gate I shipped in #2521. It resolves COPY sources against compose build: contexts only, and never reads .github/workflows/integrations-ghcr.matrix.json — a second source of (dockerfile, context) pairs. Not theoretical: #2524's Dockerfile fix would have broken the a2ui-renderer GHCR publish, and a follow-up commit (8121e95) on that branch had to correct the matrix. The gate did not and could not catch it. Extending it to read the matrix is the follow-up. Anchor ratchet: 435 findings, 435 baselined, 0 new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bottleneck stopped being discovery and became the ability to act. This is the artifact for acting: order and dependency as facts, checked against the tree, with no recommendation about what should land. "Ready" means nothing blocks it mechanically, not that it should merge. Every dependency was verified by tree lookup against origin/main and each PR head. No PR body was treated as evidence. The queue is 26 open PRs, not 13. MUST NOT MERGE AS-IS, at the top rather than buried: #2524 — base is feat/dockerfile-copy-anchor, not main. It deletes two entries from a baseline file that does not exist on main. Needs #2521, then retarget, then merge. #2526 — real failures on merge-decision and python-tests. Its purpose is to make python-tests capable of failing, so this may be the ratchet working; that has to be settled by its author, because merging it red turns a required check into permanent noise. #2517 — check-suit-release-notes fails. Not diagnosed. VERIFIED DEPENDENCIES (4): #2521 -> #2524 _known_copy_gaps.yaml absent on main #2514 -> #2515 JUICEFS_MEDIA_MINIO_REFORMAT_RUNBOOK.md absent on main, cited at jetson-combiner-archon-assignment:56 #2501 -> #2502 juicefs-cache-bounds.sh absent on main, cited twice in B850_BRINGBACK_RUNBOOK (:162, :213) #2519 -> future branch-protection-sync trigger work; #2519 adds the 5 missing `branch =` keys, without which the workflow's branch="main" fallback targets a branch 4 of those repos lack One assumed dependency corrected: #2502 -> #2501 is narrower than stated. juicefs-cross-node-setup.sh AND its make target already exist on main (mk/egress.mk:318); only juicefs-cache-bounds.sh is missing. The anchor ratchet would not have flagged it either way. COLLISION MAP: 7 files. Only one is a hard conflict — pmoves/Makefile, edited by #2521, #2523 and #2526 in the same ~20 lines around the validate-* block. #2523 should land first of the three: it is a 9-line deletion with no dependants, and rebasing two insertions onto a deletion is cheap while the reverse is not. Correction to the brief as requested: among currently open PRs only #2526 touches merge-gate.yml. The expected trim-2511 change is not open yet. Two facts that make red checks unreadable right now, stated up front: - `emit lifecycle trail` now fails on pull_request as well as push. Root cause pinned: the Archon commit our gitlink points at (1e02907ac) has seven nested gitlinks and no .gitmodules at all, so any recursive checkout dies at the first one. Archon's defect surfacing through our gitlink, not ours. Unowned. - merging needs an admin override: required_approving_review_count is 1 and the operator authors every PR, so self-approval is impossible. Six operator decisions listed separately from PR dependencies. Also disclosed: a gap in the COPY gate I shipped in #2521. It resolves COPY sources against compose build: contexts only, and never reads .github/workflows/integrations-ghcr.matrix.json — a second source of (dockerfile, context) pairs. Not theoretical: #2524's Dockerfile fix would have broken the a2ui-renderer GHCR publish, and a follow-up commit (8121e95) on that branch had to correct the matrix. The gate did not and could not catch it. Extending it to read the matrix is the follow-up. Anchor ratchet: 435 findings, 435 baselined, 0 new. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 participant
Operator-run deliverables for the Fleet-Pass cluster. The JuiceFS cache-guard code ships in the sibling PR #2501, which these runbooks reference.
Runbooks (
pmoves/docs/operations/)B850_BRINGBACK_RUNBOOK.md— closes the "not documented" gap for re-enrolling B850 "Knuckles" (the single data-tier home): hostname-resolve-first (3 drifted names — confirm live viatailscale status), CHIT enroll sequence,tailscale upwith the correcttag:pmoves,gpu,labset (pertailscale-acl-policy.json, not the z890 script's tags), the measured don't-untag regression warning, Tailscale-SSH-by-tag note, and the two HIGH security follow-ups — re-createjuicefs-mountwithMETA_PASSWORD(Supabase admin password was inline/cleartext for days) and rotate that password.JUICEFS_CROSS_NODE_MOUNT_RUNBOOK.md— 4090/5090 mounts of the canonical minio-backedpmovesvolume (NOT file-backedpmoves-media),juicefs-storage-checkpreflight, per-node sequence with fix(ops): bound JuiceFS cache per-host (df-measured) at all mount call sites #2501's cache bounds applied, Windows/5090 note.5090 MiniMax-H3 installer (
pmoves/creator/installers/).bat+.sh(RunPod), parametric onCOMFY_ROOT(in-tree PMOVES-Creator fork or the operator's Pinokio fork), clones the 3 aux node packs (rgthree / KJNodes / VideoHelperSuite — H3 nodes are native in the fork), downloads NVFP4 models viahf.Sources byte-exact verified (
hf_fs, 2026-08-09):DmitryDB/MiniMax-H3-ComfyUI-Quants— FL2VA/Ref2VA NVFP4 (10.86 GiB each, a verified 48% cut vs INT8),VideoVAE-FP16,AudioVAE-FP32OTMFLY/Qwen3-VL-32B-Ultra-Heretic-MiniMax-H3-ComfyUI-INT8-ConvRotencoderTwo brief premises corrected by the recon:
minimax_h3_fl2va_pruned_int8_convrotname traced to a different repo (Winnougan, pruned W4A8) and is unverified; the installer keeps the real repo names.Flagged for the operator: the encoder is an abliterated/uncensored fine-tune (explicit decision for UNFCU/client-facing work; no stock alternative exists), and NVFP4 has not been run end-to-end on a 5090 (architecture-based rating only).
Still pending (operator inputs, per plan)
SEAP
.bat+ folder listing, the H3 workflow JSON, and Aitrepreneur's mirror link — to reconcile exact loader wiring. The verified sources above stand on their own.🤖 Generated with Claude Code