Skip to content

feat(run): give the command an empty stdin, so no call needs < /dev/null (LAB-7678) - #6

Merged
27Bslash6 merged 2 commits into
mainfrom
lab-7678-ck-run-empty-stdin
Oct 3, 2026
Merged

27Bslash6 merged 2 commits into
mainfrom
lab-7678-ck-run-empty-stdin

Conversation

@27Bslash6

Copy link
Copy Markdown
Contributor

ck run now spawns every command with stdin on /dev/null, whatever ck's own stdin is, so no call needs < /dev/null and every call caches the same way from a terminal, a CI pipe or an agent's socket.

Closes LAB-7678

Why

The old rule cached a call only when the caller redirected stdin to /dev/null. Forgetting the redirect never failed: the output stayed correct, one stderr line said caching was off, and the rate-limited origin took every hit. In each target context (shell startup file, CI step, agent shell) the default stdin failed the gate. ck spawns the command, so it can guarantee the empty stdin itself instead of asking the caller to promise it. This is what GNU xargs and ssh -n do.

Changes

  • Supervisor::run sets Stdio::null() on every spawn, which covers a miss, --refresh, a fail-open run after a backend error, and output past the cap.
  • uncacheable_stdin and its warning are deleted. ck never reads its own stdin.
  • The key is unchanged (argv plus --scope), so existing entries, including ones written by < /dev/null calls, keep hitting.
  • README: the "stdin must be /dev/null" section is now "The command does not get your stdin", and every example drops the redirect. ck run --help says the command gets an empty stdin.

Tests

  • the_command_gets_an_empty_stdin_whatever_cks_is replaces only_dev_null_stdin_is_cached. It puts a terminal, a pipe, a socket and a regular file on ck's stdin, each holding data. For each, the second call hits, stderr is empty, the command read nothing, and a later < /dev/null call hits the same entry.
  • a_pipe_that_never_closes_does_not_hang_ck: a pipe whose writer stays open and never writes does not hang ck, and the call caches.
  • a_401_on_the_value_read_runs_uncached now also checks that the fail-open run gets an empty stdin.
  • The README check that every call ends in < /dev/null is gone. The README examples (file and saas) now run with sh's stdin on a pipe and assert an empty stderr.
  • With the Stdio::null() line removed, both new tests fail within 5 s, with "the command waited on ck's stdin" and "ck hung on its stdin". They do not hang.

cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test --locked (17 unit, 41 run, 15 saas, 1 readme) and cargo +1.85 check all pass locally.

…ull (LAB-7678)

ck spawns the command, so it can guarantee the command's stdin is empty
instead of making the caller promise it. Every command now runs with stdin
on /dev/null, whatever ck's own stdin is, and the stdin gate and its
warning are gone. A call from a terminal, a CI pipe or an agent socket now
caches the same way, and the key is unchanged, so existing entries keep
hitting.
@kodus-27b

kodus-27b Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

Kody Code Review — 2 suggested fixes.
Paste the prompt below to your agent and all review fixed at once!

🛠️ Open Agent Prompt
A code review identified the following issues in this pull request.
Each section describes what was found and includes a reference implementation where available.

Files involved:
- README.md:15
- tests/run.rs:270

---

### [1/2] README.md:15
Issue identified during code review:
Violates team rule 'No hardcoded API key/token literal in credential-named variable': WHAT: Flag a string literal assigned to a variable, constant, struct field, or default value whose name indicates a credential (api_key, token, secret, password, access_key, client_secret) in non-test source code.
HOW: In ADDED lines, look for `let/const/static <credential-ish name> = "..."` or `credential_field: "..."` where the right-hand side is a literal instead of an env/secret lookup.

---

### [2/2] tests/run.rs:270
Issue identified during code review:
Race condition in the_command_gets_an_empty_stdin_whatever_cks_is: in the 'a pipe' case the test writes to ck's stdin after spawn and unwraps the result, but ck never reads stdin and can exit before the write. On the second call ck serves a quick cache hit and closes the read end, so write_all returns EPIPE (Rust ignores SIGPIPE) and unwrap panics, which makes the test flaky. Fix: ignore BrokenPipe on that write, e.g. `let _ = pipe.write_all(b"from a pipe\n");`, or write the data before ck can exit by pre-filling an os pipe passed as stdin.
Reference implementation (from code review):

// tests/run.rs:270
if let Some(mut pipe) = child.stdin.take() {
    // ck never reads stdin and may already have exited (EPIPE).
    let _ = pipe.write_all(b"from a pipe\n");
}

---

Review each issue in context, use the reference implementations as guidance, and apply fixes that are consistent with the surrounding codebase.

Comment thread README.md
Comment thread tests/run.rs
ck never reads its stdin, so it can exit before the test writes to the
pipe on its stdin. The write then fails with EPIPE and the unwrap
panicked, making the_command_gets_an_empty_stdin_whatever_cks_is flaky
under load. EPIPE means nothing read the pipe, so accept it and fail on
any other error.
@27Bslash6

Copy link
Copy Markdown
Contributor Author

@kody start-review

@kodus-27b

kodus-27b Bot commented Oct 3, 2026

Copy link
Copy Markdown

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the `@kody start-review` command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Providing Context (Files & MCPs)

Add these hints in your PR description (or a comment) to unlock deeper checks:

  • Ticket / Acceptance Criteria: `Refs: ABC-123` (Linear/Jira/Asana/ClickUp/Trello) or a direct ticket link.
  • Bugfix Validation: a Sentry/Datadog/Bugsnag event link (or paste the stack trace/error message).
  • Endpoint Risk: mention the route (e.g., `POST /api/payments`) or controller/action name.
  • Attach a repo file as context: use an explicit marker like `@file:docs/guide.mdx#L10-L50` (replace with your real path).
  • API Contract Docs: include `@file:openapi.yaml` or `@file:swagger.json` when changing routes/schemas.
  • Definition of Done / Standards: include `@file:DOD.md` or `@file:CONTRIBUTING.md` if your repo has them.
  • Design System Source of Truth: include `@file:ui/index.ts` (replace with your DS entrypoint path).
  • Feature Flags: include the flag key/name and `@file:flags.ts` / `@file:config.json` (and optionally the PostHog flag name).
  • Edge/CDN Rules: link the Cloudflare rule/zone or describe the intended redirect/header behavior.
  • Attach an MCP tool output: use `@mcp<provider|tool>` (replace with an installed MCP provider + tool, e.g., `@mcp<sentry|events.search>`).
Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ✅

Access your configuration settings here.

@27Bslash6
27Bslash6 merged commit d392201 into main Oct 3, 2026
5 checks passed
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