Skip to content

fix(skippy): avoid duplicate lifecycle token handoff - #1202

Merged
i386 merged 2 commits into
mainfrom
agent/upstream-proposal-state-handoff
Aug 8, 2026
Merged

fix(skippy): avoid duplicate lifecycle token handoff#1202
i386 merged 2 commits into
mainfrom
agent/upstream-proposal-state-handoff

Conversation

@i386

@i386 i386 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Problem

The public #1193 squash (915e6918) introduced a regression where a request can carry pending tokens that were already delivered through the lifecycle commit_generation path. The proposal path then forwards those same tokens a second time. That duplicates the plugin's generated-token prefix and can make a valid proposal session fail closed.

Fix

  • Track the generated-token prefix already committed for each request/session.
  • Before proposal lookup, commit only an uncommitted suffix.
  • Reject rewinds and inconsistent pending-token counts instead of corrupting plugin state.
  • Add regression tests covering lifecycle-delivered tokens, missing state, rewinds, and inconsistent suffixes.

This is intentionally based on public main after #1193 and contains no private Mesh architecture or Cacheline code.

Validation

  • cargo fmt --all -- --check
  • cargo test -p mesh-native-serving-plugin-host --lib (24 passed)
  • cargo clippy -p mesh-native-serving-plugin-host --lib --all-targets -- -D warnings

This draft PR only addresses duplicate proposal handoff state. Benchmark integration and session-header wiring are intentionally out of scope.

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

ActivePlugin::propose now validates tracked generation state before committing pending proposal tokens. It rejects invalid counts, missing state, rewound prefixes, and mismatched suffixes. Tests cover these paths and prevent recommitting lifecycle-delivered tokens.

Changes

Proposal generation-state validation

Layer / File(s) Summary
Generation-state checks and validation tests
crates/mesh-native-serving-plugin-host/src/lib.rs
ActivePlugin::propose uses checked token-count subtraction, requires tracked generation state, rejects rewound prefixes, and commits only matching pending suffixes. Tests cover validation failures and no-recommit behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • Mesh-LLM/mesh-llm#1081: Introduces the linear-proposal execution path that ActivePlugin::propose now validates.
  • Mesh-LLM/mesh-llm#1149: Introduces the native serving plugin proposal path refined by this change.
  • Mesh-LLM/mesh-llm#1180: Modifies proposal handling and token-buffer state management in the same implementation file.

Suggested labels: experimental

Suggested reviewers: michaelneale, ndizazzo

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing duplicate lifecycle token handoff.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/upstream-proposal-state-handoff

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review.

@i386
i386 force-pushed the agent/upstream-proposal-state-handoff branch from 5679a89 to 9548567 Compare August 8, 2026 18:53
@i386
i386 marked this pull request as ready for review August 8, 2026 19:02
@github-actions
github-actions Bot requested a review from ndizazzo August 8, 2026 19:02
@i386 i386 added blocker blocking other PRs and removed experimental labels Aug 8, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
crates/mesh-native-serving-plugin-host/src/lib.rs (1)

920-1044: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for committed counts below prompt counts.

Lines 472-475 reject this input before generation-state lookup. Add a proposal test with committed_token_count < prompt_token_count and assert that the error contains "precedes prompt token count".

Proposed test
+    #[test]
+    fn proposal_rejects_committed_count_before_prompt_count() {
+        let (active, _) = test_support::fake_active_with_events(Duration::ZERO);
+        let error = active
+            .propose(
+                LinearProposalQuery::new(
+                    1,
+                    2,
+                    2,
+                    1,
+                    1,
+                    8,
+                    Instant::now() + Duration::from_millis(100),
+                )
+                .with_pending_token_ids(vec![4].into_boxed_slice()),
+            )
+            .unwrap_err();
+        assert!(error.to_string().contains("precedes prompt token count"));
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/mesh-native-serving-plugin-host/src/lib.rs` around lines 920 - 1044,
Add a test alongside the existing proposal validation tests that initializes a
generation with a prompt token count higher than the query’s
committed_token_count, calls Active::propose, and asserts the returned error
contains "precedes prompt token count". Ensure the case exercises the early
count validation before generation-state lookup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/mesh-native-serving-plugin-host/src/lib.rs`:
- Around line 920-1044: Add a test alongside the existing proposal validation
tests that initializes a generation with a prompt token count higher than the
query’s committed_token_count, calls Active::propose, and asserts the returned
error contains "precedes prompt token count". Ensure the case exercises the
early count validation before generation-state lookup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 35691800-bf97-4497-bf18-f8d273077398

📥 Commits

Reviewing files that changed from the base of the PR and between 915e691 and 9548567.

📒 Files selected for processing (1)
  • crates/mesh-native-serving-plugin-host/src/lib.rs

@i386
i386 marked this pull request as draft August 8, 2026 19:05
@i386
i386 marked this pull request as ready for review August 8, 2026 21:11
@github-actions
github-actions Bot requested a review from michaelneale August 8, 2026 21:11
@i386
i386 merged commit 9f55e63 into main Aug 8, 2026
45 checks passed
@i386
i386 deleted the agent/upstream-proposal-state-handoff branch August 8, 2026 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocker blocking other PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants