feat!: require the org on every call, and remove KOSLI_ORG

[AlexKantor87]

Closes #8. Replaces #63.

The server was pinned to one organization at startup. Reaching another meant
restarting it with different env vars, and a call that named none went wherever
KOSLI_ORG pointed. That is the wrong shape for a server a customer runs
against several orgs.

org is now an input on both execute tools, and required for every action whose
path contains {org}, which is all but four. A call that names none is refused
rather than sent somewhere nobody chose. As a top-level input it also shows up
in the write-approval prompt.

KOSLI_ORG is gone, and so is the fallback it fed in KosliClient.buildUrl.
The org is an ordinary required path parameter now. An existing config that
still sets the variable starts fine; the value is simply not read.

An org can be named three ways: the tool input, params.org, and inside a
write's request body. They must agree. Also rejected: any value that is not a
non-empty string, and an org aimed at one of the four actions that take none.

search_actions no longer advertises org as a required path parameter, since
that told the model to put it in params. PUT /user/{org} is exempt, because
there the org is what the call writes.

Verified

105 tests. 24 mutations proven red one at a time. Driven live against
app.kosli.com through the built binary with no KOSLI_ORG in the environment,
covering two different orgs, the refusal, and every rejection path.

Follow-up

.github/workflows/claude-review.yaml:228 still names the removed fallback as
an invariant to preserve. Editing that file makes the review action skip its own
PR, so it goes separately.

🤖 Generated with Claude Code

[dangrondahl]

Automated review (Claude Code /code-review, run locally). Baseline: tsc --noEmit clean, 103/103 tests pass. Nine findings are inline, most severe first: search-actions.ts (put_user_default_org loses its argument), server.ts (tool org rejects null), execute-action.ts ×4, claude-review.yaml, catalog.test.ts, run-action.ts.

One finding has no diff line to attach to, since the file is unchanged:

src/client/kosli-client.ts:127-131 — the org is now decided in two places with two policies. resolveOrg trims, rejects blanks and non-strings, and detects conflicts. But on every sanctioned call with no org named, resolved.org is undefined, so it is buildUrl's params.org ?? this.config.org that actually picks KOSLI_ORG. That fallback is structurally live, not a leftover, and it has its own looser rules: null falls back, '' becomes a literal empty path segment (/environments/), arrays serialize as a,b. CLAUDE.md now says both "decided in one place" (line 30) and "preserve the fallback in buildUrl" (line 29). Suggested: have resolveOrg return the fully resolved name (named[0] ?? config.org for org-scoped entries), add org only when the entry takes one, and delete the special case in buildUrl so org is an ordinary required path param.

Lower-severity items not posted inline: conflict comparison is case-sensitive ("a" vs "A" rejected, consistent with the reject-rather-than-resolve intent); search results still show {org} in path while parameters omits it and the params description never mentions params.org; ORG_INPUT's description does not warn that the four non-org-scoped actions reject it; test/server.test.ts depends on live catalog id list_envs since createServer has no entries injection point; type FetchFn is declared in three files and ExecuteOptions/ActionArgs are exported with no importers.

Disclosure: one verification probe accidentally ran without a mock fetch and issued a real POST /api/v2/artifacts/cyber-dojo/body to app.kosli.com with a bogus token. It got a 403 and changed nothing.

[claude]

Claude finished @AlexKantor87's task in 5m 23s —— View job

---

Review complete

• Read CLAUDE.md and gather PR context
• Verify the fixes claimed in the latest comment actually landed
• Review src/org.ts, src/server.ts, execute-action.ts, kosli-client.ts, search-actions.ts
• Check the catalog invariants the new tests rely on
• Post review (5 inline comments)
• Build + test locally — not possible in this run, see Caveat

---

Verdict

The architectural invariants hold, and this is a clean resolution of the previous round. Nothing here blocks merge; the five inline comments are one substantive UX gap and four nits.

The invariants I checked, all intact: exactly three tools (server.ts:38,64,82), catalog untouched and not hand-edited, KosliClient still returns the { error: true, … } shape rather than throwing, compact JSON.stringify on both handlers, User-Agent preserved, fetchFn injection intact with no HTTP library added, searchActions still O(n) and dependency-free (the filter is per-result, not per-catalog-entry), read/write annotations unchanged, .js extensions on every relative import, manifest.json's version placeholder untouched.

The one invariant this PR deliberately inverts — the buildUrl org fallback — is removed in kosli-client.ts:129 and rewritten in CLAUDE.md:29 in the same commit, which is exactly the right way to retire a documented decision. buildUrl is now generic: no parameter gets special treatment.

Findings

	Where	What
🟡	execute-action.ts:132	The refusal names no recovery route, and the catalog has no list_organizations — get_user_default_org is the only way for a model to learn an org name. Naming it in the message keeps the policy and makes the refusal self-healing.
🔵	execute-action.ts:58	The collision guard sits below the !entry.requestBody return, so it skips the 81 entries with no declared body. set_user_default_org with params.body.org is neither compared nor rejected. Not a cross-org write — the URL org is always the resolved one.
🔵	server.ts:15	ORG_INPUT gives the count ("all but four") but not the rule, and does not warn that those four reject an org rather than ignoring it.
🔵	org.ts:26	The PUT /user/{org} exemption leaves one action where the two org channels mean opposite things and disagreement is a hard error. A hints.json entry is the mechanism already in the repo.
⚪	test/catalog.test.ts:24	Last hand-copy of the predicate src/org.ts was created to centralise.

