Wanderland Dev — Best Practices (code smells & corrections)

Companion to wanderland-dev-ai-primer. The custom cops in the shared rubocop config (wanderland-core/lib/rubocop/cop/wanderland/, shipped to every repo via inherit_gem: wanderland-core) are the machine-enforced subset; this guide is the fuller catalog — the smells to look for, how to correct them, the locators, and the judgment calls a cop can't make.

Read/refresh this before a code-smell sweep. It's the content reload: all the things to look for and how to fix them, a more detailed version of what the hooks encode. Extend it as new smells surface.

Positive frame — reach for what exists

BoundaryInput owns the accessors. Before hand-rolling, use them:

• input.find("member.path") — falsy-safe indifferent dotted dig; nil when absent or blank-string, but keeps a stored false/0/[].
• input.find_or_fail("args.x") — same, but halts 400 on a missing required arg. This is the sanctioned single-fetch — not a smell.
• input.first_of_or("args.x", "domain_config.pipeline.x", default: {}) — first present across candidate paths; the path head names the source (args, config, params, …; domain_config routes through the runtime config-domain fallback); default is baked in.
• input.domain_config — the resolved config domain (config || runtime.config.domain || {}).
• input.composed_context — read-time LWW fold of the sub-crossings this boundary dispatched (what an iterator/chain returns).
• Schema seam: find/first_of_or also enforce the field's input_shape rule — a default: fills absent/blank, constraints: (ShapeMatcher) validate a present value and halt 400. Put argument validation in the schema, not #call.

Smell catalog

Each entry: Look for / Why / Fix / Locator.

1. Indifferent ||-subscript — hash[a] || hash[b]

• Look for: spec["type"] || spec[:type], route[:spec] || route["spec"].
• Why: || falls through on any falsy LHS, so a stored false/0/"" is silently dropped — the bug class that overrode termination_protection: false.
• Fix: Wanderland::HashUtils.dig_indifferent(spec, "type") (or hash.fetch(a) { hash.fetch(b, default) }).
• Locator: cop Wanderland/IndifferentHashOr.

2. Redundant || {} guard on dig_indifferent

• Look for: dig_indifferent(input["args"] || {}, "x").
• Why: dig_indifferent already returns nil for a nil receiver; the guard is noise and hides that the method is nil-safe.
• Fix: drop it → dig_indifferent(input["args"], "x").
• Locator: cop Wanderland/RedundantDigIndifferentGuard (autocorrects).

3. Hand-plucking from the input context

