Skip to content

fix(controller): bound proxy upload file-cache charges - #163

Merged
Nickfost merged 4 commits into
mainfrom
fix/proxy-upload-memcg
Nov 3, 2026
Merged

Nickfost merged 4 commits into
mainfrom
fix/proxy-upload-memcg

Conversation

@Nickfost

@Nickfost Nickfost commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Concurrent build and image-load uploads can charge enough spool-file cache to kill the socket proxy inside its 128 MiB cgroup. Synchronize disk-spool writes and release cached pages in 4 MiB windows while writing and forwarding. Refuse tmpfs/ramfs before dispatch. Preserve the 1 GiB body cap, two upload slots, memory limit, source checks, maintenance locks and uncertainty markers.

Explicit spool read/write methods prevent optimized file copies from bypassing cache release. The memory fixture now unwinds through cleanup on SIGTERM and attempts removal even if Docker log collection fails. Installer recovery reads its five capability flags once through the existing jq prerequisite, while preserving schema, helper, type, mode, exact-key and digest checks. Failed retry waits include the child output; all wait and suite budgets remain unchanged.

Validation passed on exact head 7d08ab3df8d4c3f274e5f8da6bf36e4891819fd5, base 49da9d62dfa7527f569e0855b4fb877f6f8c7aed, tree a11bbea773b20f0e6d08574c0038275a7460f4ee:

  • Native run 41615505066, attempt 1 passed all eight stages through the unchanged required producer. The nine-member official artifact matches GitHub's digest, and all eight stage-log hashes verify. Expected skips are empty, with no positive skip reports. The required build is from GitHub Actions app 15368.
  • Two concurrent full 1 GiB uploads passed their payload/hash and phase assertions. Peak cgroup memory was 24,195,072 bytes; sampled file-cache peak 8,687,616 and dirty-file peak 8,364,032 bytes. There were no upload failures or OOM kills, and the fixture container was removed. These are disposable-worker results, not a claim about the live incident's charge composition.
  • The complete suite includes the guard, opaque restoration, interrupted checkpoint, fsync, cancellation, cleanup and negative controls. Prior controller race evidence remains valid for unchanged controller inputs. Current build, module, static analysis, documentation and committed-history secret checks passed. No test, inventory, budget or uncertainty guard was relaxed. The original full-run assertion's cause remains unproven; the current complete run passed after the measured capability-read improvement.
  • Both official results and the separate 14-file worker archive were exported and hash verified. Scoped runner access was removed, executor689 restored, and final admission verified the locked account, zero active validation/cgroups/containers, protected 840 image identities and operator-key digest. The Docker service start predates native admission.

Official archive SHA256 a40dfd1cefc0b28c86ff9ec8ad1ec5c9ad08cbf0e6c99b942380d3acdf801389; report SHA256 1832b840c392af1878ded1fdfc34fb4d5730c95af67aa37fb88898590928e2c1.

Independent manual source review approved this exact head without actionable source or documentation findings. The separate existing coordinator did not author or modify the implementation. It reviewed the complete successor delta, verified unchanged prior-reviewed inputs and independently passed the three focused recovery/guard tests. Codex service quota and Bugbot were unavailable; neither supplied approval. The earlier SIGTERM finding remains resolved.

The original 21ed memory proof retains its own source stamp. Failed run 41564610251 and both 22db attempts of 41568942095 remain retained with their complete available results and cleanup evidence. They are not relabeled as successful acceptance.

Documentation impact: docs/HOST-MAINTENANCE.md explains disk backing, file-cache charges and refusal behavior. jq is already a documented prerequisite. No public schema, capability, desired-state, initializer or operator-command change. Standalone-template guidance remains tracked in RandomDevelopment/ci-fleet-config-template#47 for separate acceptance.

Producer, driver, executor and live-pin bytes remain unchanged. Source acceptance does not publish a managed runtime or authorize live adoption, restarts, storage/capacity changes, marker/hold clearing or application retry. No release tag was created.

Nickfost commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex review

Please independently review exact SOURCE 21edcfe against BASE 49da9d6, including spool byte integrity, normal EOF/error/close semantics, tmpfs/ramfs refusal, 4 MiB writeback/cache windows, unchanged request/slot/lock/uncertainty contracts, and the native regression and documentation. The source-stamped two-full-upload proof passed at 21,319,680 bytes peak cgroup memory with no OOM. The separate baseline OOM reproducer used explicitly memory-backed spooling; the live incident's filesystem backing is not established. Existing limits and source identity must remain intact. No Cursor request or duplicate independent review is intended.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-11-03T13:50:47.655533Z
We couldn’t complete this review. Comment @codex review to try again.
7d08ab3 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 21edcfea2a

ℹ️ 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".

@Nickfost

Nickfost commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex review

Please review successor b102c93 against actual main 49da9d6. The prior required run exposed one omitted helper in the existing cleanup fixture. This successor adds that single empty mock helper, preserving all ten existing exit/status/cleanup assertions. Proxy code, module inputs, native memory regression and HOST-MAINTENANCE are byte-identical to the previously reviewed source. Review the fixture correction and retained regression integration; native proof keeps its original source stamp. No live adoption or application retry is authorized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b102c934e9

ℹ️ 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".

Comment thread scripts/test_proxy_memory.py Resolved
@Nickfost

Nickfost commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex review

Please review full successor 22db077 against actual main 49da9d6. The valid SIGTERM finding is corrected using the existing fixture termination-handler pattern. A committed installed-executor regression covers cancellation through the shell with successful and failed Docker log collection; both old-helper controls fail, both fixed cases and all ten existing cleanup/status cases pass. The focused transcript is exported/hash-verified. Review cleanup and regression integration, including unchanged memory, payload, lock and uncertainty guards. No complete new-head run has started; source availability remains separate from live adoption and application retry.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 22db077a49

