fix(create): derive primary_path and channel in api mode, as the dry-run plan promises

[andrei-hasna]

Fixes todos HC-00724, found while executing HC-00723.

projects create --dry-run reported a canonical primary_path and a derived integrations.conversations_channel; the identical real create in api/cloud mode produced primary_path: null and integrations: {}. projects store inspect then read primary_is_canonical: false / exists.workspace: false.

Five projects exist today with no workspace behind them — andrei-tasks, andrei-agenda, reges-notary, financial-advisory-babos, mallorca-vacation. They are deliberately not repaired here: that is data remediation and belongs in its own change with its own verification.

A dry run that promises what the real run does not deliver is worse than no dry run. Its entire purpose is to let an operator check before committing, and this one returned a confident, plausible plan that the create then silently failed to honour. Nobody looked, because the plan said it was fine.

Root cause

Both derivations live in the local planner plannedWorkspace() — deriveWorkspacePath (workspace-plan.ts:194) and deriveProjectChannel (:201-212). executeWorkspaceCreation passes plan.workspace_input, carrying both, into createProject (:629-631), and the CLI already wires createProject: (input) => store.createProject(input) (workspaces.ts:1548).

But the api/cloud branch (workspaces.ts:1485-1526) short-circuits before the planner and calls store.createProject directly with primary_path: opts.path ? resolve(opts.path) : undefined and the raw merged integrations. --dry-run is deliberately routed to the local planner (comment at :1481-1484), so the plan and the create computed different things by construction — not by an ordering accident.

The server cannot cover for this. src/serve/pg-store.ts:

:468   const primaryPath = input.primary_path ?? null;     // derives nothing
:489   json(input.integrations ?? {}),                     // derives nothing
:457   const id = input.id ?? generateWorkspaceId();       // honours a client id

So the client is the only place these can be computed, and it may safely generate the id it derives the path from.

Ruling: the CREATE was wrong, the PLAN was right

Asked to decide explicitly. Local-mode create already persists both fields — measured, same flags, isolated temp home, PROJECTS_CHANNEL_ENSURE=0:

"primary_path": ".../home/workspaces/wks_iptt2zehghi9",
"integrations": { "conversations_channel": "local-probe-alpha" }

So the api branch was the outlier: the transport was silently changing the registry outcome for the same command and flags. The plan matches what local create actually does. Additionally, a null primary_path is the state the documented repair (projects update --path) exists to leave, so it is not a valid resting state the plan should be taught to predict.

I considered the counter-case for the channel specifically — project-channel.ts:250-254 says a derived name is "a current opinion, not a commitment" and that ensure stopped writing the link, which argues the plan should stop promising it. I rejected it: that comment governs read surfaces (projectChannelSummary), and create has always persisted the channel in local mode. Consistency across transports wins over making cloud rows quietly thinner than local ones.

The fix

• Extract deriveWorkspaceRegistryFields() from plannedWorkspace so both transports compute the path and channel identically, rather than one owning derivations the other lacks. Third copy avoided deliberately — deriveWorkspacePath is already duplicated in db/workspaces.ts:683.
• Call it from the api/cloud branch, deriving from a client-generated id so the row and the directory agree on one id.
• Resolve root/recipe rows (not just ids) in api mode, so the root path template and kind defaulting apply there too.

Measured before/after — the installed 0.1.97 CLI that caused the incident

Against a stub that mirrors pg-store semantics (honours a client id, stores both fields verbatim). Before, the POST body the installed CLI sent:

{ "name": "Divergence Probe", "slug": "divergence-probe", "tags": [] }

No id, no primary_path, no integrations — result primary_path: null, integrations: {}.

After, from this branch:

{
  "id": "wks_otri1fp8ooal",
  "name": "Divergence Probe",
  "slug": "divergence-probe",
  "kind": "generic",
  "primary_path": ".../home-after/workspaces/wks_otri1fp8ooal",
  "tags": [],
  "integrations": { "conversations_channel": "divergence-probe" }
}

The path is tied to the id that was actually sent.

Tests

TDD. The new test failed first on the real defect:

expect(body.integrations?.["conversations_channel"]).toBe(plannedChannel);
Expected: "cloud-derive-probe"
Received: undefined

After the fix: 1 pass, 0 fail, 15 expect() calls.

It asserts transport parity — plan vs create on identical flags — and carries three controls so it cannot pass vacuously:

• the dry run itself must POST zero rows (toHaveLength(0));
• negative control 1 — an explicit --path still wins, so this derives a default rather than overwriting operator intent;
• negative control 2 — an explicitly linked conversations_channel still wins over the slug-derived default.

The path assertion is tied to the id actually sent (join(home, "workspaces", body.id)), so a path belonging to some other project's id cannot pass.

Why the existing coverage could not catch this: the cloud-create test at index.test.ts:1902 always passes --path. The corpus varied flags but never varied --path absent, and the divergence lives only on the defaulting path — coverage bounded by its axes, not its size.

Gates

bun run typecheck    rc=0

Full suite: 306 pass / 6 fail / 312 tests. All 6 failures are pre-existing 5s timeouts under load (/proc/loadavg 15.34 on 20 cores), not regressions. Verified by stashing this change and re-running the two affected files on unmodified origin/main:

run	tests	pass	fail
base (stashed, origin/main)	53	47	6
with this change	54	49	5

The after-failure set is a strict subset of the base set, and projects update, archive, unarchive, delete... failed at base and passed after — confirming these are flaky timeouts rather than deterministic failures.

Scope — deliberately excluded

• The five broken projects. Data remediation, separate change.
• store ensure refusing in api mode (workspaces.ts:2669-2675). A separate defect, filed rather than folded in. Its refusal is deliberate and reasoned, not an accident. It is also partially superseded by open fix(store): resolve the machine-local app store in api mode instead of faking empty #59, whose premise — the machine-local store is keyed by the same id in both transports — directly undercuts the "cloud project does not own it" rationale. Note this fix makes store ensure less needed: it is the repair, this is the prevention.
• Slug adjustment. In api mode the server may suffix a colliding slug (ensureUniqueSlug); the channel is derived from the client's slug, so it could drift on a collision. The path is unaffected (derived from the id, which the server honours), and the channel is derivable at read time via projectChannelSummary. Not worth a pre-flight round trip.

Not checked

• Not exercised against the live cloud registry — deliberately, since that creates real rows (the trap fix(cli): projects create honors registry flags and refuses local runtime flags in api mode (#27) #40's own commit message records). Evidence is the stub plus the server source.
• The hasna.contract.json conformance gate and no-cloud-scan (part of prepublishOnly) were not run.
• No individual verdict here re-verifies fix(store): resolve the machine-local app store in api mode instead of faking empty #59's claims; I read it only to avoid overlapping scope.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

[REVIEW] NO_GO — #60 @ 7442f87 — lens: correctness+security+gates, reviewer unresolved-account002 (1 of 1)

What I ran

• bun install — exit 0; setup only, 128 packages installed.
• bun run typecheck — exit 0; TypeScript gate passed (no test count applicable).
• bun run test — exit 0; 312 pass, 0 fail, 2125 assertions across 33 files.

What I read

• Exact candidate origin/main 471d2e7 through head 7442f87: the full diff for all three changed files.
• Surrounding create/planner code in src/cli/commands/workspaces.ts and src/lib/workspace-plan.ts.
• Server/store behavior in src/serve/pg-store.ts, src/store/project-store.ts, src/db/workspaces.ts, the workspace schema, input types, and project-channel derivation.
• The PR body and its explicitly excluded slug-adjustment case.

Blocking P0/P1 findings

• P1 correctness/data-integrity regression on a supported current path: API creation derives primary_path and an explicit integrations.conversations_channel from the client-side slug before ProjectsPgStore.createWorkspace() calls ensureUniqueSlug(). When a remote project already owns that slug, the server persists a suffixed project slug (for example name-2) but the new row remains explicitly pinned to the old unsuffixed channel. deriveProjectChannel() gives an explicit integration precedence, so the PR body's claim that the channel remains derivable from the returned slug is false: agents for the new project are directed into the existing project's channel. With a root whose path template includes {slug}, the same ordering derives the original project's path; the schema's primary_path TEXT UNIQUE then rejects the second create instead of using the server-final suffixed slug. Duplicate-name projects are a reachable supported case—the server deliberately suffixes conflicts—so this is not speculative. The client-derived registry fields must be based on the exact slug the server will persist, with the collision behavior covered by a regression test.

Non-blocking follow-ups

• None.

[andrei-hasna]

[FIX] Duplicate-slug API creation now derives defaults from the server-persisted slug

Commit: a7c32c7

The API client no longer pre-pins a rooted implicit path or derived conversations channel before PostgreSQL allocates a unique slug. ProjectsPgStore now derives missing registry fields after ensureUniqueSlug, while explicit --path and explicit integrations.conversations_channel values retain precedence.

Regression coverage exercises two API creates with the same slug under a {slug} root and verifies returned/persisted slug, primary path, and conversations channel agree. It also verifies explicit path/channel values still win.

Verification:

• bun test src/serve/pg-store.test.ts src/cli/index.test.ts — exit 0; 44 pass, 0 fail
• bun run typecheck — exit 0
• bun run test — exit 0; 314 pass, 0 fail
• staged secrets scan before commit and before push — exit 0 both times

Focused re-review remains with reviewer unresolved-account002; this is not a replacement verdict.

[andrei-hasna]

[REVIEW] GO — #60 @ a7c32c7 — lens: correctness+security+gates, reviewer unresolved-account002 (1 of 1)

Focused re-review

• Re-read only the previously named duplicate-slug defect, its fix, and direct regressions from 7442f87 to a7c32c7.
• The API client now sends the client-specific id-based path where safe but leaves slug-dependent rooted paths and implicit channel links unset.
• ProjectsPgStore.createWorkspace() derives the missing registry fields only after ensureUniqueSlug() selects the exact persisted slug.
• Regression coverage exercises a real duplicate rooted slug and proves returned slug, rooted path, and derived channel agree; explicit path and explicit channel controls still win.
• Verified the remediation commit uses the required canonical git identity and final Agent: unresolved-account002 trailer.

What I ran on this exact head

• bun install — exit 0; setup only, 128 installs checked with no changes.
• bun run typecheck — exit 0; TypeScript gate passed (no test count applicable).
• bun run test — exit 0; 314 pass, 0 fail, 2141 assertions across 33 files.

Blocking P0/P1 findings

• None. The sole prior P1 is fixed.

Non-blocking follow-ups

• The optional live-PostgreSQL test lane was not available because PROJECTS_TEST_DATABASE_URL is unset; it is not a declared repository gate. The PostgreSQL store behavior is covered by the typed query-client regression test and the full declared suite.

[andrei-hasna]

[REVIEW] NO_GO — #60 @ 7442f87 — lens: correctness+blast-radius, reviewer unregistered-subagent (dispatched by agent-ceo)

Candidate tree verified. Post-image blob shas at 7442f877:

1692dc4d68c39c12058810077df40181a28c291c  src/lib/workspace-plan.ts
8ac719d6db5ff6ce306729c07b651d1a4250aa3a  src/cli/commands/workspaces.ts
7147bf94b9a25d0f6ebe6a915137b0bd09c42b3e  src/cli/index.test.ts

Negative control — same path at base 471d2e7: 0d2292bea3d158667b4e6ea1f4a47a3bda62de61. merge-base --is-ancestor 471d2e7 7442f877 → rc=0, so no retarget/merge-result hazard.

Identity disclosure. conversations whoami inside this sub-agent returns Agent: agent-ceo / Source: env var (CONVERSATIONS_AGENT_ID) — the dispatcher's identity, inherited. No reviewer identity was registered for this dispatch, so this verdict posts under the dispatching seat's byline. Its independence is asserted, not provable. Flagging rather than inventing a registration.

---

Verdict in one sentence

The ruling is right and the fix is right, but on a slug collision the change silently persists another project's conversations channel — and the PR body's stated mitigation for exactly this case is refuted by the function it names.

---

The ruling is CORRECT — verified independently, before reading the PR body

I was given the question, not the diagnosis. I reached the same conclusion by three independent routes:

1. Local create already persists both fields. executeWorkspaceCreation passes plan.workspace_input — which carries the derived primary_path and derived integrations.conversations_channel — into createProject (workspace-plan.ts:629-631). So the plan is a truthful description of what local create does, not an over-promise. Teaching the plan to be quieter would have made it lie about the majority transport.
2. The module's own doc sanctions persisting at creation. project-channel.ts:43-44: "The link is established once, at project creation, or deliberately by an operator." The "opinion, not a commitment" language governs ensure writing the link back post-hoc. The author's reading is correct and is not merely their opinion.
3. The server derives neither, and honours a client id — verified end-to-end, not from the source alone. app.ts:171 passes the raw body straight to store.createWorkspace(body as never) with no schema stripping; pg-store.ts:457 input.id ?? generateWorkspaceId().

I ran the real ProjectsPgStore against a real scratch Postgres, driven by the actual POST body the candidate CLI emits:

row[0] PERSISTED slug="collide-me"  id="wks_1n7rsagzhpkj"
row[0] PERSISTED primary_path="/tmp/pr60-probe-OFOA2D/home/workspaces/wks_1n7rsagzhpkj"
row[0] id-echoed? true   path-matches-own-id? true

The client-generated id is safe. It is echoed, the derived path matches the row's own id, and an id collision fails loudly — pg-store.ts:497 converts a duplicate-key violation into a ValidationError, never an upsert. There is no orphan-path hazard.

workspaceSlugify (client) and slugify (server) are character-for-character identical, so the client imposing the slug introduces no drift there.

Repo projects: no new hazard. deriveWorkspacePath is untouched by this diff — only its call site moved. --kind repo without --path/--root already resolved to a store path in local mode on main; the PR makes api match local, which is the stated goal. Whether kind: repo should default to a store path is a pre-existing question in the local planner, not introduced here.

The regression test genuinely discriminates — I ran the candidate's test file against the base tree:

NEWTEST_ON_BASE_RC=1
Expected: "cloud-derive-probe"
Received: undefined
(fail) create in cloud mode derives the same primary_path and channel the dry-run plan promises
 0 pass  1 fail

and on the candidate: NEWTEST_ON_CAND_RC=0, 1 pass 0 fail. Two-sided. Not vacuous.

---

BLOCKER (P1) — the collision path writes a permanently wrong channel, and it survives revert

The PR body excludes this explicitly:

Slug adjustment. […] the channel is derived from the client's slug, so it could drift on a collision. The path is unaffected […] and the channel is derivable at read time via projectChannelSummary. Not worth a pre-flight round trip.

The path half is right. The read-time-derivable half is wrong, and it is the half the dismissal rests on.

Measured — two projects create runs, same name, real CLI → real ProjectsPgStore → real Postgres. Both creates exit 0:

body[0] slug="collide-me"  integrations={"conversations_channel":"collide-me"}
body[1] slug="collide-me"  integrations={"conversations_channel":"collide-me"}

row[0] PERSISTED slug="collide-me"    conversations_channel="collide-me"   AGREE? true
row[1] PERSISTED slug="collide-me-2"  conversations_channel="collide-me"   AGREE? false

pg-store.ts:459 wraps any slug — explicit or derived — in ensureUniqueSlug, while :489 stores integrations verbatim. So row[1] permanently claims row[0]'s channel.

Now the dismissal, tested directly against projectChannelSummary:

row persisted by THIS PR  -> {"channel":"collide-me","source":"integration"}
row as MAIN leaves it     -> {"channel":"collide-me-2","source":"derived"}

Read-time derivation does not rescue it — persisting the link is precisely what disables it. deriveProjectChannel checks the explicit link first and returns before ever reaching the slug (project-channel.ts:232-238). On this path the change converts a self-healing absence into a permanent wrong link: main resolves collide-me-2 correctly today; after this change it resolves to another project's channel forever.

Why this blocks rather than lands as a follow-up — per project-channel.ts:40-44, that write "is one-way: it would survive a revert of the very change that produced it." Reverting this PR does not undo rows written in the interim. A defect that outlives the revert of its own cause is not a normal follow-up, and it lands in the production registry silently, at exit 0.

Reachability is real, not theoretical. Live registry, read-only, 2528 of 2534 rows read: 50 distinct stems have ever been deduped. Stripping the bulk test-fixture families (http-compact-project ×122, session-app ×67, …), 14 stems collided ≤3 times and they are production names — hasna-repos, hasna-machines, platform-alumia, open-emails, hasna-xyz-infra, iproj-papercuts, hasna-shield. Minority path, but routine.

Root cause worth naming: the server treats slug as a suggestion (ensureUniqueSlug) but primary_path as an assertion (unique constraint → hard 400). This change derives a field from the suggestion and sends it as an assertion. Three remedies, cheapest first — the fix is small, no redesign:

1. Don't send a derived conversations_channel in api mode; the control above shows read-time derivation then yields the correct name. Keeps the primary_path fix, which is what actually caused the incident.
2. Reconcile after the create from the slug the response carries — a post-flight PATCH only on the collision path, not the pre-flight round trip the body rejects.
3. Make the server treat an explicit slug as an assertion, matching how it already treats primary_path.

Why the new test cannot see this: its stub echoes the slug verbatim — "Mirror the real server: it echoes the client's id and stores primary_path/integrations verbatim" — but the real server dedupes the slug. The stub is faithful on every axis except the one the residual lives on. The PR body already invokes the axes-not-size rule against the prior coverage; it applies to the new test too. A slug-dedup axis in the stub would have caught this.

---

Non-blocking findings

N1 — --root + colliding name now hard-fails where main succeeded. Measured: the client renders root.path_template with its un-deduped slug, both creates produce /srv/projects/root-collide, and the second is rejected — ValidationError: workspace conflict: duplicate key value violates unique constraint "workspaces_primary_path_key". This is a safe failure (single INSERT, no partial write) and arguably correct, but it is a behavioural change not mentioned in the body. My initial hypothesis was that two rows would share one directory; the unique constraint refutes that. Worth a line in the body.

N2 — this repo has no pull_request CI. grep -rn "pull_request" .github/ → rc=1; only deploy.yml (push: [main] + tags) and release.yml (push: tags) exist. gh pr checks 60 returns one third-party [code]smith skipping. Nothing gates this PR automatically — do not read a green PR page as a gate. The real gates are release.yml (typecheck/test/build) and prepublishOnly.

N3 — the flake analysis is correct; I reproduced it independently. Isolated temp home + env -u on all eight api-mode selectors, identical command both trees:

tree	rc	tests	pass	fail
base 471d2e7	1	311	306	5
candidate 7442f877	1	312	306	6

Base's 5 failures are a strict subset of the candidate's 6; the extra is top-level list hides eval fixtures…, which is unrelated to the diff (local-mode cleanup-evals). It fails at [5051.36ms] against a 5000 ms budget under /proc/loadavg 23.50 on 20 cores, and passes alone on both trees: EVAL_CAND_RC=0 / 1 pass, EVAL_BASE_RC=0 / 1 pass. Load-induced, not a regression. Note the suite is red on main under load — pre-existing and out of scope here. bun run typecheck → rc=0 on the candidate.

N4 — the server store is untested by bun test. pg-store.test.ts:19 gates live CRUD on PROJECTS_TEST_DATABASE_URL, which is unset, so only two pure-helper tests run. The transport-parity property this PR is about has no server-side coverage in the default suite.

---

Not checked

• contracts:conformance and contracts no-cloud-scan . — not run by me either; the body says so honestly.
• The five damaged production projects — correctly scoped out; I did not verify their current state.
• I did not re-verify fix(store): resolve the machine-local app store in api mode instead of faking empty #59's claims.
• Whether any existing registry row already carries a slug/channel mismatch from another source.

Safety. No rows created in the live registry: projects list reported 2598 matching projects before and 2598 after. All probes ran in isolated temp homes against a scratch Postgres, dropped afterwards. Both review worktrees are clean; base restored to 471d2e7 with 0 dirty files.