feat(slack): adopt existing threads before readiness

[nahyeongjin1]

Summary

• add an opt-in prepare -> bind -> activate lifecycle so a live GJC session can adopt an existing Slack thread before publishing readiness
• route bind-thread through the running Slack daemon's exact owner/generation command channel; the CLI never writes conversation mappings directly
• fail closed on stale authority, uncorroborated success, ambiguous post-rename durability, conflicting envelope/payload identity, and stale coordinator activation state
• drop SDK control/lifecycle frames before every chat mutation while preserving ordinary unbound sessions' existing immediate-readiness behavior

Safety invariants

• prepared sessions expose endpoint authority but publish no readiness or Slack root until activation
• a new mapping is committed only after the exact session generation and daemon owner are re-proved inside the store transaction
• a successful command response is corroborated by the durable mapping; post-commit settlement uncertainty returns unknown, not a definitive rejection
• activation re-proves the endpoint generation and daemon-owned binding, publishes readiness exactly once, and never synthesizes already from stale/terminal durable state
• conflicting or malformed control-frame envelope/payload identities are inert before root, post, mapping, resume, close, or action handling
• workspace/channel remain configured daemon authority and are not accepted as binding input

Verification

• bun test packages/coding-agent/test/sdk-session-readiness-lifecycle.test.ts packages/coding-agent/test/sdk-slack-thread-binding.test.ts packages/coding-agent/test/sdk-chat-daemon-control-frames.test.ts packages/coding-agent/test/notify-thread-commands.test.ts — 60 pass / 0 fail / 458 assertions
• coordinator MCP non-real-broker subset — 58 pass / 0 fail / 289 assertions, including 14 prepared-activation production-route regressions
• changed-path Biome — 28 files clean
• bun --cwd=packages/coding-agent run check:types — pass
• daemon generation manifest validation — pass
• generated docs-index check — pass (120 docs)
• git diff --check — pass
• source runtime smoke — pass
• cargo fmt --all -- --check — pass on the frozen Rust bytes before the final TypeScript-only remediation rounds
• independent immutable review — no blocking code finding; all four historical blockers resolved; exact identity attestation recorded separately before commit

The local root check:ts aggregate was not claimed green: its long SDK adapter-disposition matrix exhausted the Hermes gateway's file-descriptor budget and was terminated after its emitted receipts remained green through the observed range. Focused/package checks above completed normally; PR CI should run the canonical aggregate in a clean runner.

Scope and dependency

This is one atomic opt-in delivery contract: lifecycle preparation without daemon-owned adoption is unusable, and adoption without activation/control-frame containment is unsafe. Same-UID kernel peer authentication is intentionally a separate platform-hardening concern.

Live Slack verification is intentionally not included. The installed v0.12.7 runtime still hits the conversations.replies request-encoding failure addressed by #3753; merge/install/live smoke remain separate from this source PR.

[nahyeongjin1]

@Yeachan-Heo — Maintainer review requested for exact head 577166067cfa63e8342e76c766d587388ccf2ed0.

• Fresh immutable source review: PASS; four historical blockers resolved; merge_blocked=false
• Exact candidate identity: 35 paths, digest 81f2e510bdb1117017f0babedb415741b9211425ed65c378a9ffdd7f39f9db5c
• Focused feature suites: 60/60; coordinator subset: 58/58; package types/Biome/generation/docs/smoke clean
• GitHub reports MERGEABLE; no rebase requested

Fork workflows require maintainer approval:

• Dev CI: https://github.com/Yeachan-Heo/gajae-code/actions/runs/30892830217
• Public site sync: https://github.com/Yeachan-Heo/gajae-code/actions/runs/30892830377

The PR-author token cannot use the formal requested-reviewers endpoint (404), so this single comment is the review notification. No duplicate mutation was opened.

[nahyeongjin1]

Rebased onto current dev (f9dffed426a433bf4948bdeb8fb2fe76f25ef5ca) and force-pushed; the branch was five commits behind. New exact head: c0af9cd9f1f7e42c12be13f88e4bcdb26e6c92d7. The rebase was clean — no conflicts, one commit, 35 files / +4977 −56.