Also: the commit is correctly marked feat!:, but the PR title is not. If this repo squash-merges, the title becomes the commit message and the breaking marker is lost from the changelog — worth renaming to feat!: require the org on every call, and remove KOSLI_ORG.

What's good here

• src/org.ts is the right shape for the fix. One predicate, one takesOrg, and a comment that says why three call sites have to agree and what breaks if they drift (Missing required path parameter that the model cannot see how to satisfy). The orgIsTheArgument path match over an id match, with the rename history as justification, is the kind of decision that pays for itself on the next catalog regeneration.
• The collision block generalised past the reported symptom. The finding was about org; unwrapBodyParam:69 rejects any body field colliding with a declared parameter, which closes the create_artifact/flow_name variant too — and it runs before the sibling check, so an undeclared sibling can no longer smuggle a nested org past it. test/catalog.test.ts:50 then pins the assumption that makes rejecting safe, matched against the whole stringified schema so an allOf-nested field still counts. The Unwrapped { params, collision } wrapper for the "a body with its own error field would read as a failure" problem is a genuinely subtle catch.
• delete withOrg.org instead of { ...unwrapped, org: resolved.org } — the fix for the {}-body-on-a-non-org-scoped-write case is one line and correct, and it copies before deleting so the caller's params is never mutated. It is covered from both directions ("sends no body for a write on an action that takes no org", "drops a null org instead of forwarding it as a query parameter").
• Extracting createServer earns its keep in a way run-action.ts did not. test/server.test.ts drives the real tools over InMemoryTransport, so the zod schema is now in the test path — which is the only reason the null finding is verifiable, and why z.string().nullable().optional() has a test that actually exercises the MCP validation layer rather than normalizeOrg directly.
• The refusal-path test coverage is unusually complete for a change like this: 22 new cases covering trimming on both channels, blank on both channels, non-string coercion, conflict, the "report the unusable value, not a disagreement" precedence, path-traversal encoding, multipart, and the non-org-scoped rejection from both channels.
• loadConfig warns on stderr rather than throwing or silently ignoring. Right call for a stdio transport, and the README's "starts fine, the value is ignored, and the server says so on stderr" matches the code exactly. Good docs discipline throughout — README, CLAUDE.md, docs/future/cli-mcp.md, and the repo tree listing all moved together, and the security note that a Kosli staff read does not imply a write would be allowed is a thoughtful addition.

Caveat

I could not run npm ci, tsc --noEmit, or npm test in this run — node_modules is absent and both commands need approval in this environment, so everything above is static reading of the diff plus grep against src/catalog.json. The catalog facts I did verify directly: all four non-org-scoped actions are GET (so the {}-body case is latent, as claimed), set_user_default_org is the only /user/{org} entry, no entry declares a parameter named body, and 81 entries have requestBody: null. CI's own ci.yml run is the authority on the 110-test claim.
· branch feat/org-parameter

[AlexKantor87]

All findings are in, and the change grew a step: the org is required on every call and KOSLI_ORG is gone.

From @dangrondahl: set_user_default_org keeps its org, since PUT /user/{org} writes the org rather than running in it. org: null is accepted. The org key is only set when one resolved. The predicate lives in src/org.ts. run-action.ts is deleted. The workflow edit is reverted and goes separately. The buildUrl fallback went with KOSLI_ORG.

From the bot: a request body may no longer name any of the action own parameters. Take create_artifact with flow_name at the top level and another in the body. The write went to the body value while the prompt showed yours. The removal is marked feat!: with a footer, and loadConfig warns on stderr when a stale KOSLI_ORG is set.

Kept: the four non-org-scoped ids stay pinned. That test caught three renames from #60 during the rebase.

search: grep -rn "...body" src/ --include=*.ts
tools/execute-action.ts:87 the one spread that flattens a body over the top level

search: grep -rn "params.org|withOrg.org" src/ --include=*.ts
tools/execute-action.ts:113 the read
tools/execute-action.ts:184-185 the write, the only other place the literal key appears

mutation: deleting the collision block -> 5 tests red; reverting the wrapper to a bare {error} -> the body-with-an-error-field test red; coercing non-strings -> 2 red; removing the refusal -> 4 red

110 tests. Verified against app.kosli.com with no KOSLI_ORG in the environment. Force-pushing next, so line anchors move.

[AlexKantor87]

Stopping here and handing to @dangrondahl. The five findings from the last run are all non-blocking, and none is being taken. Reasons, so the record is not just a resolve:

Refusal is a dead end. Fair, and the one I nearly took. A model that names no org gets told to set one, and there is no list_organizations to discover a name from. Left alone because the change is a message edit on a path the previous round already rewrote, and this branch has spent three rounds fixing fixes.

Collision guard skips entries with no declared body. For those, a stray params.body is an undeclared param, so it goes out as a query value and never touches the URL. Not a redirect.

ORG_INPUT gives the count but not the rule. True. The four actions reject an org with their own message naming the action, so the model finds out in one call.

PUT /user/{org} and a hints.json entry. The two channels agreeing is enforced, and disagreement is a hard error rather than a silent pick. A hint would improve the wording, not the safety.

catalog.test.ts spells the predicate inline. Deliberate. That test asserts a property of the catalog, so having it re-derive the rule rather than import it keeps it from passing because the production predicate changed.

no search: every one of these names a wording or placement choice, not a code class.
no mutation: nothing changed.