From 8e3b180d49951c3e53140710b2baad09791cc999 Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Wed, 2 Sep 2026 15:35:55 +0000 Subject: [PATCH] Preserve retained answers across steer rollbacks (#42298) ## Why A steer creates a new instruction boundary without creating a new turn ID. Retained `request_user_input` answers that share that turn ID must not all be discarded when only the later steer is rolled back. ## What changed - Associate retained answers with their originating function call and instruction boundary. - Keep answers whose source calls survive rollback, including through compaction, resume, and rollout migration. - Remove answers whose source calls belong to the rolled-back instruction. ## Testing - Cover live rollback and resume with answers before and after a steer. - Cover rollout migration when delayed answers share a turn ID but originate from different calls. GitOrigin-RevId: 225e7eb7b6a3ae5a5e7fa6fcb80cb7d259db90d0 --- .../app-server-exports-stable.json.zst | Bin 146321 -> 146322 bytes codex-rs/core/src/context_manager/history.rs | 15 +- .../tests/suite/guardian_retained_context.rs | 141 ++++++++++++++++++ codex-rs/history/src/retained_context.rs | 9 +- .../history/src/retained_context_tests.rs | 2 +- .../local/rollout_migration/rollback_plan.rs | 45 ++++-- .../src/local/rollout_migration_tests.rs | 73 +++++++++ 7 files changed, 262 insertions(+), 23 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index f9ad24af2765e6299b0ed2315f1882ae173fc173..8779f99699f0e0522f8d32c5827ecbdfa1f08075 100644 GIT binary patch delta 8238 zcmV+}Akp8E_Xv{r2!MnEv;uD!1oX84wwH4l0{=hXIEf54Zq83L36VV6c4 z0!V+S=F~~ilcG;!YLgGEC^0>(c5>lkbQRE&fhm%}3e?nL3osGbx6e-88#$miv)< zlXJdDv1=WIjncOC{C`k%VxaQ;^TVkPr>os*K*q;`@wu7S-YQ@gw)<_c=O*W`!a4(R z3A7e1Rm6r#i@h&ReNF_xs7I+JDr@IcnP+@(T30eCRtF>8Z3gSzS(ft9US~N%3mQ<5N61=>ol>J?H@^+*OMt zdeiZG+5P;CJ&vwkJJ&HnRXl7t)L4H%cv65K`;@rcX`<8RW)WJJtZR{2i{la;fg`YX zqe+M#XQA_wxE6+Dv+KC6x%RBqv=~AE6AA-)J0ezfV9k*G9?cHU-44B`Y`)NIKxRicBp#B7W9grYLfpwRiLq(sPp@)C4u&OXJ z)@}V8p$IaD&jfL}Fyrv#cQC?BPN9mz z1Q`~8kAF%2XvxfSb_6RrE#LQpd&DPviH3OJMu_Gyi;T48br^w^w(Ea1cC2g%S2<_& z&v5e#bJ#coaqct5ikGDu8b~)0#{iXn-F~Yh@Go5Yq6;mIclr<{A7n*sSrl)Y03bF8MrPqfI-p#92L84F=F}Ym4QSN5c>X+px$*^h>|5$ zI~)l~%GxgX)-L-lxRf`L%z`P^2Szn{i}^_>gA$=TUWTku3EqFZ*_GL5od)aTUxvWi zhCflmHS`}MD_%W*6UD~a<=`Gvi{>W>QjMV{`57k&NyF+{@O^Sc6zSrJoxmUQ4s{4V)W%bj(k0riLo zqU9n9H`FQV5;}iOT}=ZR?4jPDvohz~G6yhmRv9)F84W{+R0+_pPBS+sFmVOQxO4@u z1H#e?EIhx|2!=FCy6_&YiM|4u=TjwqT@zpOb`ipW^SC{7n{xUWvdZ?+wDN97;zv5R z{@c*15=i>OF$*4v<*0Kl#gpu^+ovqom4m8EXF_NCqm~U=MWW^;`$(&}23m&N>=pAA4g}0+ocKO)>~* zN+w8>ol&|nUNxu&RAhi`4k;3NnV#(r=8gm3Y0i7yUmctdwD)?@&0W!hH1M={23E6Y zVa$}gf_~bV(^~QoTn}2PCPc1K^fR@1rRokMW8i-UPf2wij_Xe$f*=5|s%&K6fygvP zM1yT$E-U0;Cz3xFr}WTp@bjm&%J;CIkG}%(@eHIrUhqT6VF5gsk(i1csr6$2?F?G2 zHYm4|Vz4Nk$ocMI_@E0}pt>gy$!qa=r@fx?Zd}$E-1yM5<*o}+j*4#StXY9qO4seS&p%#{_3I(8)IWSnV>Drj z$gAryFzPA_-6M-zPPhzi`7o#Km1FK%to$sgh7ms;p`(imPaK&#AQK_KPKEUfQ#n87 zkUc*fGr@;|rHzqmaX=V{uvKx&z#|eNq27NFM9Lu&?S8sHH}Y8`V?TO8KME>kO$?Hk zC)Pu+B`gR4wc*bhi3R2}nDV_0qAQ9^d2CVeh>1z&ods{?Oomb>5w7zJueWE;q1iez zY-y8nt>Qzv3(lrMuM@JeC(p?s%y>v&aWX}7E57{@tQI{A)-!zowLATJlC$IOJ)(cW z`5#_c?j&mN#<*1~-GZ!6+sPavQp1+r+6p;sbB5{V=hETj(H}GqCfa^FAOcHvm4#?oZp9=357_( z|JEZZ8m8by=_ka!{(I@?Hu`@)31uRi-}=#nTBj{@L3z)D)A>YlIL3Z5(*oiL&Ye-biq zls1;e#o2U=i8&xaSVGx7%E5*y%>-|6A?cz1o>QCxHY1%CUwk_0HR}DYU(c#pQ=;-JYC_JI)&%{FCct zEoRZ=meVpiXmc@$ccmX?fl-;jH23OIhVBEetN~rzat}fne;Kxg$pFykX7m(>JgH1N zX{wAT0VtJH|Asq2Yngu~x(pq~X`3dt+Avfq5+gSPiSOm{`n=vs^ROc7)uni>O|J** zci($W6$(o61wg^h4SnuiIu4A3<&su}Fr=9k(TIhZ_rzC{@T6R#6aC>{lp6j13m ztq`a5yX(r+M)J!PDW%_~QxeoMM$oXk-(b*j6)hYvRmkMzgT#NFHk;%G#dyzSh&#Mr zv~3GHyN^RTNBLUAbrY`VAE*1F=dpRwtsLv*;E>fp0iR2VpAN!&nWL5@GF;CYAzpd6 zm|YIvRm=;B^kWp(d;3MJNK&We_V$~i6hSqwdi#w)n%;hW_PJIr_dNT&{fc5%ZUA4p zO)_F-@1^~=`YV6e`aNa)KC_Il6@=>;GRl#(HJ%VT4^2h#lYuT0QL2*<7{kew1d8E& z$hY~6r*Y;>MKOv(_ESG<<7 z<8Jr3c0d_Xa?04O+8pP52f!W8lU2DXRoI=7EIEqOXIXzQUWsH%^BvX4bRmr(6Ast- zl$qRhmMk$S-L&R0E!7=GV5R5H9}#`v=bqb9498xfN_7=lz=k8n@r}(5Rg6C$`gqSV zdJ1(zit}nrpAzR#`TeKe(XS{;`%FiR*pz)O6Clrr(XL8*X$L-yXT}pYcjBMs*BGx& zGCYuwFPMK&3B@ue&V{7Vg)L8GBev3rCL>@~FwAhz%KnqSl6-^i8<&2GAeH@9FGEM~WA-BR2*;$Ow))@<->r+;U_3d&H%d)r_r#?^EEDme z7Towa3F;5y1k%g2my=qnTW$U;H}Pop&Bx(l8{U7p6(AQWC9EPYw39IBw7fJ3DQd$r z#)d?-B9EzWM}Bs7ke54L^Hcf>H5Fswc>dQ<h=^D#7;e%}^5hptL}g{ww%ocP6t8 z^HvQSieiukLK(1%DG6aNL{%}8Y%w&Y%cx*g`hJs?w#ZcpI;My^ID_xWu9pXPNZZFy zoS1**J5lj`AL_Sl;%wdhE4g}eBiInGbj!Di!>-~D19C`;I)0SgtKr7In#PPtspe?k zRbwg#c_KnmfeDojAhV5guk2ykPp~w(Fj#&|PJ}hOI7?V==dV~2O(MyyOm1kymT+vA z@r#HhyDxu_iJBfhkBHE3F9T!PjWr)Nfzp3ladJj3fzPB5B%W)v(tGkeq0Qfyl2VjU zkRdfkkODm%@G^>{k^N(t($2`v7YVSjL&jfx%b}W&?-){+m4KYSRdsKVjBYrGh2>bc zOuEPRt82xysGjY9GuNwvjO%{albD1m7T6hFN_cf~IU4sX!ls1?c zI$a?6?M6M|B@qcAXk=W*8?CJMe9UWI3QYIk7VAEtg+yRm8k0>8GCtgM-|p8Zi?r=| z9hcHqw7g)*IxtM$K{>sq!3Uh9|MY*7_#?xJR$IOpNV7Tv5JFYC+THS(qH$gmw!%i< z-Z5(o5J+34$iS~{-O+dF-^&ECb{%rM&Jl-yuj@^XY`R^(zn4r_@9#a`>D2yqQ3(Py zVN30Ad3Wbcf`gx7RPzvuH-C#z`>QD)sQtyZ_S61?rdCh;tEjru{vw{1)Bb;o7S5*q zU8JOGe?NuEw7*eIv0&QYFU_U>eFRkA(h&@*u)B>o>kw2Se}6V5-Ua6NzRpuRpb7(R z0if5QKNSX5^;4nLb-i9Yfwx^inrb#XOk%vK(Ev+&v^NPsu@Yf877K#y%w3cVK2&*N z0ZeF8H4W`wYWZaf499Mr=T)MD%t7%cV zCO73xR0QtoZws}nzs>NjnAGqU>pLC3?w>F;pi!xpB$VFF;z8`Ix$@6mL-`etZ@g0tH9Uqz%A8G1&k(hTz)@bt2P zU*faLTL5A@>%bfpl(t2sSOz8B>x9XlOaOZ8^{;%(#*19mWzj7(!B{dgmhop5AF3NP zk6(g4A`$c!JobMN-D}&rPfOkiL*GN;7?!%>YTSnq$0QhCG25O?^haVh5Y6%5sC~RK z$?M>2A}e>6bVhc#=_u%d^I`@kji_5R{_i#x1PemQseAN;U*hIvV3!=tyyy{xKwr z@-9i40RmuTx4Q2iAY5H0o{V@NuTF=siQxeCD`E74vVbX2Le|5G)t1=C-4p51@Q1!M zFYRkHXGwo&?%>VD2j{*UvfEZ-(f>%LmXxlslP26#3 zc8%b)zi(FKQdJ13;F!p3D&IM*&^v=G+!aMY;=+FrNhnzV6=G>?=C8kM9v;6HU<8i` zqbpy)>0C=%sO!0fq~qtcH^kgX&!^_6Ba-4UWAO$RB}kjOb>X`%ruYtQRXf)trdyeC zW~!O>fdSn7yv6Nw^9p&41Q&^^MRvd}PfJ;H^^=9QUcHm|eIn7Hy zL#Si_fDf^zZ{L&B^Tsn69d0sBi4xbYiH(pbKiurv04j3QDw8ErQ}>wN)*7T?LBj7GaV4rGJQfT4LjcSDzlFajulD}vzF8DVgf zqCsoKAUsN|QkZ=vAVlgCPji5QHXjNTCxP&nFY>ROte6kJHltO^e}i`}o_CUUZmeHy zi6*omR(#0FEhruHR6k*-w|V*$s~+J=msCRx>kYK!&3F;N5t}J~YVRKB%Gp1?8EHXU zk6Z?-+P)N_QwDUXu{RojDE{X%Dd+uxfL`t;Nl{v-VqSXic$Bv|cBuI$ax5Btv4Qzu zUQ_r8ZD7`zb$}Ok#`F9bkD_xQ2vWCP;R1l4Bdt|mlT23E-ky#$=#{olL&`C z)^ zXScP7hA9wW=AIkd;YXwK!OCL4S{xv=)r!V+he0RYI;#%Kg%~}th1Y_=IdBeVYJ#id zP%qG2B+2L&0&*n}>2uK&zpsb7$W%l1iDq#sSrO1z+6m7QfC{GPbmJiC7Q`?B9Di)P z@|V0a0vUg&gon5+om6L4K6vZwx(?apZjI%r$$L=M|(b(|Fnd~5Tlk(J1X2fTiEMl)Cshbql1p}*M| zhXG&m4_L_#JTwmL0x8b&kDp5R8N0E}V{b$Ed;bN#XsyX*)4o@1bQmH#8}xU#H866e zjRKViBwqxQn}6+yPC^)&F0{u6!K8>x(w4wMqGSOcCU@w{+_$=nH#V1*Gy)_Of65`+ zt*be>Kmt`4>0%+A!YhoI%rpWfe~mD9hI(fP+Nolc2s&65*FIgvQdx~+${dcJwF5hb zu(7k>L5PJU{rJ0dXO(SYA*TSa_VU}}jdoy9M&BtSi5X{j3&hpeGJ?bY^Y*t$))T-c z9VH9Y6iNo&G2}%xxbfFP{iF$$FmFst_&>6_Pnx7^l1W89iRhPAj*{G?e}V2S#Q#o_WUt2Rchm@QO82S|(yeC<%1Ho+%SyPKKHRIcF(z;gg0PZrfUnu5 z9zR$GqV$#rn+`H17QKU&e~s4%d(5Og0>C>mCyz%Kww#9?fK(64A5aWFJA*an`4Qsf zpvq{Rb(F|KmW_0dv597_15D&Pr6#g)0z*>Qiy;GLVbRfOi`s2p9#xEkO-`YMO#?H~ z^-F*5A1UN9Ul9p)3aH|MXcdB#FSYNd(Um1qxCi2LBp+){#^Z|te?;|m{nmY1*`I29_CMLfJZZWR#c|`PNmHX} zQJv)hz$GCXr0hqr>YdqJ!K%G8r+lg%eV7jKZO8P#wIV#Te^@8#M))v6vZ=v-1a@M4IISGsSTH8{iQ2(y+O2_O0}ZiUQ;o(2F{F z93!%K77qiyf6E1TNPFP+&Z_PYYn$`~_{e>+15b|g5)9BKqcHVstAr`kWM2(5cpCXVq zUayG&%^KS)IDfJ799XWbRgx2y}pHXnr6<3;#Wnf6>pinl-uaKCD( z)vi~(wt(q}>SJRK?+vE_N@0YZ0ssRsPyaB)wq}27?2HdIW4^ z^lknsE40YVSC=b{s-(lIa< zGqkIUzh4G*(518T=oxfrRx%f!j(FA^T%9GQI3Uz0FQYzgM2{ zUGJp?w#`b_qes^_Yw)@e6;lqki0IM1ipADxcAx>j9ZsQIsz$wSUuUD zc}Vq@r6d_^$2P*@Vup@(r*s%>m&zr#XJhfX(fYEjXRV#K!Dj4#EO(m#GX*v zD#R6nTt<&52`zsaVuJV$T4nrR0ug#(yp8FN0LN!Mo~p z10Rsy7_}Ql-~%@Y?3l*D+;?DqL{oK(j7(rID24}2OvL<4%??KO%anH40jbV;yfY*% zdpK(}g4U8sXJz?;ZEi@H%Me1P++a}Si;2x_t&yjtV*JcBy36Rx73cjc8x1~)rnLDW zy4GViuwXQS1uW6kBX7)DR1b{{N#>kzV3jx~92hLFi^+v(O~QeTvoy9gBL;)q=zfP(ly17VAX`)wimeQ(r7X2FHHqIRF3v delta 8237 zcmV+|AkyEG_Xv^q2!MnEv;uD!1nji{rk8UV0{=hwjl=kP)Za7D?Ym)=`BK~8k^gnt z+&YG;$4?u2@@Xiyd85+@FF1C{b^PcyiNOatmv#G&E83zmw~eXA_Xn4;83L361(!w| z0!V*8=F~~i6Q56GYSRs?C^0syc9P&@bQRE&f{i9b35uhm%}3e?nL3mWGAV|B-88#$miv)< zvvR&iv1=WIth-kK*qp<@wu5++bUodw)<^B=O*W`!de1w z3A7e1Rm6tLhrKUNVN_G%MnF_xs7I+JBtLIc*eFRC95(#iQbN z`GsGH41?)l>sTl#@(T30Zs8Ug{SzS(Yt9US~Nx5pt<5N61Y5%;TA?N`M+*OMt zdeiY<+5P;CJ&vwkJJ+#3RXl7R)L4Iic2a;I`;@BOX`<8RW)WJJrE8Hv+KA$x%NEPv^YTj6AFWMJ0eeYV9k*G9?kO2-44B&Y`)V4~zS3w~PccalQ^m}<+(p#jgKfr1gf^7-y*EH%jh1yo5%t+h&QwH8w+ ztE}O7@hiAB{htrQw4}iJLtX5W85`iOzG5`+^b)j?7Hmy*v}T$k)%uV>GwtsPHOyR# zV&>@f0zB`Re@t;daM{OCK;wUM&}(h8KqU+ezxwacPv^kLzci9Ga*6e^IM0@Cp6V9U4eIfngxF;SOQ6>RDeg{fp_sHeZcGKF6&VxCw0 zB1onill0JVyqG(adHsJkRtrF-5s@0U-?GcKLXmy}leIF|@|vcVjjCgV)=MLr2`yIc z_+rO1Iclr<9At-_rw~TxY0CM88MrPqfI-p#95uRZF=F}Ym1RT`Q272Kpxy;ph>|5$ z0~`rS%GxLQ)-L-lxP~{7On)iW>qRwr%lAnqOA?_wUWTku3EqFZ*_GL5od)FMUxq-} zhCflm{qr9pD_%W*6UC0%<=_xhizX%qQjMV{`52^1A|ySrJoVmUQ4sd@K1-%bh)>0riLo zq6H!eH=HTy5;}iOol654?4jPD(=q2;F9$GcRv9)F87)GG)CJJ5-ZD2RDRCvoxO4@u z1H$qMEIhx|2!-l%ge#aryM8>TGxl!K~DXFF&5Ba;nS#h&IRO859g!sn1l z-UH>z42yqiP1zQgrCCBtQLhMlsre%9o5WIO>;`$(&}23m&N}{NAA2iR0+oHDO)>~D zN+w8lol&|nUNxZxRAhi`4rvg0nV#kk=70m=smy!bUk{uPeD`|Lzg^LzGw`%`23GTB zVa(LJf_~bV(^_&4Tn}2PCPc1K^fR@1iRuorV&H!T7fE#<9_vp9f*{DQs!(L#fmk#} zL=$abt|#PQAd)}cru5J_@bjmq%J;C4kG}%(;|!!dUT{FjVF5gsk(de@sr6$2-wayq zG$^-`qOK^N$oc1B_#g;bpt?s6$!pbkr-h#K{#(`;&iK&x?fg8Z#w7SPLXq?rcpnJ} z_!)m?X~K~!9!iE`7m`)w#;_N><*w6EjtXn(tXXqc%G2$(Uq4<}_3NRl)ISVMV>DBV z$aCv4Fykr;9U_bCO*jc|`7o#Km80xgJp3%Fh7ms%p`%j@PZgOuAPgbjO@$Q-Q#n87 z&^tdIGrEU>HI0$0Z$Mau@KbThq$3g`Y2JS)q#Tl$e@%aG)DlI;e(->P5LC(v7$h&x zuZLbsUJw9k!=Ez}3(RLQ<$D=KR}`1>*rMPO6O+t43*N|?45dsWT;~;DZ_k`Vvvp+H z(kA6v#fNkkoK1mVCuC($o|8eC@sPgaWQyiieET6-EqWBJXZiqYclz@rXUE%nM1g{3<4AaZcrNhgkKaBYG^Es?!)dbXD!MJe|L z^+)=l3Fkhy-K>5z$uBMF{%RTkL z(0zvu=D%P}wEc8I1eWY73(>ONib((@fU*tfHvZR%GjN38qwP67or{8)zw3Y27LyI~ z6{Q`ZzTu5P^(70L0fGW?Kr@s>oX(XV{kE)8bPqikH?NWdQg~XPw*wf>O!Y%S2a z69tBvfe5QT?r^C12;N>r3{YdJIJn4~oqJWTN}+XPumK08=srK4#_zzDENVE>{S{3A ztw&NcOu>uNPl$W{_tMX8^nHI4%0xE5^`i;Nu}6H@`piR`o=1EBPJLhVA@|B&#B2A_ z!DHm8az`*(m7TCJBh99_Zb8?IZ81z~rD>fX1;BTJm9$9JJzuvKJXtn6VMGu9BxK+y zZ7hw8v*{KSb3lTygtB{-gAG%f3Etj9(nI|{r(h|L7c>}BMw|x~og#n3b;QStp4V?& zv5>VGjCm1iN`&zKB-k!`279)vLdGHeTz0ie;%=qU_&Qkitp zR2feKP%5SV4R?UnGE0AS89Iv7Hcf1`VW?CjMs5TW-^=6mdA*h9VMWxdOYvBnUJusq zzW16c6qMo%fP$SH`rNy892f`7C9McyNHZ&<5eqTziLWH#N!P&Dy{-pPJPIT!pwe$z zAx`Oc*OjM@O z+ZJ+mABS>|^0kKRCS1=yPWMC4WAmb0Io8R+A*+J|K9>+b9fbKZM=eQYxSlgYyz*`_ zyBxl&m=_S~$0)4#_KQ}Lq)yB2?Ked!f@)s%_8WmTz5V*^bFEzNdG>kx6~(OF0KRma zWW>teOZ#p0SFV5cd&>5GW*K2C2-h)Wlp|?tJRx!(nu_En16?GdR3{%WhLb4?6vO$D zZ}S&V?Or^P>|CEjn-;sbA8|zu9~Q2h2gbV(+57KlV~D$qB5ruu=sbUzlp!Locr9bc z-R^PifHI=wl(AX0InMVEfIFHet8!DSusb1HaulV{vR;3@63LY2JF1WALK;CP9Io*x zGr8+5Sz=JSY0YC=sym9nO3#}=BKpA3J-4G6j=e&a>MFE=4M&XQ8=D)d7=J$W@t$M! z6zYZ)=hc`#CC;Jp`%k;0Us00wnT{5*Df?O`K%NhyU6u6G4tyHVj3;dF#6QiiF3InBkt4{U?1T`2_z4gj!8#t^}!jwTNkszcyfYol$y-$iA#f6CgMXa zxbblk)E~wPq?c(gC$(0$+Wc2;;?eAzkHf__ymNmmKrT{BSVdfDCt=QMd1(+*)P`q_ z4T)++9#h|r{Osx=FL$`+r}PtQD#pU`{I8KHi7vZUg6-d%p(OM{X@MyHSMbU1OlBG8 ztr|2G#UKrYGGG-`62e@Fs$wMBVrWX2QNgP8{U#}Gk*gAPOc8Z(2H%riFAwaHwvVAW zG0T5UBw#)6*s2y1k4mayE;U$G>bM3P&X+|Y(C;n*zW z7ZFQ#U;Z8wH9dSD5ux8+2F9=(Yd&fMrMG|LfRn1-Ea;I%du|9 zv*mXkPo+zFO4lgGCjUu`N5i(Pzo3HsYAW(whXvX&`teR@;-pInRBbT1g2HFzgK>W; zRZksBvjdj&T(8v=_P;hM}`rtwtO*=W_1Q2gsO72yX7xM%*C(G(qe%D}T;u|MVx@O0;P0eTW>{RVEt^|$hB8rr|q^5dW)JQ0fTppn@}o~!unTVqDk z3?#ffbl>@La;;c-e`m~CxS%yZikFNF%dTLl z&$1xGiU>-ry11yr^$Ml}{;LjIO#M~P!La@k5-O~}ox=q@bw2?g2%)uC$m<({NFLM^ zP5@o2&#(H6C;^%oWLAGx#pwk*GAD%d~M;e|b@Xdu#Q#swHammw4`Jy4ByXQHTS%`ny9ouKupLbYb&W)1q)q zZpxde2;9}*7HU_2o8etCso^WucRGB_!PZ0)D;y#tRa`TujzE9aaOU8j+S1=y$gzYj z{f)&IYESvn-{HcCElhvI1i+j(roZFfqv>|_&Cmt~XSMIYibyRo^o&xa8Rj|Q>16@G z#AlPY0K|0GfjKHDZHr2=3`)4y36npW0QA=DU-_1e7rCs>q!+*S2+^mb?*$zK6mwEOo=xxDO$YNie!%wmp~VkHl^un&ZDw`*>rL z*TL6BR_-k6jO>Je4_47J$+^4dtH46K}@y2hTXE!gQpkMUO67Cap=eN zIwNIQG84@p4<8F(0hvD%Rm8uuEh7NSP3wTL$_AVsW*L8l&3DpMHRcbo>!d`eCLnzj z#Ux@72NM4?fWD_V@MX| zU6L{b1i;8{b>BZgxVlU{8Sy+`oep6W!vX47!srEM0aKuatcMY+EwPQeC(@ze4}ED~ z+Sg{zlH7knGHCV1EKZ`rWizujI-PokHgtTo5rbJcw|zqsnatatY9U~3BAv!TZ;pXw zks~B0bWvs;UY2ikPhm0Q9j=Svf3A7bdwwC9|KBwq1p-U~G05NruY%Ojws{DexZ}?3 z8o_CQ->k-^st{1YF_G6)zH?ZicLrCuD~f=`g&}{EP_X_h#M0KxUw_p+Jbo*{2p$hc zSH6JLxt6q0*K-R=$IokTh`EuTPt8w9B*kII;teWFkT!Gc!gpOv@g3NzcCJZGw=&_( zR5Rym|A5;7Dbf`*XDsM*Jk37{PNX10V`~{D%r{nkIJQMK{SS)(_O^JJh!8 zCKZ1G^lkHI#z7+8rNs)9A5gEV+!k1usz_HIHhgmsbJj?+FaBYo+@KF)hM2XraRDHd z`HMLtp&lREJ%UbSbd2-uWuMRwhch~`V8(c;>VIg$XH?jv+(^=Cz-#vCF{HEu^NXzq zNaPMrGjTYO^YWYqz5T_s!yC@s6^(WRtY?30Y$cyj`7rzW?m*E35m3TMAo=!^A#aF> zl4ujWDP9!V6OoI1-Nmmx9*o&B|l!z|AUhntw`$ zP{;lOA7V}4z9*&Ujb|`A++>;(C9Yi)8zE7CxY@Me|x zxs85|KoFNkFajwVW;a0HeF23Q-^rkiM!P!>WP{Ftp?N%aCzpsY0w{kEg5cE|VQ___ zL2JYyJW8uln0+Q7L@E(abAW+X9tsmDf$*0v@~@k$m=C@-qt(TKgLf`wcan8(tY2)2 zCbS_|e8|WxC;{_SKVhe-dHNKq9^pxsTka)EQCg>BUV89&l(#r`sQD*yEE;~XfjM7Z zQ#c20VAhy*fR}Q{bNLvLqH`YzQny^;0)U?*tw&#z%u?6h-i$q^Q8!~!F$|`eq8Azt zDehL>#H;h!)Lhl%jn=$iXyTpsJy?>=e2%Ue7Ifxl9)!M``7eL=kCGNO8Y<5D95tnA zV8O2N9s7HzfY+)V?|8S)Bj6cfp70LHiFNN(>IKaM3lCcTGnvgFDkI85VC?1ij`Wx#McE1p1G)0T;kQq|PSE-J9T(2!}r7 z`5m(b;1O4Muzi1!U8QtMW1v51O{2nqB6IFgU-A8ej}i*@-;=oFnNNLqW+?oN({&_g zx0#2ADG*@ho*Ub7N2Bq<%2dBv93ZpRipFM#K_}cgtB%Hn7$LER*Mk2ya1L2&g3IDi zFVI{h$>^p5azzg5lh6~tuZOzGI79V`W^pP@5ztrK3C|IL3a006;~?l3L@fXue{8$D zm%TCq8Gnm}hqz3fRA=Sn@Y&I8QJTX}iywfr^%gm^;NEbZ@7KTTeCn@5=~5qs#x+yq z@w4<5Y>qh_>Y4d8&zJj(8Pj(i=t;EzQ?Hvfc|zmFZjUF0*62yQGt_e#Ea)RWHXTy2 zE}=`Wr~Z`fvUP{93>{Sx3Og|hbOrO~iuY&jn18;_moeO(`b8HQ`i;V6XNZ_(Mv$Rg z%&H6O=hT9yMoDPnr5ZLos$twL-maSc*kuI0lvuM3RS94+>=$n+1SF}_NVf?JtP$Eo z0H73BbCB$$qTq=50rLl29f{7tO-b9|51U#~>smFk>F=KCYGl?rVNdgFL!vkRG09{RDB7BuVEtG}gKznTMHt1p@y5bTP>t?2S$8yJL)>zu6au z0q^k-Sji4GG!E+mDPHoApGx)%yRpn;Z$tNO{{_Bit;yxjzE^B?7zR5V^mn&4Fl(ia z0<{JtUxbdEf9;5FK^U1Xw8sX)q=-z?mWV;3WC0!~cj(I8x3i2lFPD}y0wfc^$RXOT zt2wwp0#z63Vj-NuD{z<1Gy*1n4J~$tdS?dOsbZ7}I#?CgK3&C9Syf`nJdK^T13QM4 zv9sSnh=msY_`7sqm2F}nrvR|N^4sE#c3@9Nrzs*)8E1G4#MRd_g2Vpv_P0pZ6Tl`9 zB@5K}Ne0g`9Q<-`7X;uwYY@w00%TbcX9YDl z?$4=8QfC3g|1yzeug2z^j-&>4l*Scp@Wrwjh_d5%%nX6;5srVb4M1ooQE8MRP)InPz*jhgEi;*5#r^b z>SvsFjmSZ^jC78piDs=sO5{4lC9-e=V^PPa(a|uA+HGGRRg8m8-k^g`1N+bQ zOL6WWN#ikJ5eaqDr{ZL26-tyZwYR6yl_gU61>$lfA8SqK;)?-))bWMfbELi}TmWvy zLvdp2WWUy|dt%5%b-u^AtF^dp=Yl1jfZYB?0N0@%26br5+fCe9C*INv4lr4ENTf^Qj#&2n7JOORXKH%(2id~}X}S=papUMmQ=@26 zo#g?*7a_@S(o!MJas=YIBe5xINm=5o4$MnCoA|SJWSSNZ$_`pE2sil77^AlE& zuqiSp*Eist5&~e^Od21Qbi8OivkjVuMRbK-hB@t7PrAejfu*iVvaXn zuZaN78r!?$jfn+{`Lxm5QDQ29L==T*-4)NNtP8^$AB5K9Mfg)G_FP7aw?GSUziOz} zpjW)Mfa!+{V`B~P4L1PfMQ616vVacVo2+i%s_AcQ5v&bWe%DGQO<@oQg9C(m1Z*qh z!JzYhvtmS7wVP+E=K6`2hi{lNO=Eb6Mr#v~#0L3BE)KtJ7#D0zGJ81+BTsv`EcEIC zHxpqNMi6+=(c4jq0vTcoSVwwTfMzilo#D2W4frtc z$piR1>MkKXa4;`EN)vqDL&jG%|Dr$V)`C)hk(?AbQ~@{m)&)?hf}x^IHthz;v{rrt zFo_B=%gR;js@`9&u8HmyO-Gfz%T^g+p$lQki~$*iv$9_VzW5`!_u3T1B< z#1(>EKaVI0CVxI%e~2u4K7P7-S>Ljr4suxPueQNr&j6{Ve=XZDgH*b~yXtiV zACTS{wHrp@gEI&0D8|5CbYOo(Q+11QOkgf3h6hYc#QaOm4o3CMv~|}3Da(1hQzI># zH*2(h){;tRW%+?^Zb+BQ5JIKgU{K?WiOr0xk*B3%{LD1E%jnA$2mLD>ojr)A^!FgT z)>SvKU>Jb~EYZ~?ugX|d^Nb5g=74Zul{g<97%Z-R$%Sa$!GSlBwEHELDF0=-2NtpF faPvDIi+ae_g2W6^LNBux>qQCG_vn~YUo1ifcO=rP diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index f6b459261c..a5b9e05919 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -1,6 +1,6 @@ //! Parent model history and bounded host-owned context facts. //! Compaction replaces only the model window. Snapshots include retained facts atomically; -//! checkpoint replay and source-turn rollback share their live lifecycle. +//! checkpoint replay and source-call rollback share their live lifecycle. use crate::context::ContextualUserFragment; use crate::context::ModelSwitchInstructions; @@ -514,7 +514,18 @@ impl ContextManager { .iter() .filter_map(|item| item.turn_id()) .collect::>(); - Arc::make_mut(&mut retained_context).remove_turns(&removed_turns); + Arc::make_mut(&mut retained_context).retain_answers(|answer| { + // A steer creates an instruction boundary, not a new turn ID. Replay exposes + // the original calls from rolled-back checkpoints, so prefer the exact source. + if let Some(source_index) = snapshot.iter().rposition(|item| { + item.turn_id() == Some(answer.turn_id.as_str()) + && matches!(&item.item, ResponseItem::FunctionCall { call_id, .. } + if call_id == &answer.call_id) + }) { + return source_index < cut_idx; + } + !removed_turns.contains(&answer.turn_id.as_str()) + }); self.replace_annotated(retained_items); self.retained_context = retained_context; self.review_history = review_history; diff --git a/codex-rs/core/tests/suite/guardian_retained_context.rs b/codex-rs/core/tests/suite/guardian_retained_context.rs index 2ef4f593b3..8f45117e48 100644 --- a/codex-rs/core/tests/suite/guardian_retained_context.rs +++ b/codex-rs/core/tests/suite/guardian_retained_context.rs @@ -175,6 +175,147 @@ async fn compact_and_assert_answers( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn retained_answers_rollback_only_the_steered_instruction() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let test = test_codex() + .with_history_mode(ThreadHistoryMode::Legacy) + .with_config(|config| { + config.experimental_thread_store = ThreadStoreConfig::Local; + for feature in [Feature::TokenBudget, Feature::DefaultModeRequestUserInput] { + config + .features + .enable(feature) + .expect("enable test feature"); + } + }) + .build_with_auto_env(&server) + .await?; + let questions = [ + ("before-steer", "Publish?", "Only privately."), + ("after-steer", "Publish the README?", "Do not publish it."), + ]; + let mut responses = questions + .iter() + .map(|(call_id, question, _)| { + sse(vec![ + ev_function_call( + call_id, + "request_user_input", + &json!({"questions": [{ + "id": "publish", "header": "Publish", "question": question, + "options": [ + {"label": "Yes", "description": "Publish privately."}, + {"label": "No", "description": "Keep local."} + ] + }]}) + .to_string(), + ), + ev_completed(call_id), + ]) + }) + .collect::>(); + responses.push(sse(vec![ev_completed("done")])); + let response_mock = mount_sse_sequence(&server, responses).await; + test.codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "Check whether to publish.".to_owned(), + text_elements: Vec::new(), + }])) + .await?; + let mut answers = Vec::new(); + for (call_id, question, answer) in questions { + let request = wait_for_event_match(&test.codex, |event| match event { + EventMsg::RequestUserInput(request) => Some(request.clone()), + _ => None, + }) + .await; + if answers.is_empty() { + test.codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text { + text: "Also inspect the README.".to_owned(), + text_elements: Vec::new(), + }])) + .await?; + } + test.codex + .submit(Op::UserInputAnswer { + id: request.turn_id.clone(), + response: RequestUserInputResponse { + answers: HashMap::from([( + "publish".to_owned(), + RequestUserInputAnswer { + answers: vec![answer.to_owned()], + }, + )]), + }, + }) + .await?; + answers.push(VerifiedAnswer { + turn_id: request.turn_id, + call_id: call_id.to_owned(), + questions: vec![VerifiedQuestionAnswer { + question: question.to_owned(), + answer: answer.to_owned(), + }], + }); + } + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + assert_eq!(answers[0].turn_id, answers[1].turn_id); + let requests = response_mock.requests(); + assert_eq!(requests.len(), 3); + assert!( + requests[0].has_message_with_input_texts("user", |texts| { + texts == ["Check whether to publish."] + }) + ); + for (index, request) in requests.iter().skip(1).enumerate() { + assert!(request.has_message_with_input_texts("user", |texts| { + texts == ["Also inspect the README."] + })); + for answer in &answers[..=index] { + let (output, _) = request + .function_call_output_content_and_success(&answer.call_id) + .context("answer tool output")?; + let output: serde_json::Value = + serde_json::from_str(&output.context("answer tool output content")?)?; + assert_eq!( + output, + json!({"answers": {"publish": {"answers": [answer.questions[0].answer]}}}) + ); + } + } + test.codex + .append_rollout_items( + &answers + .iter() + .cloned() + .map(|answer| { + RolloutItem::RetainedContext(RetainedContextEvent::VerifiedAnswer(answer)) + }) + .collect::>(), + ) + .await?; + let mut thread = resume(&test, &test.codex).await?; + compact_and_assert_answers(&test, &thread, &answers).await?; + for expected in [&answers[..1], &[]] { + thread.submit(Op::ThreadRollback { num_turns: 1 }).await?; + wait_for_event(&thread, |event| { + matches!(event, EventMsg::ThreadRolledBack(_)) + }) + .await; + compact_and_assert_answers(&test, &thread, expected).await?; + thread = resume(&test, &thread).await?; + compact_and_assert_answers(&test, &thread, expected).await?; + } + thread.shutdown_and_wait().await?; + Ok(()) +} + #[derive(Clone, Copy)] enum LifecycleBoundary { Rollback, diff --git a/codex-rs/history/src/retained_context.rs b/codex-rs/history/src/retained_context.rs index 3214212a0c..97ed607b5d 100644 --- a/codex-rs/history/src/retained_context.rs +++ b/codex-rs/history/src/retained_context.rs @@ -44,7 +44,7 @@ pub enum RetainedContextEvent { } /// Bounded snapshot of retained families, persisted with the parent compaction checkpoint. -/// Facts live until their source turn is rolled back; compaction does not expire them. +/// Facts live until their source instruction is rolled back; compaction does not expire them. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct RetainedContext { verified_answers: VecDeque, @@ -123,10 +123,9 @@ impl RetainedContext { } } - /// Removes facts with explicitly removed source turns, not merely compacted tool calls. - pub fn remove_turns(&mut self, turn_ids: &[&str]) { - self.verified_answers - .retain(|answer| !turn_ids.contains(&answer.turn_id.as_str())); + /// Retains facts whose sources survive a lifecycle change. Compaction alone is not removal. + pub fn retain_answers(&mut self, keep: impl FnMut(&VerifiedAnswer) -> bool) { + self.verified_answers.retain(keep); } } diff --git a/codex-rs/history/src/retained_context_tests.rs b/codex-rs/history/src/retained_context_tests.rs index baa34b748f..5129daae0a 100644 --- a/codex-rs/history/src/retained_context_tests.rs +++ b/codex-rs/history/src/retained_context_tests.rs @@ -35,7 +35,7 @@ fn answer_lifecycle_is_bounded_idempotent_and_checkpointed() { } assert!(!context.is_complete()); assert_eq!(context.verified_answers().count(), MAX_ANSWERS); - context.remove_turns(&["turn-2"]); + context.retain_answers(|answer| answer.turn_id != "turn-2"); assert_eq!(context.verified_answers().count(), 0); assert!(!context.is_complete()); diff --git a/codex-rs/thread-store/src/local/rollout_migration/rollback_plan.rs b/codex-rs/thread-store/src/local/rollout_migration/rollback_plan.rs index 58d1944407..44bd5c6937 100644 --- a/codex-rs/thread-store/src/local/rollout_migration/rollback_plan.rs +++ b/codex-rs/thread-store/src/local/rollout_migration/rollback_plan.rs @@ -5,6 +5,7 @@ //! surviving turn after a newer turn has started. This planner keeps compact per-record ownership //! metadata for SQLite visibility, then combines it with `rollback_replay`'s cold-resume answer //! before the writer makes its second streaming pass. +//! Retained answers follow their source calls, not later steers sharing the same turn ID. use std::collections::HashMap; @@ -86,7 +87,7 @@ pub(super) struct RollbackPlanner { pending_user_response: Option, pending_delivery_boundary: Option, turn_boundaries: HashMap, - retained_fact_sources: Vec<(usize, String)>, + call_boundaries: HashMap<(String, String), Option>, compactions: Vec, model_replay: ModelReplayPlanner, } @@ -103,7 +104,7 @@ impl RollbackPlanner { pending_user_response: None, pending_delivery_boundary: None, turn_boundaries: HashMap::new(), - retained_fact_sources: Vec::new(), + call_boundaries: HashMap::new(), compactions: Vec::new(), model_replay: ModelReplayPlanner::new(), } @@ -154,6 +155,14 @@ impl RollbackPlanner { // no later turn to attach it to. self.pending_context_records.push(index); } + if let ResponseItem::FunctionCall { call_id, .. } = &response.item + && let Some(turn_id) = response.turn_id().or(self.active_turn_id.as_deref()) + { + self.call_boundaries.insert( + (turn_id.to_owned(), call_id.clone()), + self.record_boundaries[index], + ); + } } RolloutItem::EventMsg(EventMsg::ThreadRolledBack(rollback)) => { self.apply_rollback(rollback.num_turns)?; @@ -228,9 +237,16 @@ impl RollbackPlanner { RolloutItem::RetainedContext(codex_rollout::RetainedContextEvent::VerifiedAnswer( answer, )) => { - self.assign_targeted_record(index, Some(&answer.turn_id)); - self.retained_fact_sources - .push((index, answer.turn_id.clone())); + let source = (answer.turn_id.clone(), answer.call_id.clone()); + // A late answer still belongs to its call's instruction boundary, even + // if the same running turn has since received another user steer. + if let Some(boundary) = self.call_boundaries.get(&source) { + self.record_boundaries[index] = *boundary; + } else { + self.assign_targeted_record(index, Some(&answer.turn_id)); + } + self.call_boundaries + .insert(source, self.record_boundaries[index]); } RolloutItem::SecurityRiskScore(_) => self.record_boundaries[index] = None, } @@ -245,7 +261,7 @@ impl RollbackPlanner { compactions, model_replay, turn_boundaries, - retained_fact_sources, + call_boundaries, .. } = self; let replay_anchor = model_replay.finish().empty_replacement_history_compaction; @@ -253,14 +269,6 @@ impl RollbackPlanner { .iter() .filter(|(_, boundary)| !boundary_alive[**boundary]) .map(|(turn_id, _)| turn_id.as_str()) - .chain( - retained_fact_sources - .iter() - .filter(|(index, _)| { - record_boundaries[*index].is_some_and(|boundary| !boundary_alive[boundary]) - }) - .map(|(_, turn_id)| turn_id.as_str()), - ) .collect::>(); let compacted_items = compactions .into_iter() @@ -268,7 +276,14 @@ impl RollbackPlanner { // Migration removes rollback markers. Checkpoints must therefore carry // the same surviving facts as their standalone source events. if let Some(context) = &mut frame.item.retained_context { - context.remove_turns(&removed_turns); + context.retain_answers(|answer| { + call_boundaries + .get(&(answer.turn_id.clone(), answer.call_id.clone())) + .map_or_else( + || !removed_turns.contains(&answer.turn_id.as_str()), + |boundary| boundary.is_none_or(|boundary| boundary_alive[boundary]), + ) + }); } if Some(frame.record_index) == replay_anchor { frame.item.replacement_history = Some(Vec::new()); diff --git a/codex-rs/thread-store/src/local/rollout_migration_tests.rs b/codex-rs/thread-store/src/local/rollout_migration_tests.rs index 3ffc7aa5ef..7ed8810601 100644 --- a/codex-rs/thread-store/src/local/rollout_migration_tests.rs +++ b/codex-rs/thread-store/src/local/rollout_migration_tests.rs @@ -1160,6 +1160,79 @@ async fn migration_rolls_back_pre_compaction_turns_from_sqlite_history() { ); } +#[tokio::test] +async fn migration_preserves_answers_before_a_rolled_back_steer() { + let home = TempDir::new().expect("create Codex home"); + let thread_id = ThreadId::new(); + let mut items = vec![started("shared-turn")]; + let mut answers = Vec::new(); + let RolloutItem::Compacted(mut checkpoint) = compacted(vec![ + input_response_message("user", "before-steer"), + input_response_message("user", "after-steer"), + ]) else { + unreachable!() + }; + checkpoint.retained_context = Some(Default::default()); + for call_id in ["before-steer", "after-steer"] { + items.push(user_message(call_id)); + let mut call: ResponseItem = serde_json::from_value(json!({ + "type": "function_call", "call_id": call_id, + "name": "request_user_input", "arguments": "{}" + })) + .expect("request_user_input call"); + call.set_turn_id_if_missing("shared-turn"); + items.push(rollout_response_item(call)); + let answer: codex_rollout::RetainedContextEvent = serde_json::from_value(json!({ + "type": "verified_answer", "turn_id": "shared-turn", "call_id": call_id, + "questions": [{"question": "Publish?", "answer": "Only privately."}] + })) + .expect("verified answer"); + checkpoint + .retained_context + .as_mut() + .expect("retained context") + .record(&answer); + answers.push(answer); + } + // Delayed delivery must use the original call boundary, not the turn's latest steer. + items.extend(answers.iter().cloned().map(RolloutItem::RetainedContext)); + items.push(completed("shared-turn")); + items.push(RolloutItem::Compacted(checkpoint)); + items.push(RolloutItem::EventMsg(EventMsg::ThreadRolledBack( + ThreadRolledBackEvent { num_turns: 1 }, + ))); + let path = write_rollout(home.path(), thread_id, SessionSource::Cli, items); + let store = indexed_store(home.path()).await; + store + .migrate_rollouts(apply_options()) + .await + .expect("migrate rollback of a steer"); + let migrated = read_rollout(&path); + let events = migrated + .iter() + .filter_map(|line| match &line.item { + RolloutItem::RetainedContext(event) => Some(event.clone()), + _ => None, + }) + .collect::>(); + assert_eq!(events, answers[..1]); + let checkpoint = migrated + .iter() + .find_map(|line| match &line.item { + RolloutItem::Compacted(checkpoint) => checkpoint.retained_context.as_ref(), + _ => None, + }) + .expect("retained checkpoint"); + assert_eq!( + checkpoint + .verified_answers() + .cloned() + .map(codex_rollout::RetainedContextEvent::VerifiedAnswer) + .collect::>(), + answers[..1] + ); +} + #[tokio::test] async fn migration_preserves_reverse_replay_anchor_after_pre_compaction_rollback() { let home = TempDir::new().expect("create Codex home");