Skip to content

Fix remote session state after clear and skill activation - #876

Merged
1jehuang merged 2 commits into
masterfrom
fix/latest-remote-session-bugs
Aug 12, 2026
Merged

Fix remote session state after clear and skill activation#876
1jehuang merged 2 commits into
masterfrom
fix/latest-remote-session-bugs

Conversation

@1jehuang

Copy link
Copy Markdown
Owner

Summary

  • re-register the fresh session as a new swarm root after /clear, without restoring old plan participation
  • carry the selected skill name on remote message requests and resolve it against the daemon registry before starting the turn
  • reject client/server skill registry mismatches instead of silently running without the requested skill

Verification

  • cargo test -p jcode-app-core handle_clear_session_replaces_runtime_handles_and_updates_shutdown_registration --lib
  • cargo test -p jcode-protocol test_message_request_roundtrip_preserves_images_and_system_reminder --lib
  • cargo test -p jcode-tui remote_skill_invocation_with_prompt_sends_remote_turn --lib
  • cargo check -p jcode-protocol -p jcode-app-core -p jcode-tui --all-targets

Fixes #874
Fixes #873

--- — Jcode agent (automated triage), on behalf of @1jehuang

shaikahad-tech added a commit to shaikahad-tech/jcode that referenced this pull request Aug 10, 2026
PR 1jehuang#876 added a new `active_skill: Option<String>` field to the
`Request::Message` variant (crates/jcode-protocol/src/wire.rs) and
updated every call site — except `src/cli/acp.rs:874`, which was
missed and broke compilation with E0063:

    error[E0063]: missing field `active_skill` in initializer of
    `jcode_tui::protocol::Request`
      --> src/cli/acp.rs:874:20
     874 | .send(&Request::Message {
         |                ^^^^^^^^^^^^^^^^ missing `active_skill`

The ACP (Agent Client Protocol) path has no skill concept, so
`active_skill: None` is the correct value — matching what every
other non-skill call site uses (client_api.rs, communicate_tests.rs,
client_lifecycle_tests.rs, randomized.rs).
@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown

Greptile Summary

This change propagates remote skill selections to the daemon and resets cleared remote sessions as new swarm roots. Testing reproduced a concurrency defect in client_lifecycle.rs: when two clients share a session, a later request can replace the skill used by an earlier queued request. This is not safe to merge until skill selection is isolated to each message turn.

Confidence Score: 4/5

Not safe to merge until concurrent messages cannot overwrite each other's selected skill.

The behavior was reproduced deterministically through the real message-processing and provider paths: the first queued request selected alpha, but both resulting turns used beta.

Files Needing Attention: crates/jcode-app-core/src/server/client_lifecycle.rs needs request-scoped or atomically applied skill selection during turn execution.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P1 finding and linked it to the corresponding review comment.
  • T-Rex produced a second proof for a posted P1 finding.
  • T-Rex completed a general-contract-validation proof confirming the interleaving order under forced same-session interleaving; the harness was reverted before completion with only the uploaded evidence remaining.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 First same-session turn can use the second request's active skill

    • Bug
      • A deterministic interleaving queued two requests behind the shared agent mutex: first with skill-alpha, then with skill-beta. After release, both real provider turns observed beta and not alpha: [(false, true), (false, true)].
    • Cause
      • start_processing_message stores active_skill in the shared Agent at lines 2876-2879, but the spawned turn later reacquires that same mutex before building/running the turn. A later request can overwrite the shared field in that gap.
    • Fix
      • Keep the selected skill request-scoped through turn execution, or capture and apply the selected skill atomically inside the spawned turn while holding the agent mutex before prompt construction.

    T-Rex Ran code and verified through T-Rex

Prompt To Fix All With AI
### Issue 1
crates/jcode-app-core/src/server/client_lifecycle.rs:2876-2879
**Shared active skill is overwritten between queued turns**

`active_skill` is written to the session-wide `Agent` before the message task is spawned, while the task later reacquires that same agent lock to build and run the turn. Another client attached to the same session can submit a message with a different skill during that gap. In a deterministic two-request run, the first request selected `skill-alpha`, the second selected `skill-beta`, and both provider turns used beta. The first client can therefore receive a response generated with a different skill than it selected. Keep the selected skill request-scoped, or capture and apply it atomically inside the spawned turn before prompt construction.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix: propagate active skills to remote s..." | Re-trigger Greptile

Comment on lines +2876 to +2879
if !agent
.lock()
.await
.set_remote_active_skill(active_skill.clone())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Shared active skill is overwritten between queued turns

active_skill is written to the session-wide Agent before the message task is spawned, while the task later reacquires that same agent lock to build and run the turn. Another client attached to the same session can submit a message with a different skill during that gap. In a deterministic two-request run, the first request selected skill-alpha, the second selected skill-beta, and both provider turns used beta. The first client can therefore receive a response generated with a different skill than it selected. Keep the selected skill request-scoped, or capture and apply it atomically inside the spawned turn before prompt construction.

Artifacts

Focused active-skill interleaving harness

  • Review-authored Rust test source that holds the shared mutex, queues alpha then beta requests, releases it, and records the skills observed by both real provider turns.

Focused interleaving execution output

  • Captured output from the executed Cargo test showing the observed values `[(false, true), (false, true)]` and exit code 0, confirming both turns used beta.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/jcode-app-core/src/server/client_lifecycle.rs
Line: 2876-2879

Comment:
**Shared active skill is overwritten between queued turns**

`active_skill` is written to the session-wide `Agent` before the message task is spawned, while the task later reacquires that same agent lock to build and run the turn. Another client attached to the same session can submit a message with a different skill during that gap. In a deterministic two-request run, the first request selected `skill-alpha`, the second selected `skill-beta`, and both provider turns used beta. The first client can therefore receive a response generated with a different skill than it selected. Keep the selected skill request-scoped, or capture and apply it atomically inside the spawned turn before prompt construction.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@shaikahad-tech

Copy link
Copy Markdown

The Windows Cross-Target Check (Linux) job failed with a compile error because this PR adds a new active_skill: Option<String> field to Request::Message (in crates/jcode-protocol/src/wire.rs) but missed one call site:

error[E0063]: missing field `active_skill` in initializer of `jcode_tui::protocol::Request`
  --> src/cli/acp.rs:874:20
   |
874 | .send(&Request::Message {
   |                ^^^^^^^^^^^^^^^^ missing `active_skill`

Fix: add active_skill: None, to the Request::Message initializer at src/cli/acp.rs:878, matching what every other non-skill call site does (client_api.rs, communicate_tests.rs, client_lifecycle_tests.rs, randomized.rs):

--- a/src/cli/acp.rs
+++ b/src/cli/acp.rs
@@ -875,6 +875,7 @@
             .send(&Request::Message {
                 id: prompt_id,
                 content: text,
                 images,
                 system_reminder: None,
+                active_skill: None,
                 no_reply: false,
             })

The ACP (Agent Client Protocol) path has no skill concept, so None is the correct value.

I have a fix branch on my fork (shaikahad-tech/jcode, branch fix/acp-missing-active-skill-field) but couldn't open a PR due to the repo's collaborators_only PR policy. The branch is based on this PR's head commit (fefae76) with just this one-line addition.

@1jehuang
1jehuang merged commit eec0362 into master Aug 12, 2026
6 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants