fix(context): restore skills after compaction - #9043
Conversation
|
👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated. Missing sections:
Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle. |
1 similar comment
|
👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated. Missing sections:
Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle. |
Provider-native compaction drops session-start context. Re-arm the existing one-shot skills reinjection after confirmed success so the next turn retains the available skills.
35c7c9b to
df70886
Compare
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsI've independently traced the change: the No findings. [OPUS-REVIEWED] df70886 |
Design Review (Fable 5, fork) — ✅ PASSDesign-level review of Design-Verdict: PASS Correct reuse of the existing one-shot reinjection seam at the three real completion channels; tests pin each path plus the failure negative. Watch
[DESIGN-REVIEWED] df70886 |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of All evidence is gathered. The fix reuses the existing one-shot First-Principles-Verdict: CONCERNS Right seam, right level for the dashboard — but the identical lost-skills cause survives, uncounted and unmentioned, on every messaging surface. What this change shipsIntent: after a compaction completes, the next dashboard turn re-delivers the skills index instead of losing it until restart — a FIX.
Watch
[FIRST-PRINCIPLES-REVIEWED] df70886 |
Problem / Motivation
Provider-native and manual ACP compaction replace the model context but do not arm the existing one-shot skills-context reinjection. The next user turn can therefore no longer discover the skills available to the session.
Why it matters
Compaction is meant to preserve an ongoing session. Losing the skills index after it completes makes tool and workflow discovery depend on a restart rather than the normal next turn.
What changed
Arm the existing
mark_needs_reinjectionflag when a native compaction completes, including immediate Claude completion, deferred Kiro completion, and provider-emitted completion status. The next turn consumes the flag through the established prompt-building path.Pattern harvest
When a provider mutates or replaces its context outside the normal session lifecycle, rearm durable session-start context at the completion event. Reuse the existing one-shot reinjection seam rather than duplicating prompt assembly in each provider path.
Tests
PYTHONPATH=src /home/prems/dev/repos/premshay/KiroCrew/.venv/bin/pytest -q test/test_dashboard_chat.py -n0— 767 passed.What changed (motivation → approach → change)
N/A — covered by the existing
## What changedsection.Manual verification
N/A — focused automated coverage is sufficient.
Related Issues
N/A.
Checklist
Contribution License Agreement
N/A — template placeholder; no CLA wording is supplied.