ℹ️ 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".

@Nickfost

Nickfost commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

Required run41568942095 attempt1 failed at the unchanged10800-second tests-and-fixtures deadline. Five prior stages and all65 local-validation tests passed; the container/native memory stage did not execute. Failed artifact13208043754 and all six recorded stage hashes are retained/verified, with scoped execution cleanup complete. The unchanged late adapter cases both passed in2.629seconds. The exact22db077 head keeps its clean independent review. One bounded existing-run retry will retain the same source/base, full coverage and10800/240min/16200 budgets. Another timeout will stop automatic retries and require a concrete performance correction; no budget increase is planned. No live adoption or application retry is released.

Nickfost commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex review

Please review exact SOURCE 7d08ab3 against BASE 49da9d6. The new commit batches five capability probes through the existing jq prerequisite, preserves exact JSON-string capability matching and all release/helper trust checks, fails closed on reader failure, adds guard regressions, and retains the interrupted retry output on failure. The current focused executor689 proof uses this committed source with unchanged assertions/wait/budgets; full exact-head required validation remains pending. Earlier proxy/cancellation review findings remain resolved. This is the single needed successor-head independent review; no Bugbot review is requested.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Nickfost commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

@codex review

Retry the successor-head review that failed to complete at 2026-11-03T09:27:31.851739Z. SOURCE remains 7d08ab3 and BASE remains 49da9d6. No new actionable review comment or unresolved inline thread was returned. This is a retry of an incomplete review, not a replacement for a valid review. The focused committed-source checkpoint proof has now passed every original assertion, exported/verified with empty cgroups and zero containers. Nested activation took approximately54.789seconds; the unchanged-source focused measurements were66.008 and67.785seconds. All wait and suite budgets remain unchanged. Full native acceptance still waits for the completed independent review. No Bugbot action is requested.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Nickfost commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

Independent manual source review for PR #163

Reviewer: Codex gpt-6.1-sol in the existing coordinator session, separate from the implementation session. I did not author or modify the reviewed source. No new reviewer agent, delegate or review-service retry was started.

Reason for this review route: the Codex GitHub code-review quota is exhausted, as recorded in #163 (comment) and #163 (comment). This is a separately documented independent source review, not a claim that the failed service review completed or that a required CI check passed.

Reviewed identity:

  • Head: 7d08ab3df8d4c3f274e5f8da6bf36e4891819fd5
  • Base: 49da9d62dfa7527f569e0855b4fb877f6f8c7aed
  • Tree: a11bbea773b20f0e6d08574c0038275a7460f4ee
  • Prospective merge: 2409780590b0f730128b861341963e9b6d766cf7; its ordered parents and source-equal tree were verified.
  • Incremental delta from previously approved 22db077a491733205bb286bcd6fbc042ea09226b: SHA-256 e6e91dd82610af6eb40b4997347fc744e63363c77f5785c2523b5960de51f387.

Scope and assessment:

  • Read the complete three-file successor delta in scripts/install-worker-controller.sh, scripts/test-install-worker-controller.sh and scripts/test_portable_image_lifecycle.py, plus the affected recovery callers, capability parser, tool prerequisites and relevant operator/validation documentation.
  • The existing Python capability validator still rejects malformed schema, duplicate keys and non-boolean values before jq runs. The single jq read fails closed. JSON-encoded, whole-line key comparisons preserve exact capability names, including unknown names containing newlines. Required helper presence, symlink, executability and release-digest checks remain.
  • The retry diagnostic retains the existing wait count, delay and failure assertion while exposing the child's output. I checked the failed-subshell output-retention behavior separately.
  • The new regression checks missing and symlinked helpers, executable shell helpers versus regular Python helpers, disabled capabilities, newline-containing unknown names and jq failure.
  • Verified that the controller/proxy implementation, spool regression, module inputs, proxy-memory fixture, cancellation/cleanup regression, suite entrypoint and host-maintenance guide are byte-identical to the accepted 22db077a491733205bb286bcd6fbc042ea09226b review. Its completed review and resolved-finding evidence remain retained at fix(controller): bound proxy upload file-cache charges #163 (comment). This review adds independent assessment of the successor delta; it does not relabel the older review as a service approval for the new head.

Independent validation:

  • Ran the existing focused capability-helper guard, opaque runtime/checkpoint restoration, and pending-checkpoint/fsync-repair tests. All three passed in 19.488 seconds.
  • Rechecked the clean source, exact head/base, prospective merge identity and all review-thread pages. There are zero unresolved threads.
  • No full suite, native run, Docker/VM workload or live-host query was started for this review.

Documentation impact: the existing host-maintenance upload guidance remains accurate. jq is already a documented host/validation prerequisite. This successor changes no operator command, public schema or capability contract. The PR's focused-proof and timing statements retain their limits and do not claim a proven cause for the earlier full-run assertion. Configuration-template guidance remains tracked in RandomDevelopment/ci-fleet-config-template#47 and remains pending separate acceptance.

Verdict: APPROVED for independent source review at exact head 7d08ab3df8d4c3f274e5f8da6bf36e4891819fd5. No actionable source or documentation findings, and no unresolved blocking findings from this review.

Complete current-source native validation, its exports and scoped cleanup remain required before source acceptance or merge. Earlier failed runs and original-source memory proof retain their identities. This receipt grants no live adoption, restart, storage operation, limit change or application retry.

@Nickfost
Nickfost marked this pull request as ready for review November 3, 2026 13:50
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Nickfost
Nickfost merged commit 1c685da into main Nov 3, 2026
4 checks passed
@Nickfost
Nickfost deleted the fix/proxy-upload-memcg branch November 3, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant