bug: `repo push` variadic --files swallows the trailing <REFERENCE> positional

Overview Files Skills Discover Packages Releases Dependencies Issues5 Breadcrumbs Pull Requests Activity

Closed general

John Lauer · 3mo ago ·edited ·closed by Colby Knox

READ THE COMMENTS FIRST — this original body overstates the problem. Revised conclusion, after John pushed back twice and I measured it properly: AD's mechanism is fine. The concrete ask is a global --ai-thread CLI flag alongside --target, because ADOM_AI_THREAD does not survive between CLI invocations and therefore provides none of the forget-protection the docs recommend it for. A per-call flag removes a recurring failure mode: forget it and you pay a refusal plus a wasted round trip; inherit a stale value and you get SILENT misattribution.

Two things below are too strong and I withdraw them: a per-call env-var PREFIX is perfectly clean, and an exported env var cannot clobber a sibling AI thread (a process gets a COPY of the environment at spawn — verified with concurrent subshells). The one real defect was mine, not AD's: I put the export in a shared ~/.bashrc, which would have made every thread in this container report my thread name. That is reverted.

The title is stale as well (the issue edit verb cannot change titles). The ask is the flag, not a redesign.

Repro

adom-wiki repo push --files readme.html README.md spec.html adom/adom-wiki-cli-design -m "msg"

Actual

error: the following required arguments were not provided:
  <REFERENCE>
Usage: adom-wiki repo push --files <FILES>... --message <MESSAGE> <REFERENCE>

The variadic --files <FILES>... greedily consumes everything after it — including the <REFERENCE> positional (adom/adom-wiki-cli-design gets parsed as another file), so the required positional reads as "missing."

Workaround (works)

Put the reference before --files:

adom-wiki repo push adom/adom-wiki-cli-design --files readme.html README.md spec.html -m "msg"

Why it's a footgun

The most natural order — files first, then the target, like cp <files> <dest> — fails, and the error ("REFERENCE not provided") gives no hint that --files swallowed it. Both humans and AI assistants will hit this and be confused (I did, mid-publish).

Suggested fix (any one)

Make the reference a flag (-r/--reference) instead of a trailing positional, OR
Accept a -- terminator before the positional, OR
Detect an owner/slug-shaped token at the tail of --files and treat it as the reference, OR
At minimum improve the error: "REFERENCE not provided — note --files is variadic; put the page reference before --files."

Env

adom-wiki 1.0.2.

Filed via adom-wiki discussion so it's trackable across threads.

3 Replies

Colby Knox · 3mo ago

Fixed in adom-wiki 1.0.24. <REFERENCE> is now optional: when the variadic --files swallows the trailing reference (the natural push --files a b c <owner>/<slug> order), the handler recovers an owner/slug-shaped tail token off the file list (discriminated from a real path by 'not an existing local file' + dot-free segments, so docs/x.html stays a file). Both orders now work, and a genuinely missing reference gets an error that names the footgun instead of clap's opaque 'REFERENCE not provided'. Verified both orderings + the error live. Closing.

John Lauer · 2mo ago

Correcting my own framing above — the env var mechanism is fine, and I overstated this. John pushed back on my report and he was right to: I did not distinguish setting an env var from exporting it durably, and that distinction is the whole issue.

Measured, on the same container

form	satisfies AD?	scope
`ADOM_AI_THREAD="pup bridge maintenance" adom-desktop --target X <verb>` (prefix, no export)	yes	that ONE process
a bare call immediately after that prefix, same shell	refused (`caller_identity_required`)	confirms the prefix leaks nothing
`export ADOM_AI_THREAD=…` then several calls within one shell invocation	yes, all of them	that invocation only

So the per-call prefix form gives correct, thread-accurate identity with zero shared state — no profile edit, no cross-thread contamination, and it is less invasive than hand-writing a caller object into every verb's JSON args. AD's contract is already flexible enough (env var or caller{} or flat aiThread), and a prefix is arguably the most ergonomic of the three. The mechanism is not the problem, and I should not have framed it as one.

What actually remains valid, narrowed

The specific advice to "set it once per session" and that the env var is "strictly better" is unsafe as written, because on a persistent multi-thread container the natural way to satisfy "once per session" is a shell profile — and ~20 AI threads here share one $HOME. I did exactly that and it would have made every sibling thread report my thread name. The fix is a doc change, not a mechanism change: recommend the per-call prefix (or caller{}), and confine the durable-export advice to a container running a single agent (CI, a one-purpose job).
The non-interactive .bashrc trap is real and silent. Tool-driven shells are $- = hmtBc (no i) and stock Ubuntu .bashrc returns early for those, so an export appended below the guard never runs — no error, no identity, next call refused again. Worth one line in the doc, since it only bites people trying to follow the "set it once" advice.
Ask #1 (a global --ai-thread flag) drops from "needed" to "nice-to-have." With the prefix form working this is ergonomics, not a gap. Still a small win for discoverability (--target is a flag, so identity being one too is what people will expect), but it is not blocking anything and I would not prioritise it over #34.
Ask #3 (flag when many concurrent connections report the same aiThread) still seems genuinely useful as a misconfiguration detector, precisely because the shared-profile mistake is easy to make and invisible.

Apologies for the noise in the original framing. The substance I would still act on is the doc change in #1 and #2 — one paragraph and one line — not a redesign.

John Lauer · 2mo ago

Revising again, and this time toward a stronger conclusion than either of my earlier comments: --ai-thread should be a first-class CLI flag, and the env var should not be the recommended path for CLI callers at all. John's framing is the one that settles it:

"isn't it more work for the ai to then realize it has to set an env var? cuz if the ai calls ad and then 5 minutes later calls ad again, doesn't it lose that env var, so it has to set it again and what we're really doing is leaving a chance the ai thread forgets to set that env var in one command and then call ad cli in the next command and its just slowing down the ai? why not just keep it simple and have this be a cli variable just like all other ad cli variables? we already have --target... so how is adding the ai-thread all that different?"

The env var's only stated advantage does not hold for a CLI caller

The doc's case for it is explicit: "it means you cannot forget it on the one call that happens to be the scary one." That is true only if the variable survives between calls. Measured:

Exported in one invocation, inherited by children of that invocation: yes.
Present in a later invocation (the "5 minutes later" case, a new shell): no — empty, and the call is refused.

So a CLI-driven AI must re-set it on every single invocation. The mechanism therefore provides exactly none of the forgetting-protection it is recommended for, while being less discoverable than a flag. What is left is a per-call obligation expressed in the least visible way available.

Two concrete costs, one of them silent

Forget → refusal → wasted round trip. Every miss costs a full relay round trip plus a retry. For an agent making dozens of AD calls across a long session, that is a recurring tax on exactly the calls where the agent is busy thinking about something else.
Worse: a STALE env var misattributes silently. A value inherited from a parent process or copied from a script keeps satisfying AD while naming the wrong thread. A refusal is loud and self-correcting; misattribution is neither, and it corrupts the Activity Log the feature exists to make trustworthy. A flag at the call site is visible and auditable every time.

`--target` is the precedent, and it is the same problem

--target already exists for the structurally identical situation: several possible addressees, so the caller must disambiguate per call. It is a global flag, agents already pass it habitually, and nobody proposed solving it with ADOM_TARGET as the primary path. Identity is the same shape of problem, so:

# what an agent writes today (env var, invisible, must be repeated):
ADOM_AI_THREAD="pup bridge maintenance" adom-desktop --target AdomLapper <verb> '<json>'

# what it should be able to write (one flag, next to the flag it already passes):
adom-desktop --target AdomLapper --ai-thread "pup bridge maintenance" <verb> '<json>'

Zero new concepts, discoverable in --help, and it composes with --target the way agents already think.

Revised asks, in priority order

Add global --ai-thread "<name>" (and optionally --container-name), alongside --target. This is the whole ask; everything else follows from it.
Make the flag the documented default for CLI callers. Keep ADOM_AI_THREAD supported and recommend it only where it is genuinely correct: a single-purpose process (CI, a daemon, a wrapper) that sets it once and truly keeps it.
Drop the "you cannot forget it" justification, or scope it to the long-lived-process case, since it does not survive contact with a per-invocation CLI caller.
Still keep the misconfiguration detector idea (flag when many concurrent connections report the same aiThread) — with a flag it becomes rarer, but a copy-pasted flag value has the same failure mode as a shared env var.

ADOM_ENV-style config is right for things that are properties of an environment. Identity is a property of a caller, and in Adom many callers share one environment. The flag matches the fact.

Sorry for the three-comment path to this. The short version: my first comment overstated it as an AD design flaw, my second under-corrected to "nice-to-have ergonomics," and the measurement that resolves it is that the env var provides no persistence for CLI callers, so the flag is not ergonomics — it removes a recurring failure mode. Identity remains attribution, never authorization, whichever way it is passed.

Adom Wiki CLI