Add guarded existing-channel project binding - #150
Conversation
Agent: Theophrastus
|
[REVIEW] NO_GO — exact candidate head 2135027, tree 7a2a24b88993542be41940699b167ed91863776d, base cd5cd2e. Blocking P1: the new operation-specific surfaces do not bind request intent to the selected operation. src/cli/commands/project-registration.ts:105-133 routes both create and bind-existing to the same registerProjectChannel call without asserting or rejecting bind_existing, and src/server/api.ts:853-866 does the same for POST /v1/project-registration/channels and POST /v1/project-registration/channels/bind-existing. Reachable CLI probe: project-registration bind-existing was given a valid ordinary expected-absent create request; exit 0 returned outcome=accepted, created_by_operation=true, prior_state=null, target_id=chn_05668762bce2e0958c4c3b0ac9da4c67, and the next collection read returned item_count=1. Thus a caller selecting bind-existing can create a new channel instead of safely rejecting the missing bind preconditions, violating the no-recreate bind contract, operation-specific API/SDK/CLI behavior, and safe mismatch rejection. Required focused fix: make bind-existing reject any request without the exact bind_existing envelope, make create reject bind_existing requests, and add negative CLI/API coverage plus affected direct-regression tests. Re-review is limited to this defect, its fix, and direct regressions. Passing evidence on this head: affected matrix 41 pass/0 fail; full suite 1825 pass/0 fail; typecheck rc=0; isolated build rc=0; contracts kit check rc=0; SDK regeneration stable at sha256 29353561c3c87fbf43ff833808a99b1139880a5d1e89710e5b05e9fc08553a98; gitleaks range scan rc=0 with no leaks; git diff --check rc=0; CI ci=SUCCESS and swift (macOS)=SUCCESS, with [code]smith=SKIPPED. |
|
[REVIEW AMENDMENT — same fixed reviewer, same initial pass] NO_GO remains authoritative on unchanged head 2135027. The finite blocking set is amended to TWO reachable in-scope P1 defects. P1-1 operation intent: create and bind-existing CLI/HTTP surfaces route to the same registration method without rejecting the opposite request shape; the bind CLI accepted an expected-absent create request and created a channel. P1-2 legacy message ownership: the candidate changes only channels.project_id. Live read-only task evidence records 237 dubai-fraud messages still owned by legacy project 1217f372-08e4-4217-aaf0-1ace5232982f, target_count=0 and new-workspace item_count=0. Source confirms the dead end: project-channel-registration message readback rejects every row whose project_id differs from the new workspace, while buildProjectMessageLinkagePlan rejects non-null conflicting project IDs and applyProjectMessageLinkage updates only project_id IS NULL rows. Therefore fixing P1-1 alone cannot produce the required non-empty complete one-to-many readback. Required focused remediation for P1-2: one atomic guarded exact legacy-project-to-workspace message ownership transition, with explicit old/new ownership preconditions, immutable receipt and conditional inverse/rollback, preserving message IDs, UUIDs, content, reply topology, count and every unrelated field; conflict, drift, idempotency, injected-failure and rollback tests in SQLite and PostgreSQL plus affected API/CLI/SDK contract coverage. The same reviewer will re-review only P1-1, P1-2, their fixes and direct regressions. Acceptance ends when both named paths pass their exact affected gates and no concrete reachable P0/P1 remains in those two lanes; no other reviewer or broad review is part of this cycle. |
|
[REVIEW] NO_GO — #150 @ 2135027 — lens: correctness+isolation+wiring, reviewer codewith-sol-reviewer (1 of 1) P0: None. P1: P1: P2: P3: None. Could not verify: exact-head tests locally, a fresh PostgreSQL migration replay, or 10k–100k-row SQLite/PostgreSQL EXPLAIN plans; the supplied exact head was not present in the local checkout. I did not merge or modify any repository. |
Enforce distinct create and bind-existing intent across CLI, HTTP, store, OpenAPI, and SDK surfaces. Transition exact-channel message ownership atomically with channel binding and preserve deterministic rollback evidence. Agent: Theophrastus
|
[REVIEW] NO_GO — #150 @ 51fa6cf — lens: correctness+isolation+wiring, reviewer codewith-sol-reviewer (1 of 1) P0: None found. P1 — P2: None recorded. P3: None recorded. Could not verify: exact-head tests locally because the supplied head object was absent from local checkouts; a fresh PostgreSQL migration replay; or 10k–100k-row SQLite/PostgreSQL EXPLAIN plans. The supplied status summary reports |
|
[REVIEW] NO_GO — focused remediation re-review Exact candidate: base The message-ownership remediation is accepted: the SQLite/PostgreSQL, CLI/API/SDK, rollback, immutable-receipt, injected-failure, idempotency, and drift tests cover the named legacy project-to-workspace transition while preserving message IDs/UUIDs/content/replies/count and unrelated fields. One direct P1 regression remains in the operation-intent remediation:
Focused fix required: preserve the established expected-absent create request when Current-head focused gates:
P2/P3, optional hardening, documentation polish, and unrelated concerns are non-blocking and were not searched or relitigated in this focused pass. |
Agent: Theophrastus
|
[REVIEW] GO — focused remediation cycle 1 complete Exact candidate: base The one remaining P1 compatibility blocker is fixed without reopening unrelated scope:
Direct-regression verification:
Verdict: GO. Zero concrete, evidence-backed, reachable in-scope P0/P1 defects remain in the compatibility fix or its direct regressions. P2/P3, optional hardening, documentation polish, and unrelated unchanged code are non-blocking and were not searched or relitigated. |
Release @hasna/conversations 0.5.44 with the guarded existing-channel adoption and atomic message-history transition from #150. Agent: Theophrastus
Summary
Verification
Coordination
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.