• Look for: dig_indifferent(input[...], …), dig_indifferent(input.domain_config, …), chained input["args"]["x"] / input["config"]["pipeline"]….
• Why: input owns accessors that are falsy-safe and (via the seam) schema-aware; hand-digging bypasses them.
• Fix: input.find("args.x") / input.first_of_or(...). A single input["args"] (handing the whole member along) and a lone find_or_fail("args.x") are fine.
• Locator: cop Wanderland/InputAccessByHand, plus grep dig_indifferent(config / dig_indifferent(args for the local-var cases the cop can't trace.
• Gotcha (cop authors): input in def call(input) is an lvar, not a bare method call. A cop matching only (send nil :input) is inert on real boundaries — match lvar :input too. (We shipped this bug and caught it on calver: the cop read 0 everywhere until fixed.)

4. Multi-source fallback plucking → first_of_or

• Look for: input.find("args.x") || dig_indifferent(config, "pipeline", "x") || {}, or the x = input.find(...); x ||= dig(...); x ||= {} ladder, or a config = input.domain_config local used only to dig once.
• Why: verbose, re-derives a config local, repeats the fallback.
• Fix: input.first_of_or("args.x", "domain_config.pipeline.x", default: {}).

5. Reimplementing a core helper

• Look for: a local def dig reimplementing HashUtils.dig_indifferent; recomputing input["config"] || runtime.config.domain || {} (that's input.domain_config); the tripled context.respond_to?(:reconstruct) ? … : {} (that's input.composed_context).
• Why: drift + duplication; the core version is tested and indifferent.
• Fix: delete the local, call the core helper.
• Locator: grep def dig, respond_to?(:reconstruct), runtime&.config&.domain.

6. Argument validation hand-coded in #call → input_shape schema

• Look for: default application (x.nil? ? DEFAULT : x), present-but-blank checks, ad-hoc type checks, is_a?(Hash) ? … : nil reads in #call.
• Why: the contract belongs in the declared schema; the find seam enforces it for free and it's introspectable at /inspect.
• Fix: declare default: and constraints: on the field (ShapeMatcher: matches: '\S' = non-blank, plus count, present, eq, prefix, gte/lte, …); read with input.find. #call collapses. If you keep a named default constant, use it as the schema's default: value rather than referencing it in #call.
• Reference: calver's format — optional, default: DEFAULT_FORMAT, constraints: { matches: '\S' }, one-line #call.
• No cop (judgment call) — the heuristic below flags candidates.

7. Partial execution — validate before you mutate

• Look for: an iterator that validates each entry inline and halts mid-loop, after earlier entries already dispatched real side effects (deploys, stack mutations).
• Why: a malformed entry N leaves entries 1..N-1 half-applied.
• Fix: a pre-pass that validates/normalizes every entry before the first dispatch.
• Reference: iterate_components (normalize_component pre-pass).

8. Dual-form DSL a boundary already subsumes → collapse + shim

• Look for: a special-case config form the engine can now express uniformly — e.g. a component chain: [...] when chain is itself a boundary, so it's just type: chain, args: { steps: [...] }.
• Why: extra branches, extra validation, two shapes to learn.
• Fix: collapse to the uniform shape; keep a back-compat normalization shim so existing configs keep running; add a prominent authoring note steering new configs to the canonical form (echo it in the input_shape description).
• Reference: iterate_components.

9. Reaching runtime.config.domain in a worker boundary (#5's cop-blind corollary)

• Look for: runtime.config&.domain / input["runtime"].config.domain (often paired with a local def dig) inside a boundary that runs as a background-worker process:, to read config the route path would have injected.
• Why: a worker-dispatched boundary (Workers::Pool#dispatch) receives {runtime, args} with no explicit config member — the route seam that seeds it (dispatch.rb → env.set("config", runtime.config.domain)) never runs on that path, and input.find("config.x") reads the explicit member ONLY (no runtime fallback — only domain_config has one). So the pull to hand-roll runtime.config.domain is stronger here than anywhere.
• Fix: input.domain_config / input.first_of_or("domain_config.x", default: {}) — both fall back to the runtime, so the SAME read works on the route and worker paths. (To also drop a manual config inject into the sub-inputs a worker orchestrator builds, migrate the sub-boundaries' find("config.x") to first_of_or("domain_config.x"), or seed config core-side in Workers::Pool#dispatch.)
• Locator: grep runtime&.config&.domain — not cop-caught. InputAccessByHand flags only input-rooted digs, so a runtime-rooted read and a custom-named def dig both sail past. Case: wisp materialise_run, the async-/submit worker tail (task-0f5bf7ca).

10. Secret handed to a shell adapter as an argv element

• Look for: a secret passed as a positional argument to a shell adapter's run — run(..., "-p", payload), run("--password", token), run("--from-literal=k=#{secret}"), or a credential inside a KEY=VALUE env / --build-arg / --param pair or a URL.
• Why: LoggingDecorator logs every adapter call, and HashUtils.render_log_array prints an all-scalar argv verbatim for elements under 200 characters. Shell adapters carry no path-glob masks. So a secret in argv reaches the debug log in full. A boundary's own info { } line is a second channel — it interpolates the value at info level with no scrubbing.
• Fix: register the secret with the adapter before the call — adapter.redact(value, as: "label"). The LoggingDecorator then substring-redacts it from the argv, env, stdin, and response, and scrub_against_declared_adapters! scrubs it from the returned context too — a secret inside a URL or a JSON blob included. Also keep the raw value off argv where you can: pass it on the stdin: kwarg (--password-stdin, --patch-file /dev/stdin) or the env: kwarg, which keeps it out of ps as well as the log. redact does not reach a boundary's own info { } line — strip the secret from that string by hand.
• Reference: k8s_secret_json_patch (--patch-file /dev/stdin, stdin: payload), ecr_docker_login (--password-stdin, stdin:), gem_push (credentials in env:). redact / mask_set on Adapters::Shell.
• Locator: SecretInShellArgv flags a secret-bearing flag (--password/--token/--secret/… and -p/--patch in a secret context) followed by a dynamic value. It cannot see a secret inside a KEY=VALUE pair or a URL — check those by eye.

The heuristic (when no cop fits)

A big block of defensive hash-digging means there's a better way somewhere.

Usually an accessor, a first_of_or, or a schema rule. If you're writing x.is_a?(Hash) ? x[...] : nil, a ladder of ||, or a hand-rolled default, stop and look for the seam.

The cops (machine-enforced subset)

Live in wanderland-core/lib/rubocop/cop/wanderland/, listed in lib/rubocop/wanderland_config.yml, shipped to every repo via inherit_gem: wanderland-core.

• IndifferentHashOr — smell #1.
• RedundantDigIndifferentGuard — smell #2 (autocorrects).
• InputAccessByHand — smell #3 (locator, no autocorrect).
• SecretInShellArgv — smell #10 (locator, no autocorrect).

To add a cop: drop it under cop/wanderland/, add its require + config block in wanderland_config.yml, rebuild/publish core, then bundle update wanderland-core in each repo. Test it against realistic boundary code (where input is an lvar param), not a sample where input parses as a bare method call — see smell #3's gotcha.

Rollout loop (per repo)

• bundle update wanderland-core — activates the current cops.
• bundle exec rubocop + the grep companions.
• Fix, run specs, push. Core-before-pack when a fix depends on a new core accessor/seam (the pack must bundle-update onto the republished core, or its CI regresses).

Status so far: wanderland-pipelines-pack (iterators + calver), wanderland-aws-pack and wanderland-k8s-pack (verified clean).

Adapter streaming & log levels — prefer stream: true over :debug

Shell adapters (docker, mvn, git, … — all the shell adapter kind bound to a command:) share one generic run surface: run(*args, cwd:, env:, stdin:, stream: false). stream: is not command-specific — it's the shell adapter's own keyword (adapters/shell.rb), backed by Open3.

• stream: false (default) → Open3.capture3 — captures the child's stdout/stderr at exit into result[:stdout]/[:stderr]. You only see that output through the boundary's debug entry/exit dump — which is why you end up reaching for :debug.
• stream: true → Open3.popen3 with a reader thread that logs each line live at Wanderland.logger.info(":stream:<command>") — on the normal info stream, tagged :stream:docker / :stream:mvn, greppable as it runs.

So to watch a long shell (build, deploy) on the standard output stream without turning on :debug, pass stream: true on the adapter call. That's the lever for "stop using :debug for adapter shells." Reference: docker_build and compile_jar (mvn.run(..., stream: true)).

Traces (boundary entry/exit) are a separate surface — logged at Wanderland.logger.debug(reg.name) (the :stream: tag is shells only). Getting traces onto stdout without :debug is a log-level / dedicated-sink change, not stream: — a distinct piece of work.

Readiness waits — poll + compare, don't pad with a fixed delay_s

When a step waits for an external system to become ready (a hot-reload endpoint, a rolled deployment, a synced configmap), the readiness mechanism is the poll: fire the request, compare the status to the allowed set, pass → move on; retry at interval_s up to max_attempts for the not-yet-ready case. A large fixed delay_s up front is not readiness — it's a blind guess that runs on every invocation whether or not the wait is needed, and it defeats the retry loop that's sitting right next to it.

Scar tissue (2026-08, jcasc hot-reload): the reload http_poll carried delay_s: 75. The endpoint (/configuration-as-code/reload) returns 302 the instant it's hit regardless of content, so there's nothing to poll for — someone padded a 75s pre-delay instead of trusting the retry. On an unchanged configmap apply it burned the full 75s and then succeeded on attempt 1: pure dead time. Right-sizing: keep a small buffer only for the genuine physical lag it covers — here, the kubelet syncing a changed configmap onto the pod's disk before JCasC re-reads it, ~20s is plenty — and let the retry loop be the readiness wait. Rule of thumb: a delay_s longer than interval_s is a smell. Ask what you're actually waiting for; usually it's a poll you haven't written, or a buffer several times bigger than the lag. (And if the wait is only needed on a changed apply, gate it on kubectl_apply reporting a change — the Dispatcher::GuardView { args, context } slot guard makes that a one-liner; see wanderland-dev-ai-primer §Constraints & gotchas.)