Exact-head validation, run in a disposable worktree checked out at exactly c0af9cd9:

$ bun --cwd=packages/coding-agent run check
$ biome check . && bun run check:types
Checked 2523 files in 1213ms. No fixes applied.
$ tsc -p tsconfig.json --noEmit
# exit 0

$ bun test \
    packages/coding-agent/test/sdk-slack-thread-binding.test.ts \
    packages/coding-agent/test/sdk-session-readiness-lifecycle.test.ts \
    packages/coding-agent/test/notify-thread-commands.test.ts \
    packages/coding-agent/test/sdk-chat-daemon-control-frames.test.ts \
    packages/coding-agent/test/coordinator-mcp-server.test.ts
 126 pass
 0 fail
 787 expect() calls

GitHub CI has not run on this PR. The workflow runs exist but are held for maintainer approval because this is a fork PR — Dev CI and Public site sync both report conclusion: action_required, which is why the check-runs API returns total_count: 0. Approving the run on c0af9cd9 would produce real CI evidence.

Ready for review.

[nahyeongjin1]

dev moved to 732856b3 (#3813) and this branch went CONFLICTING. Rebased and force-pushed; new exact head 0fdceb5e317f75c524dfedfab5f7fe766602b9ed.

The only conflict was in packages/coding-agent/src/internal-urls/docs-index.generated.ts, which is generated — both sides had added a doc, so the file was not hand-merged. It was regenerated from the merged tree:

$ bun run generate-docs-index
Generated src/internal-urls/docs-index.generated.ts (120 docs)

Re-running the generator against the committed result produces no diff, so the file matches what CI would generate.

Exact-head validation in a disposable worktree checked out at 0fdceb5e:

$ bun --cwd=packages/coding-agent run check
$ biome check . && bun run check:types
Checked 2524 files in 1147ms. No fixes applied.
$ tsc -p tsconfig.json --noEmit
# exit 0

$ bun test \
    packages/coding-agent/test/sdk-slack-thread-binding.test.ts \
    packages/coding-agent/test/sdk-session-readiness-lifecycle.test.ts \
    packages/coding-agent/test/notify-thread-commands.test.ts \
    packages/coding-agent/test/sdk-chat-daemon-control-frames.test.ts \
    packages/coding-agent/test/coordinator-mcp-server.test.ts
 126 pass
 0 fail

mergeable is back to MERGEABLE. GitHub CI still has not run on this PR — the workflow runs report conclusion: action_required because it is a fork PR, so approving the run on 0fdceb5e is what would produce real check evidence.

[Yeachan-Heo]

REQUEST_CHANGES — exact-head hostile review

Reviewed exact head 0fdceb5e317f75c524dfedfab5f7fe766602b9ed against exact base/current dev 732856b3ccb3fade6e9fbc17908a4fbca5a7682f. GitHub reports MERGEABLE / UNSTABLE.

Blocking findings

1. HIGH — a reported cancellation can still commit later (chat-daemon-command-channel.ts:435-436, 626-641). The timeout path returns cancelled after winning the response claim, but request deletion suppresses every error and the response claim is then also removed. If request unlink fails while response unlink succeeds, the still-live request can be served later and commit after the CLI was told no mapping changed. Cancellation must remain fenced when cleanup is uncertain.

2. HIGH — prepared sessions still accept mutating controls before activation (sdk/host/host.ts:225-232, 350-367). Deferred mode withholds only session_ready; every control_request, including prompt controls, still reaches the application control dispatcher without checking prepared/ready. The new pre-activation interval therefore makes activation optional for input admission.

3. HIGH — binding/activation authority has restart and endpoint-swap ABA gaps (existing-thread-readiness.ts:60-71, 108-132; slack-thread-binding.ts:140-166; chat-daemon-runtime.ts:447-461, 522-528). Host generations restart from 1 in a new process. Activation proves endpoint A, rereads and connects to whatever endpoint exists, then authorizes only by session ID plus numeric generation. Binding also ignores the indexed default versus chat scope, endpointMtimeMs, and restart-stable incarnation, while the runtime compares only generation to its captured attachment. A replacement process can repeat the generation and consume an old binding; valid .gjc/state/chat/sdk sessions can also be rejected or checked against the wrong endpoint.

4. HIGH — malformed preparation input fails open (coordinator-mcp/server.ts:4184-4199, 5078-5081, 5124-5130). Runtime dispatch does not validate the advertised schema. prepare_existing_thread: "true" is silently treated as false, so a supplied initial prompt starts an ordinary ready session instead of being rejected before mutation.

5. HIGH — failed defensive cleanup is swallowed and terminally cached (coordinator-mcp/server.ts:4236-4248). If the broker returns a live non-prepared session, the compensating session.close failure is ignored; the created session identity is discarded and the start failure is sealed by idempotency. A ready, untracked session can remain alive while exact retries only replay the cached error.

6. MEDIUM — shutdown does not fence in-flight activation (sdk/host/host.ts:273-319). stop() leaves #started true while awaiting broker unregister. An already-running activation gate can resolve during that await and publish session_ready after shutdown has begun.

Dependency and CI

• This exact head adds mandatory conversations.replies root verification but does not contain #3753's form-encoding fix. The contributor's live evidence shows the current JSON path returning invalid_arguments; #3753 must land first or be folded into this branch before the feature is deployable.
• Exact-head Dev CI run 30936007622 and Public site sync run 30936006416 are both action_required; the commit has 0 check runs and 0 statuses. No CI was approved, rerun, or controlled in this review.

Local evidence

• bun --cwd=packages/coding-agent run check — passed.
• Five focused feature files — 126 pass / 0 fail / 787 assertions with a 20s test timeout after one default-timeout-only rerun; the isolated activation subset was 14/14 green. This local evidence does not replace exact-head CI.

Contributor ledger

nahyeongjin1 remains the owner of this #3816 feature lane. The same contributor's #3753 is the canonical form-encoding prerequisite; #3824 was self-closed as its duplicate, so no replacement/duplicate implementation lane is needed.

gajae.pr-review-verdict.v1 REQUEST_CHANGES head:0fdceb5e317f75c524dfedfab5f7fe766602b9ed base:732856b3ccb3fade6e9fbc17908a4fbca5a7682f reviewer:Yeachan-Heo

Signed: GJC / Yeachan-Heo via GitHub CLI

[Yeachan-Heo]

REQUEST_CHANGES — exact-head independent re-review (glm-deepseek lane)

Fresh hostile review at exact head 0fdceb5e317f75c524dfedfab5f7fe766602b9ed against exact base/current dev 732856b3ccb3fade6e9fbc17908a4fbca5a7682f. GitHub reports MERGEABLE / UNSTABLE. This re-confirms the prior REQUEST_CHANGES disposition under a new lane; each blocking finding was independently re-derived against the exact head, with one refinement on Finding 3.

Blocking findings (all re-confirmed)

1. HIGH — a reported cancellation can still commit later. submitAgainstScope claims the response entry and returns cancelled (chat-daemon-command-channel.ts:614), then the finally unlinks the request before the response (:640-641). Both unlinkScopedEntry calls suppress errors (:435-436). If the request unlink fails while the response unlink succeeds, the orphaned request remains servable: a later serveChatDaemonCommandsAgainstScope sees no response entry (:735), re-claims it, and the daemon's commitAuthority re-reads the still-present request (:810) and commits. The submitter was already told cancelled — no mapping changed. Cancellation must remain fenced when cleanup is uncertain.

2. HIGH — prepared sessions still accept mutating controls before activation. host.ts #onFrame dispatches control_request straight to this.#options.control with no prepared/ready gate (:353-367). Deferred readiness withholds only session_ready (:225-231); it does not withhold control_request, query_request, or register_provider. The pre-activation interval therefore makes activation optional for input admission, and no test covers a control_request arriving on a prepared-but-not-ready session. Confirmed untested: sdk-session-readiness-lifecycle.test.ts has zero control_request assertions.

3. HIGH — binding/activation authority has restart and endpoint-swap ABA gaps. Refined from the prior review: the daemon generation is a static constant (slack: 25, chat-daemon-control.ts:50-53), not a per-process counter, and incarnation is OS-proven process-start provenance (process-incarnation.ts:108-121, Linux /proc/<pid>/stat start ticks — monotonic, restart-unique). So the owner tuple {ownerId, pid, incarnation, generation} is restart-distinct. The genuine ABA surface is the endpoint generation, which is the SDK session host's event-ring generation and is not globally unique across distinct sessions: activation proves endpoint A, activatePreparedSession re-resolves and connects to whatever endpoint the index currently reports, then authorizes by session ID + numeric generation (existing-thread-readiness.ts:104-136; slack-thread-binding.ts:140-166). The binding fence also does not consult endpointMtimeMs or the default/chat scope that attach() uses to reject stale/foreign attachments (chat-daemon-runtime.ts:447-461). A replacement process that repeats the endpoint generation, or a valid .gjc/state/chat/sdk session checked against the wrong scope, can consume or reject an old binding.

4. HIGH — malformed preparation input fails open. coordinator-mcp/server.ts:4184 uses args.prepare_existing_thread === true. The MCP dispatcher passes params.arguments verbatim with no schema validation (:5124-5130), so a client sending prepare_existing_thread: "true" (string) evaluates to false: the session starts as an ordinary ready session and accepts the initial prompt, instead of being rejected before mutation as the schema implies.

5. HIGH — failed defensive cleanup is swallowed and terminally cached. When the broker returns a live non-prepared session, the compensating session.close failure is swallowed (.catch(() => undefined), server.ts:4239-4244), the session identity is discarded, and the thrown SdkClientError is caught by withToolIdempotency's .catch(error => sdkError(error)) (:2690) and sealed as completed (:2692-2697, since isNonterminal defaults to () => false). A ready, untracked session remains alive while exact retries replay the cached error.

6. MEDIUM — shutdown does not fence in-flight activation. #stopStartedHost awaits broker.unregister while #started is still true (host.ts:313-319); #started is set false only after that await. An activation gate resolving during that await passes the post-gate #started re-check (:290) and publishes session_ready after shutdown has begun.

Dependency and CI

• This exact head adds mandatory conversations.replies root verification (findMessageByTimestamp) but does not contain #3753's form-encoding fix — at this head #request still serializes JSON. The contributor's live evidence shows that path returning invalid_arguments; #3753 must land first or be folded into this branch before the feature is deployable.
• Exact-head Dev CI run 30936007622 and Public site sync run 30936006416 are both action_required (fork PR). Head has 0 check runs and 0 statuses; commit state pending. No CI was approved, rerun, or controlled.

Exact-head local evidence (independently reproduced)

Detached worktree at 0fdceb5e, natives rebuilt from source:

• bun --cwd=packages/coding-agent run check — pass (Biome 2524 files clean + tsc --noEmit exit 0).
• Five focused feature suites — 126 pass / 0 fail / 787 assertions: sdk-session-readiness-lifecycle (9/29), sdk-slack-thread-binding + sdk-chat-daemon-control-frames + notify-thread-commands (51/429), coordinator-mcp-server (66/329). This local evidence does not substitute for exact-head CI.

Contributor ledger

nahyeongjin1 remains the owner of this #3816 feature lane. The same contributor's #3753 is the canonical form-encoding prerequisite; #3824 was self-closed as its duplicate, so no replacement/duplicate implementation lane is needed.

gajae.pr-review-verdict.v1 REQUEST_CHANGES head:0fdceb5e317f75c524dfedfab5f7fe766602b9ed base:732856b3ccb3fade6e9fbc17908a4fbca5a7682f reviewer:Yeachan-Heo lane:glm-deepseek

Signed: GJC / Yeachan-Heo via GitHub CLI

[nahyeongjin1]

All six blocking findings addressed. New exact head 5a17d79b5b2a76ab29ac79c0e301d0d253e7072a, rebased onto current dev b014ec1d. The fixes are an isolated second commit so they can be reviewed apart from the feature.

Thank you for the Finding 3 refinement — the owner tuple really is restart-distinct, and pointing at the endpoint generation instead sent me to the actual window.

1 — cancellation fencing. unlinkScopedEntry now reports whether the entry is gone, and the response claim is released only once the request is proven removed. When the request unlink fails the claim is deliberately left behind: a leaked response object is inert, whereas releasing it lets a submission already answered cancelled be re-claimed and committed.

2 — controls before activation. control_request is refused with not_activated while prepared. I confirmed first that activation travels as its own session_activate frame type, so the gate cannot deadlock a prepared session. Two cases added to sdk-session-readiness-lifecycle.test.ts, which you correctly noted had zero control_request assertions: one asserts the dispatcher is never reached and the refusal is structured, one asserts an activated session admits controls again.

3 — binding/activation authority. Two defects, one fix. resolveSessionBindingAuthority now derives the state-root scope exactly as the runtime's attach() does (default / chat, underivable is not authority), requires the index to carry endpointMtimeMs, reads discovery at that scope, and stats the endpoint file to prove it is the one the index observed. It then returns the proven endpoint, and activatePreparedSession uses it instead of re-resolving. That removes the read-A-prove, read-B-connect window entirely, so a replacement process repeating the endpoint generation can no longer be connected to, and a .gjc/state/chat/sdk session is no longer read at the wrong scope.

4 — malformed preparation input. A non-boolean prepare_existing_thread is rejected with invalid_input before any mutation, instead of coercing "true" to false and starting an ordinary session that accepts the prompt.

5 — swallowed compensation. The compensating session.close result is now tracked. On failure the error names the session that may still be running and uses a distinct code, and the start_session idempotency call passes an isNonterminal predicate for it, so the key is not sealed and a retry can still reach the session. This uses the existing hook for exactly the case its own comment describes: a response that states only that the outcome could not be observed.

6 — shutdown fencing. #stopping is set before the first await in teardown and is checked in the activation re-check, so an activation resolving during unregister can no longer publish readiness after shutdown began.

Verification at 5a17d79b

$ bun test \
    packages/coding-agent/test/sdk-session-readiness-lifecycle.test.ts \
    packages/coding-agent/test/sdk-slack-thread-binding.test.ts \
    packages/coding-agent/test/sdk-chat-daemon-control-frames.test.ts \
    packages/coding-agent/test/notify-thread-commands.test.ts \
    packages/coding-agent/test/coordinator-mcp-server.test.ts
 128 pass
 0 fail

$ bun --cwd=packages/coding-agent run check     # Biome 2524 files + tsc, clean

Not tested, stated plainly

Findings 1, 3, 5 and 6 are covered only by the existing suites continuing to pass, not by dedicated cases. Finding 4's rejection path has no test either — the coordinator harness needs setup this change did not add. I would rather say so than let the green number imply coverage it does not have. If you want those pinned before merge, say which and I will build the harness.

Dependency and CI

The #3753 prerequisite still stands: this branch does not contain the form-encoding fix, so conversations.replies returns invalid_arguments until #3753 lands. I have deliberately not folded it in — duplicating it here would recreate the #3824 situation you already had to close.

CI remains action_required on the fork. Note dev moved to b014ec1d (#3835) since your review, so the earlier heads on my other PRs are now behind that base; I have not rebased those, because doing so would invalidate #3753's MERGE_READY verdict and #3821's pending re-review for no content change. Say the word and I will realign them just before you admit CI.

---

Correction. The head SHA in the first line of this comment was wrong when posted: I wrote 5a17d79bd7e8a3ee08b58f6b7f7d0f38b30e1a5e, which is not a commit in this repository. The real head is 5a17d79b5b2a76ab29ac79c0e301d0d253e7072a. The short prefix 5a17d79b is correct — I extended it to a full SHA from memory instead of reading it back, so it looked plausible and was not. The comment above is now corrected in place.

Nothing else in it changes: the branch, the six fixes, and the verification all refer to that same commit. I re-checked every other full SHA I have posted across #3753, #3816, and #3821 — ten of them — and each one resolves to a real commit. This was the only fabricated value.

Given this repository reviews by exact head, a made-up SHA is worse than a typo, and I am sorry for putting one in a verdict-bearing comment.