From c141c3132319ed667011eae00ec6ec7ca24a9321 Mon Sep 17 00:00:00 2001 From: ioanbudea-OAI Date: Wed, 4 Nov 2026 00:14:45 +0000 Subject: [PATCH] Preserve requested approval reviewers across policy overrides (#59272) ## Why A model requirement can force `auto_review` even when the requested reviewer is `user`. Persisting or sending that effective value back as an explicit choice loses the original selection, so removing the requirement cannot restore it on resume. ## What changed - Retain `requested_approvals_reviewer` separately from the effective reviewer in configuration and thread settings, with a fallback for older histories. - Restore the requested reviewer on resume while applying current model requirements and reviewer allowlists. - Let ordinary TUI turns, continuations, retries, and forks inherit the server's reviewer instead of echoing its effective value as an explicit request. - Preserve a worker's requested reviewer when reloading it rather than inheriting the sender's selection. ## Testing Extend resume coverage for explicit `auto_review`, removal of a model requirement, and a tightened reviewer allowlist. Update TUI assertions for omitted reviewer overrides and verify worker reload preserves its reviewer. GitOrigin-RevId: d8c9c3a7b89e55e63f0e8ae210d6a87cb5547933 --- .../app-server-exports-stable.json.zst | Bin 166936 -> 166985 bytes .../persisted_resume_settings.rs | 21 +++++++++---- .../persisted_resume_settings_tests.rs | 1 + .../src/request_processors/turn_processor.rs | 1 + .../tests/suite/v2/thread_resume.rs | 28 ++++++++++++++---- codex-rs/core/src/agent/control/spawn.rs | 1 + codex-rs/core/src/agent/control_tests.rs | 11 ++++++- codex-rs/core/src/codex_thread.rs | 4 +++ codex-rs/core/src/config/guardian_policy.rs | 23 ++++++++++++++ codex-rs/core/src/config/mod.rs | 9 ++++++ codex-rs/core/src/session/mod.rs | 21 ++++++++----- codex-rs/core/src/session/session.rs | 18 +++++++++++ codex-rs/protocol/src/protocol.rs | 5 ++++ codex-rs/state/src/extract.rs | 1 + codex-rs/thread-manager-sample/src/main.rs | 1 + .../thread-store/src/thread_metadata_sync.rs | 1 + codex-rs/tui/src/app/misalignment_policy.rs | 1 - codex-rs/tui/src/app/safety_buffering.rs | 2 -- .../app/tests/misalignment_policy_tests.rs | 2 +- .../src/app/tests/startup_defaults_tests.rs | 25 ++++++++++++---- codex-rs/tui/src/app/user_turn_submission.rs | 12 ++------ codex-rs/tui/src/app_server_session.rs | 6 ++++ 22 files changed, 158 insertions(+), 36 deletions(-) create mode 100644 codex-rs/core/src/config/guardian_policy.rs 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 4f7853dcc40dac6999bcfbae5452a6065eb48436..aaed74c052f89e39b0a76773d2aefebf5428466f 100644 GIT binary patch delta 7227 zcmV-B9K_?8m-mLOjuAamMd_Dg&}&fm!cSFLbgkCuxP@~xF7#J!oBEcB>~}+uUQ2Lfx&??f2ct1G!Y>B zioya7qznd1j60)`K>9TiS&)dRSjyF5L?qoZE!SSbs+dSLGz2oyTIutZ11*J;{+fiu z+p0;(^`ia%${0n-Sb3w&+m{_t@cCcP240AyOxeGE7I7T;a6+oN#;Hc18phhs(r8hE z4b`z7Iyy{r$dTC?%Yfg6e+d7+lT&>_@cu}yWghk9ti_#jQ=vESSswy|l|IuutQTG{ zfZG$3O23_7@5h=v?3M^e(%{N0&8faC8*62}^C-q)>(Vimw(6WMB5~mj>F>W~@YGk2 zVwkd978X|_!xIwkZAVgU`vQTi``cFVnt7Fj#iH}p*MM?MAP@mA=vX8U4#Hs2m#}dH8-Hc|D{GH6L!#xe zVglQSxkjDfl!x^tc<=O=4=bK+qV^|wB;UD4yy-oXk7%`{uIfkfzwEt}e03jIgh`kP zLrkdLq2hHh-^J~yvLsV++~AajzjJH8DdSqx0gC~`&B0I zj#?EFyvx8_9;afij zBD}U6fxuv_!j?+^Ku8PCiWGATe!R;84faa}@Ln9ghUf>g-C|sAqQt}O>4a73@=m~z zjaxYPqHCj;u0?WD23bh?veI}tMZpJ-G)?8{j>imd0RkG|x%pa`#>KUWU@sz~JbfIo zA;m2jxx7R`iGR}*peiqo1C9X&Dpl$yq5>NH-u7HC`$Yg5YL@$Wyp zxbGm)c1>XBah0f(o8BAk+b{VN8)_}2e5yeb4HT0{<}^kCHg5CWlS|F9)p z37{udzilDOS8il*ZzJ{_qBq7MpljFzaz%>c)zmIR&RQAa$~wlqvmy=-gY5#cBq4IU z>ijT~YJc1WY9I!|*sC(7(5|$;=?~=7I(r1f)Lb5oE6wFG&XBX*AeH8y9KVn^KQmnw z^7PSK@RdT&h3`TxWBQ>3u-`Ap`}-9vXEcCIJ=W~SZx*GOTUlCal?0sSWEpvW!;6Y@ zb{bDv#y$t6{oPE{{@pOaxWyO3lUp)Wtv_x-Nq^xPYD|-}xlxM62WY>ULItcgDv*<3 zpr(Z!4{O;)5eP&UX^)!V_i#lfku2VY>Q;hL)deAtjYIjs>I!>znW(I;OY)9p|4|cf zINK3h0Z5q<@FARepV?FTGQ|awtnq2~n~k9td2E!xw{I#ZL7kHbkuvMVz0kLzrMMEZ zLw|{3ab+U#W3kpQ_#K7Q9i0%BKNF=sD*bf`YvrtLd@v+yys(lW5u>CBX9fvWGfGP6 z;Pq|y8hEf|+cVH)aR($B!uowOtywD|9~ESr{3~G!L#Iul1S3Q$Z;x^W_9=%Jxf7B} znb`S~^WGv)F0WE9i6?w}Y&A9rTGS>9Sby@9yKp~I0j#Vvd-={Rvxp4ZWzej3_-=d9 z|2h%G!~I^=EB?6jY;Jl;;(l>_*o`&?0;uB}{p}^7{o&qA(p(FfgZ6{@D|vP~Y?Wcr zob0@El_3@+WuCpjJDq;0PPByophz|31Dy388`Nv5pIe{L-OHB^n@=-|+dAZjihqAv zX)jrfZOG>x!vSG8(S-WGV6sCC_gKFIArG@Cm{%EzNK7VeoIa&Jf4a%~RLk@WKZ1&py`&*o$;1r}! zd>!)ogrZ?c(RnKjb*hA9tVOInrGGsj2Floc!Oz3ZdRs>Bm-*m7?5UoS`#=FnX|}ar z-=%fU=4NbcTYF69yoe3%P1Y0^i(&KZxDR z!SOD^6s9z|A-tL7{99j7E(PCSrD`}?W7Nbbeq*6}*K+W^VpaZlY=07?I95+5S9Tky zp9-L*^XYY>K%+J8c4cHI-0%);FbL31UDeO+7IxEF*Jg`X@T!8v8KX=*mGxHC+tH_axBd*a4$f%0Xs|Go9KL zX6V-RgMO%^GX%QIOMgK=vJkCe`P7Z5x|Nu1d!(BWqu@7})+?`$=j!MP`y}H@aw7L3 z%z_Oi0>_SP~=c}5TB<~`eGK_NY zaw#i!x(Dt}y!e(pD+LPDwlLf4Rxb1%7WUU3!d7MgXQkANrhnhRyLLm)57I%jG?@q( zV0sm(-Au$9D9d*=M;^~mhoR*x&=(*6o){_7gF>^A#EaP5aEFIFSA#Eg@$V%ut9C(- z=MYeJ&UO{~hs0+e-FeX)G-QS9F$};KtJZVy+)*VXdY`G{^M^{c`HRTVKSGbm?;VMX z4-tg`k{>VG9dG|ME9ZK$!Wai~jJ^>nR<~n9L8okO82hc+T!Dh!0 z$TwTEu1yK}NHJ0ZzC+uy9|gIF67bnEbPkjlwm)E{D}Qc3hl2+kE~>}UV+tiB>IT(m z{a)mZMTj7v7&=6{o}de&=||)D*Nyo7{ex3AlK>T|(L4t-ft4hZagS64WP;2U*sjR^ zKzAb*etmeZE7uIU+DRxl#f%NRDTZiYI^ZU{q5*&|?n&vGE;KE7HWfi>SdvL+D5O?% z-FK!$4}TGlSA?(aSpD%(?!cCmR2VvAv@OV2hVs5|-T%^4!u_%zxi&a&Ph()mykF`c!Vd4*$mIgNM=zmI!IzAqH<`NCfs)wSO)HTF+-3 zmGA1)1&PK|7RKfPfh>-ybJ z>B(2?(HUZ;@_ST)vBGmw<5lw}uiq23SZJoc5!Z@SHNfW_r!h*~Q$EQ?MPgxej(;{S zf&hbmb2LeoW*o$$D&hBjbt5h$FXfBhpm@Fkx2XJ9<`ki8Y*KI__sOOQ7pSj_m9ZJ* z{F@mH<4ofHZi9)moAH!V7Wjb_z=#4Yf9K#)d$PY1h)&}NGd4O9VFR$pYg#0l()bU^ zf5ogwij0})Q3@|~-;0W|B5DkQ@PFfA(|~D@OlRR$)%a#>^!v0t%dVHo|=qWUqm zN>#&XskQARWV>)VeKt6wH1Xoqbir>q0vk(MAZau0jVzA=l zJ^0f;=inHv^fJq<;JhCpNR-yIeqaF9RJuP_kg@1utczisnoQ2Y`eb_(po$gPQ8#7h zJtc`6Z~8DBQ+)1;tEGMNL3b7usd$yDKMSGV| z6qCa|iLuA>xq3C7`>KKwX^d{eIS+7;*-@fMO6$hcma~@635-#s z|MjH#M%AXB%%Egv!jdP@p#J8t@cXsjBEbT@*dVtuRZZMb7k|;SZdjIk$#Dz|F+_yK z8J{v^y8vS_9mbiQVD4eCfB%@D=%_#jX=epcifh4v@e5#BUL5}*#~Aykf$PhlojWVj zY};GYFZC@Nf`J>Z+ZVV&T&=rMqLpKvE5|Vy15`eC5=atJ|>5*W@iv|K!;ITdW*P zuwMG~q;G>pC4SQaSPbU`Xt;eFPKVh~_dcMLMZLTYkQYfSgpy8P2S?=2&W;nOM}`uT z)7ipuAZV_}z|a%U96RDsx*;7acMzloAFOp~fisHOjMa|>=uuXOop5kY7#m-1Umz! z@&OL=ynn!?OK6EXY_{aVJq2kYi53Z<&j%%Y^RM#W=qwX?>*w;+$^(X72;9L?|LksF zpndz1c_BS7oCXhLCZ@;^4|f!=Gb2!3PLFh+dF6vB2;iHE%x}L5^7b?!3eGj!Y)Di% zY5l-jX<9d59R@L_6yYg+5RC4Tg&xZg5@VOD5r5KAtxM%2ED#0t52*+PjNNTD%f977 zmxT54m;@otF|c-!_Ri1Yu8J#QttdSb8q3pU859>67s#K7-bMHqyrX3?S}ah6-21{+bz-+NM3!TmTy<+dmcLKtzfC6_%ty{) zxi{~z?|H<{u(sc7j7CB z>wO5#$mF(P`7!k+23ngzw5NSLOn=UZIbcB@CvP}OUPms>VqmV)`KsDi(FiZgc?TEs`Zt&&2Np;bI(dtrez<(;@_G1w? zQqQoINlopPyCQ*#oqPchz@HpYu4n5gy2q0t-3;vvgZH;%QKDkzM7nLi7R}uzdVI2R zL163#Wed*-VkAyun)?NeF5{TM$>KB*QlGi-h?`Y@9-0+xjTL5KP#4+s|Ipqtj9Sc4 zT`$EY$r5>3}La}abRlFGpIVnm_p>(YX=Co&mhUf3yO|0 zkgAnP-w8-$dEwNrw`d2p#xqk}eL!?a64C^m&TVI|0`xHYIe+`!q@QbOmVVx{ zlIiClVoW~|B;EA${jaB=o0&uXyc{;PKC%DgJz)>$d2c&YC*iQF;`;GmQH}A3ogMl0 zLZu^9kF4Wx7+nKF5E7u*1NO|;`U7~aTsKb)Yu?@)J-KIdd~o27H5>wmaY)aH=( zv+^Kca?1l`Z<%z4bAQ=0L zR-O2k$oTZv7e=SCH*iuG3eHl0MTI<3Cn*|~07m+!afkn2goWu?GojdkYKBg@=e9q} z*Sg2Ubv-`_zKD8lO`+UlxIo74!vK$D({N3}90iWCP{Kq)3xB_oj6&AAP&nF7#EF^lUza|9- zGHQE>F#}h4u;3J_vDfuc-AiO+)y2I<-7y;rQ=+`P20_(a*6r1rII#bU#7#r8&Ku!r zWqW!{#UCQNJ%4yC`X64x!S$|}YhtdNY@C+ypv`ar$}1;OE>@!wtwV{|msxobylKl7 zWIM;p{}v`FfKoT-PM_o{lW3=D|6+c)T1o{P&;Xsv9_dmbQpeNHS!o^)<*KdT_-;b# zS(?=|_EjoHpqL?RyQkwZWR<4S%!wNs!E+U&WK_RWuU87f#{p zT=mp7m|(os|1fiu$*T|(AVhNzFaZJ;A>2(}8vg5VUa_t6w%N*90kdZ)lI}8E`~b z*t5H15FL9f=N!6f2J{hUjhNy^LhWysvhBa+*Nfzpt;g8Kq_2ztHhO#0pmv|>=1%&Y zAW+3<1|%WE-z=9Ue`-MNusHMq?GW^DKY{LeIx+lg%p>=hXO`_EKa4p zWx;An1^)|~fnB_D+qAy{9889fO>vlvuU6h<%VM4gX~2mD{=iM}R-4^>&=4t&J@Qe; z7-#5^vwo?B;b)e^oLj2l9Y`#^|6t7^4ZV3M^u7`AA(zE@pQHI#g$D4OIE{g*z4#j; zzw9zk8ts_dmIl2aO+F>9GE_T%z;?eg?Wci_N48S&?)H%z`EYMXX%K>8CO3&P3o1 z7lwkRgr4%?pJIs5n3 ztsx9Qt;U6!wI1O#Z6aem<+j1cV;y|y3S}2WirfMi-uZa&!9t0x#_--}*A;B)+&$L| zs{!t(!Nbo%W=oeD^frzO-zE9Jl#kJrf>{Q2qC_|aevS}Ere{0qLwb9lB@}E%mY_{tUl~Zp<-CDuTfDD zHi9hP1%b(SPd_*?4j^IvKU&9vqSLAaZohKot$fgtdWMcHtCOt7tB;qg7$r3@6nrvju*t6n(jiLp>^HC$vC zScs5I;2`9Y_8A|DUseWe!tm_El5J8@A-+%mGod@Ol(SN@#kA6YAo!VvLZdDT1!^d6 zC&(_fIswBQ#eJWQ0$_p7nCLm2WMc+RQdSP%>3=RXOpQc;vyr@N%I~0J0nDUvjY*Do z$j}56#hed(@T1%JXU8%3vYHb#Cg6q{9d;6Lq0zh&3_npyLKE&v6Iy^qvV896fK|C& z$O4ueo;p$*Ih<~VLQ0}0n!zO=5vQ|zvulrLdeam+?`tc}E2qdoVvB0J5y)S5DlHx%QHy&IR2YqOOUSXu>Yi#?r6?Z{G zm>K8r+by3M{`(J~Vp!C#+XbtdYnaf^Ew71LAG!~o4Y={*lHm$n__B%W7HFx(?h$FtMD6BBzQ=Aw6l{sfN91% J4mUi3GjBda?VbPt delta 7178 zcmV+l9QEVLmQPW&f3ONHP&-Wo z2&~HW*Te)pR0ad7#+}heKs}R)C`3R|u;uD5B9fL;F(*9&Mlq3OWC&y;#?qYzjl`)F4vGPWlw=X-Q;Pbzn47?9Yo3?-ZEfNzAdVfBg(2`u^RHIi- zW9>IJR#2dX>evn)9VR-ge`HFnWx%h(gMZ)3sX82ZcPJ-i9(Cm~#;tO5p*QbYpMe3U zKGi#{7iKSj+Y^&Zznx$27pWfq>ks<`;Yb)<+ti%v+oHi3#ygK1 zL*U3%$9)B_<8Y+}jE%9#cP`t1Sfb=bpa2cQlXov!x`izcWE24lWMm}Gg^6TfNr_1# zJc<0!p4wPl4h-b2phMGNvK7G<#?qssFk+hQq>TM)K0brAc#N6E1EknFT};TvsIqf? z7sW7)6?t)ZeAcnNFllmxl+*=Q`|Mazs>^?Gd#@gQ@rlUdeK`)*?=*;iIRW2pwcYk2 zChyQ`^*ykcf$KYh(HosZ`C!!e*^vZFi5yGZ^F^2AV;G_5n{{GSpr-(Wls6FeW`jTG z3bnnq8-c*!@`T+g{X>v;%8C(l3$A?s1sc3Z1pK|&bq(hStGTtf+Jp)Zzw1P`(v_P4 z2pj+6-0iM4DIG#`Ee^7OWJYA&@^V@e2R~BZm8TMq#lBTwXk_E&=0h5l*J8T8n4c2& zaZEspV`Su7Q}is(HL#|MG_G^Z1(2LlJmC?@_dA!TF6_7d=wH*#>Weq<$mik95lwW; z_>Tc8-?~$r-A`N%0H$6n=kI_zPQPM%F=gbH5opBeY$p^?kpCTj!m&4Cu*#1HjuEQP z@4usqNlo-2J?LTdH11SaA50=~v6t|4v}P`NqEALB^N;MGEB&KmdOHGiNBORSJ+yS}z1?9@yE>~f#=#(0b@q@C`2Z!SL=U7uB zToQXSY*P(kwOOpdNPAUC7J831-}H#^9L+v-Fg4GkX>_(=#g&a@`8t)yvDCkZhcP^n{fK;`KrxvTP}1nxl6~ z+hDuyE@iyz0GppaQ=6YU3>aM8LHJ`DgJSTMEuhQ>puRUbo3*40$x$?qDLMmQ0|KG< zdv>l0k*cC^eLy&WNIx&tyEi(TL`x+uv$P^cBA0AIP!5%Ykqhg)il{~|q2z_jz85B5 z^CTm-;yGo1;^IS;n$V5^hiianvO(aDs zc<}0a3OrbN&KNkam;(~?V0%8X>%2#{h(d=yy37|>m)tKXTwfQDBVGo zE+26{R(aZ)8VG>nNl4f~B_p?(x{p(oI^T&OaA9y4+zIQ^2165DPDaqhHzEQ6p)*uD za=`pzcmTG1!{EX#ypt6qj)x|dK`U=5x$U5T`bDCIPr?3!oV)MWW?S96-x*2^g?1g+ z!E*H<6LRh!Tk-~t`XLGU`h-}3{Pd-*jj4WHk1n^$ZL_%N<`W~}sroGLc?4wAplYvh zH|LJdUvFcm+B3`aBG9)#SW^^_z1}PI?ddw`+b8(~Y&QePG$A|M@5HkXdluKY~KkKg|yP#l+}=7fcPXlUL<};+e;e? zv(EMvuhofha;;4LwnC7B=z=VcVipE}d>la`a7Lu9K~>mr{0~fU#=@VT0oYzm;m^O>>x{PY8FuH-=U!!f4Qx7i2M_iuiowchqNvf`qH7PkGa7ba$b@^Tn zu|5R-sb1&YBBkvCTu*e3i+fIgT_))he$)rsx>oOD6sGIt_`WPO#U0Aks(;ALd)8b7 z!5rU!zr&KExN1QpnJ;KT0HME_F?Dr&YZU<7BDdxN6TkuR9VO8A=9t7#x04gEOY|6GY3(@)!^r)K4XUbNux5Yjqnh zi1YGgXG}=l!hhX}Q%rD>gM~%72;~yUL8ksvS=iwTz*)*zG)MRsi*A8brB5RavrfP& z(%VpN$cS$s6W{OK>qu<31t8CVUrhBobfiQNip^5-6}7kR-V0Ws8ak=Je&5PW%{2KI zA%F*U=Dp&=n}z+rNzPV(Yn9e}9Iq)w_2e0HBEYppbUs7vCxa-#d{M{*KZ26Uyx~IS zn=qyDWu~_UHQlkgJ`9f>X?xu$y9t_N4=EqN^nU=;D6lkLgU)a%b})z=D-9 zDHa86HK+AtO6eef;u(qXzmC;V4&_EHr6-lGGX%5+Jjzg>_?E$6dY!F_i;bkJ&$vdQ* zK2tY_8JUuo5ky2D`+C@N&)dA9VMpTuKSw#QQFCy_Jrwbu?EYFsZF4X|5+&K6ULyIa zM|U@1_b^Qe_R_tioQ@ZORt-!|{xcy;c=*a0J%@Z84JxqyuObhoO+c~5P@~)_W`{gG z#85`fEQ-*7g6xp_eaS1!p=4_WmAC~+(4N#|f)Qj+|8JC%w^|~GFD^(l79;mZCi_`qJdV=iY(@)2vY<9QLBV?XR9M=MawVVX)Du&Ud$9f% zY#mODgmJqE3k(x$#>bDYt}UpxUj4hEA+;~y*x($0a|)rtiNgydm{wv{6x}p<`Ps}#f&kk)$5Uv?EJS(=4>kAe!n$E|xRBrm6n-!#PYB{HJ&w#=z%*Vty@^4@3CJtRS4>kV zpTRJ3i>!e1lpXkn9tXh&!6WG-Go0ius~xq2_6{txB_sNy+J(48b5eW-c;3TyaQX{m zA5p!Dts=U^XcgM_5u9yvIlbqczclgh)pX#u9M#4OEReKuc1NujXcZ(NBh^9|93g0b z`NZhqBO?B^mvC?lOL|$xtKh^(NLfqk**dUWN)5aJ6lAQs_{GIAj-AQ5v_ILN0;pnp zchoiPyr;K9jYmJs2D+bnadm}pqJ;gk*v{es_hD?GngL%W;^~7ZocjDwgdx@^?4Au1 zzsQE=qhcDECmHOq{8q1~Bpx#>``coFfHctL4>6g;!*B~wrE&JN5+#avo88fdProPg zJ0gK33IQCHE%!8Jc6B@fCRGA`hIV;=|z=>!;TwhUPX5B1F^&5c%Y_{&j6=t!?y1WB$ z$abU#H4l<*oI9LCbd61j0n(~70z3fkx6s-KuzZJ67D3~9;j0M&o6TL(+_J`|bMTp3 z8O^3{q-VVg)RZQMM(L3~CZ5QD-Yk4`16`y6r+676oQgs3P*>*ObdV18MS!2Z51mJM zY%Ebj&~rRv6|bJ(K;)H*L)(KfC1rC@{!I)uZmjbGfUm=c>}X%y(^&^mWMu2cBjl_l zt^kuZ(*NSad?STvC!0Lk*$d?fB(2}vt^I!VS|qptFE$vmrmBe>E^2vy){W%n4n2-x zh>Cb(I^)wVwhJ(Zr^C?63H}~t_ivB+362W%l6F=HrIr>PFus7Xfc#8YumnUeo2FA2nKGT!@j@`uWH@p9jzS2TsiX17@&NylR&jNl$n{Y;48VkjQoKO zP=P&T%!XB;Z$lKf?rTqffG>R1k-I)h)cE*Q7X4pC4NH+SPXldXt;eFNQbST;(frfiCW#Rk{9U}LW)jqWJl!A&W?wm zUz!qk@5hpvUpeg6^K!9oj58!6IHB@-3`-`2?HmEf~2#f zqchy8e1PJe*GTez5`f2?$5Ha&p5nBSOhvAsvV+1>@~@JC=qxXJ>+g7KMK;4Ogc@OJ zd3HAy)4u%}%RrnX!OQN9sLn=lCV^^)_ z*ta}vcAdV|Wm% z<1+muvI_%a9@FlopZ~1K#VZ0<4KK}o$)9a3OWf$L{Y>LJsI;yiw|> z-L%PC6vv9q#)Lh;hx<$d;An1c9wgQ>pf%xEW0=76?FgrXz2VD~km@q_qt(UffK{k} z*1@7ba>jU)N%8Es9wUJ+oqS;yz~6gPu4n6Py2+Cv-3;#xgZI~CQKD)l8|k+FP7(KD z(c_an4k9TxC|kcj5F>FK)7&p&bQy;YGsS6+k@_%(M~tO%dT1_MYb*!?gNogz|A(u> zFlsSFbiH6dcl?W_@GS$SpR`ZtfdWH+gdw3BBqbK>=LhB^4}+>x))WfHzIK3f`wW6a zyr<|C0VyJh^qu&PEK5(_XNxYpHBK|NjRZt@{s~Q}_POox)zi}} zXqJA?uO-vZW5k$#{!F^*=a#>oe%|~X>gR6Qu=>RQ`+34xpXa?WzOJwS~Q&)-Cn zJEE$@QU#9_=R`(Ce|-t+G~xm1WKrcf`YQk=PoyJ7qY6r;zcKgj-%BG*C!Prf2&iTV z4fouJN5xuqKU~!FL*JKHuhSICJ+=!R?LG|Xkt_|@8_XbZOl>4gq%Hh^l@zOWZW1~v zHa{k1SgBD0o!aW|`Gu_<$d2rUk$QYM$QX&zAeu>MPA#VX!3kj^WUOH!dv88bIL1OC zLjCbCxO=v-hfgwqf(Hjqk;!(sAJ4suY+SW0v-lpfIW#58VHYN-Fc-VM8i{Y_UlF=# zNZENKFfCzE-%|01h_?rShDBe*aRL4U=_F%P7!haDaN{Aj)OXsKoA2;{7r! z4+2lxas}D?dHKGD2?U_)&C%&8d5TRG)3ir1KVX$oK@Qk~4rO0-DUY<^>E?(q4+nbH zw%GV?LR;}rmHq%D_7LHwLcKXRD)00onjbfv^nvzB!)@KMW_apgWy95Y27sl$}&C2lvDfc!s zf$JG?aFq4g-EoDE&CloDxN2tj5ywMJu}VVi?;>T}Z|6G{$t$3bvAv`(BM$6td(-`O zFX`q;`kct5is29>LWIA_VfhuBg7z(_n0fj^3qh?O=zbz9DAQ#0V0CI&Bm%WDq zMt`hS+FKT^rc{AWyn)xWzX60wijR#!l@wHM-Z4uS(g(Hy!4Y6#N2II+yXVs~ELl8C zuZ+oOl*kz}JEFyB4#6la)kht2mApsfWl2N#{s!vNh|lDzbKdc2mZ?HN7?Y&_k2Npu zM#vad%y*+5aoeIn?_ZOqq*aEB2iP(%K!1B{U}I$474LpNvLGL@7*g7Tt>X~2s7AXF zE_;JM2lM8*oW7QWyY5Y=a;cx2GJDt2798<|4W>iwOebfniG>;g8`IDjDJA_7j4;y* z_{qW`b~d?3Mhr-g0wi&e`$&ZFu=x)$07GekrOrvv@WT@M?=zX0SK&2t)BD=wcYp4j zxN>O-BT19EFk{7sIZaEMRdd`nXmHgWEFG-u97rx-0ROvW5B{8&4sCSpJ=s)(S)F^( z6k%+@Z43rb@7bAh+?bjp= zs0-JChuabjLZ7GLEa+=&w8zt20pl?I$Pxjc$U5H);HUr~CIzg38=CN9%t0wRkWfkh zkvLsJ?LW1b;@o++RmUkD=0vID`#viNS>kw*BDvD?QFVlFg3D|WD64NiMt@iW3+8AP z4-$@43ZZ~nWSie=L*%X;i|S zr@L*v!L>cMe5mqJzOz+~N`Fn;;~`YTcy3AKXxCvPZ?a4oNMWH8r%bDUWz-YNcC=kW z#aNURRhIBUSVGz@J}`3g81e&uHN$MOJkNZr+#Q4tbzzFm<6)Z z02)Gg+)#C>hE-(rO)uEb&sqr9<`SM-8O@}uH(1lFkY~mn!;wue{X3cEFFDvq z)+PsONgmrDX@DF~w+|sPg%TBGB_5&Srn`HuS7qMw`OBfAdBiHOY#)d7iOAIlD460Y zxW{;P4up!Qs_%kT|9^-chbSJZxT?Biv861rFQRRef;$d$JXj(QchUe|;fjVhgMBC$ z_aYOOG3M~tK?^bLr)RvU}We9B5HkUIzt& zKB;Ask^DEdeHHN;;AGQ Mwe=w|;gY?QAw^%er~m)} diff --git a/codex-rs/app-server/src/request_processors/persisted_resume_settings.rs b/codex-rs/app-server/src/request_processors/persisted_resume_settings.rs index a9747d58ab..0b66ce67d2 100644 --- a/codex-rs/app-server/src/request_processors/persisted_resume_settings.rs +++ b/codex-rs/app-server/src/request_processors/persisted_resume_settings.rs @@ -43,7 +43,11 @@ impl From<&ThreadSettingsSnapshot> for PersistedResumeSettings { fn from(settings: &ThreadSettingsSnapshot) -> Self { Self { approval_policy: settings.approval_policy, - approvals_reviewer: Some(settings.approvals_reviewer), + approvals_reviewer: Some( + settings + .requested_approvals_reviewer + .unwrap_or(settings.approvals_reviewer), + ), active_permission_profile: settings.active_permission_profile.clone(), unnamed_permission_profile: settings .active_permission_profile @@ -78,9 +82,12 @@ fn latest_persisted_resume_settings(history: &[RolloutItem]) -> Option turn_context.approvals_reviewer, - RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => { - Some(event.thread_settings.approvals_reviewer) - } + RolloutItem::EventMsg(EventMsg::ThreadSettingsApplied(event)) => Some( + event + .thread_settings + .requested_approvals_reviewer + .unwrap_or(event.thread_settings.approvals_reviewer), + ), _ => None, }) }), @@ -93,7 +100,11 @@ fn latest_persisted_resume_settings(history: &[RolloutItem]) -> Option Result< Ok(()) } +#[test_case(ApprovalsReviewer::AutoReview, "", ApprovalsReviewer::AutoReview; "explicit auto review")] +#[test_case(ApprovalsReviewer::User, "", ApprovalsReviewer::User; "requirement removed")] +#[test_case(ApprovalsReviewer::User, "allowed_approvals_reviewers = [\"auto_review\"]", ApprovalsReviewer::AutoReview; "allowlist tightened")] #[tokio::test] -async fn thread_resume_preserves_persisted_approvals_reviewer() -> Result<()> { +async fn thread_resume_preserves_persisted_approvals_reviewer( + requested_reviewer: ApprovalsReviewer, + resumed_requirements: &str, + expected_reviewer: ApprovalsReviewer, +) -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; let codex_home = TempDir::new()?; mock_responses_config(&server.uri()).write(codex_home.path())?; + let requirements_path = codex_home.path().join("requirements.toml"); + std::fs::write( + &requirements_path, + "[auto_review]\nrequired_on_models = [\"gpt-5.4\"]\n", + )?; + let thread_id = { let mut mcp = TestAppServer::builder() .with_codex_home(codex_home.path()) @@ -1214,12 +1227,16 @@ async fn thread_resume_preserves_persisted_approvals_reviewer() -> Result<()> { let start_id = mcp .send_thread_start_request(ThreadStartParams { model: Some("gpt-5.4".to_string()), - approvals_reviewer: Some(ApprovalsReviewer::AutoReview), + approvals_reviewer: Some(requested_reviewer), ..Default::default() }) .await?; - let ThreadStartResponse { thread, .. } = - timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(start_id)).await??; + let ThreadStartResponse { + thread, + approvals_reviewer, + .. + } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(start_id)).await??; + assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview); let turn_id = mcp .send_turn_start_request(TurnStartParams { @@ -1246,6 +1263,7 @@ async fn thread_resume_preserves_persisted_approvals_reviewer() -> Result<()> { thread.id }; + std::fs::write(requirements_path, resumed_requirements)?; let config_path = codex_home.path().join("config.toml"); let config = std::fs::read_to_string(&config_path)?; std::fs::write( @@ -1271,7 +1289,7 @@ async fn thread_resume_preserves_persisted_approvals_reviewer() -> Result<()> { approvals_reviewer, .. } = timeout(DEFAULT_READ_TIMEOUT, mcp.read_response(resume_id)).await??; - assert_eq!(approvals_reviewer, ApprovalsReviewer::AutoReview); + assert_eq!(approvals_reviewer, expected_reviewer); Ok(()) } diff --git a/codex-rs/core/src/agent/control/spawn.rs b/codex-rs/core/src/agent/control/spawn.rs index a4ac17f71a..e557f814c9 100644 --- a/codex-rs/core/src/agent/control/spawn.rs +++ b/codex-rs/core/src/agent/control/spawn.rs @@ -281,6 +281,7 @@ impl LocalAgentControl { // Preserve the worker's permissions while keeping the current wake configuration. config.permissions = evicted.config.permissions.clone(); config.approvals_reviewer = evicted.config.approvals_reviewer; + config.requested_approvals_reviewer = evicted.config.requested_approvals_reviewer; } let mut environment_selections = evicted_environments .as_ref() diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 91fcfe365f..a0f3ea2e2d 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -1172,10 +1172,13 @@ async fn cancelled_v2_send_finishes_reload_registration() { .is_some() ); + let mut sender_config = harness.config.clone(); + sender_config.approvals_reviewer = ApprovalsReviewer::AutoReview; + sender_config.requested_approvals_reviewer = Some(ApprovalsReviewer::AutoReview); let followup_request = |message: &str| SendRequest { caller: parent_id, target: AgentTarget::Id(child_id), - resume_config: harness.config.clone(), + resume_config: sender_config.clone(), input: AgentInput::Message { message: AgentMessage::Plaintext(message.to_string()), mode: MessageDeliveryMode::TriggerTurn, @@ -1223,6 +1226,11 @@ async fn cancelled_v2_send_finishes_reload_registration() { .evicted_environments(child_id) .is_none() ); + assert_eq!( + reloaded.config_snapshot().await.approvals_reviewer, + ApprovalsReviewer::User, + "reload must preserve the worker's reviewer instead of inheriting its sender's request", + ); // Evict and reload again with the single child slot. This also catches a leaked // pin or pending residency reservation left behind by the cancelled sender. assert_eq!( @@ -2459,6 +2467,7 @@ async fn spawn_agent_fork_from_paginated_parent_uses_model_context_prefix( service_tier: None, approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, + requested_approvals_reviewer: None, permission_profile: PermissionProfile::workspace_write(), active_permission_profile: None, cwd: harness.config.cwd.clone(), diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 1dd31677bb..5b47ffe4c2 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -161,6 +161,8 @@ pub struct CodexThreadSettingsOverrides { pub profile_workspace_roots: Option>, pub approval_policy: Option, pub approvals_reviewer: Option, + /// Retained selection when restoring an effective reviewer forced by policy. + pub requested_approvals_reviewer: Option, pub sandbox_policy: Option, pub permission_profile: Option, pub active_permission_profile: Option, @@ -877,6 +879,7 @@ impl CodexThread { profile_workspace_roots, approval_policy, approvals_reviewer, + requested_approvals_reviewer, sandbox_policy, permission_profile, active_permission_profile, @@ -890,6 +893,7 @@ impl CodexThread { disabled_plugin_ids, } = overrides; SessionSettingsUpdate { + requested_approvals_reviewer, turn_extension_init, step_settings: StepSettingsUpdate { model, diff --git a/codex-rs/core/src/config/guardian_policy.rs b/codex-rs/core/src/config/guardian_policy.rs new file mode 100644 index 0000000000..e1f21cde38 --- /dev/null +++ b/codex-rs/core/src/config/guardian_policy.rs @@ -0,0 +1,23 @@ +//! Resolves the effective Guardian reviewer while retaining the requested choice. + +use codex_config::ConfigRequirements; +use codex_protocol::config_types::ApprovalsReviewer; + +/// Resolves the effective reviewer without replacing the retained request. +/// Trusted private reviewers omit the model to bypass model-required review. +pub(crate) fn resolve_guardian_reviewer( + requested: ApprovalsReviewer, + model: Option<&str>, + requirements: &ConfigRequirements, +) -> super::ConstraintResult { + let reviewer = if model.is_some_and(|model| requirements.auto_review_required_for_model(model)) + { + ApprovalsReviewer::AutoReview + } else if requirements.approvals_reviewer.can_set(&requested).is_ok() { + requested + } else { + requirements.approvals_reviewer.value() + }; + requirements.approvals_reviewer.can_set(&reviewer)?; + Ok(reviewer) +} diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 6990af063c..d32db75af7 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -168,6 +168,7 @@ use toml_edit::DocumentMut; mod auth_keyring; pub mod edit; +mod guardian_policy; mod managed_features; mod metrics; mod network_config; @@ -179,6 +180,7 @@ mod permission_profile_selection; mod permissions; mod requirements; mod resolved_permission_profile; +pub(crate) use guardian_policy::resolve_guardian_reviewer; mod runtime_refresh; pub(crate) use runtime_refresh::RuntimeConfigRefresh; #[cfg(test)] @@ -693,6 +695,9 @@ pub struct Config { /// been escalated. This does not disable separate safety checks such as /// ARC. pub approvals_reviewer: ApprovalsReviewer, + /// Requested reviewer when managed requirements replace the effective choice. + /// `None` uses `approvals_reviewer`, including programmatically constructed configs. + pub requested_approvals_reviewer: Option, /// enforce_residency means web traffic cannot be routed outside of a /// particular geography. HTTP clients should direct their requests @@ -3993,6 +3998,7 @@ impl Config { let mut approvals_reviewer = approvals_reviewer_override .or(cfg.approvals_reviewer) .unwrap_or(ApprovalsReviewer::User); + let requested_approvals_reviewer = approvals_reviewer; if !approvals_reviewer_was_explicit && let Err(err) = constrained_approvals_reviewer.can_set(&approvals_reviewer) { @@ -4593,6 +4599,9 @@ impl Config { explicit_permission_profile_mode, custom_permission_profiles, approvals_reviewer: constrained_approvals_reviewer.value(), + requested_approvals_reviewer: (requested_approvals_reviewer + != constrained_approvals_reviewer.value()) + .then_some(requested_approvals_reviewer), enforce_residency: enforce_residency.value, notify: cfg.notify, base_instructions, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index b620d57db5..64941d620f 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -798,6 +798,9 @@ impl Session { "No models are available. Set `model` explicitly or check your model catalog configuration.".to_string(), )); } + let requested_approvals_reviewer = config + .requested_approvals_reviewer + .unwrap_or(config.approvals_reviewer); let trusted_guardian_reviewer = crate::guardian::is_basic_session_source(&session_source) && !matches!(conversation_history, InitialHistory::Resumed(_)); if config @@ -824,13 +827,17 @@ impl Session { ); } } - config - .config_layer_stack - .requirements() - .approvals_reviewer - .can_set(&ApprovalsReviewer::AutoReview) - .map_err(|err| CodexErr::InvalidRequest(err.to_string()))?; - config.approvals_reviewer = ApprovalsReviewer::AutoReview; + } + let reviewer = crate::config::resolve_guardian_reviewer( + requested_approvals_reviewer, + (!trusted_guardian_reviewer).then_some(model.as_str()), + config.config_layer_stack.requirements(), + ) + .map_err(|err| CodexErr::InvalidRequest(err.to_string()))?; + { + let config = Arc::make_mut(&mut config); + config.requested_approvals_reviewer = Some(requested_approvals_reviewer); + config.approvals_reviewer = reviewer; } if allow_provider_model_fallback && let Some(requested_model) = config.model.as_ref() diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index dc4ae7ba02..5cc388cd40 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -336,6 +336,9 @@ impl SessionConfiguration { service_tier: self.step_settings.service_tier.clone(), approval_policy: self.step_settings.approval_policy.value(), approvals_reviewer: self.step_settings.approvals_reviewer, + requested_approvals_reviewer: self + .original_config_do_not_use + .requested_approvals_reviewer, permission_profile: self.materialized_permission_profile(environment_selections), active_permission_profile: self.active_permission_profile(), cwd: self.legacy_fallback_cwd.clone(), @@ -369,6 +372,9 @@ impl SessionConfiguration { ), approval_policy: Some(self.step_settings.approval_policy.value()), approvals_reviewer: Some(self.step_settings.approvals_reviewer), + requested_approvals_reviewer: self + .original_config_do_not_use + .requested_approvals_reviewer, permission_profile: Some(self.permission_profile()), active_permission_profile: self.active_permission_profile(), windows_sandbox_level: Some(self.windows_sandbox_level), @@ -575,6 +581,17 @@ impl SessionConfiguration { &updates.step_settings, &next_configuration.step_settings_constraints(next_environments), )?); + if let Some(reviewer) = updates + .requested_approvals_reviewer + .or(updates.step_settings.approvals_reviewer) + && next_configuration + .original_config_do_not_use + .requested_approvals_reviewer + != Some(reviewer) + { + Arc::make_mut(&mut next_configuration.original_config_do_not_use) + .requested_approvals_reviewer = Some(reviewer); + } Ok(next_configuration) } @@ -624,6 +641,7 @@ pub(crate) struct SessionSettingsCommit { #[derive(Default, Clone)] pub(crate) struct SessionSettingsUpdate { pub(crate) step_settings: StepSettingsUpdate, + pub(crate) requested_approvals_reviewer: Option, /// Omission preserves the current data; an empty initializer clears it. pub(crate) turn_extension_init: Option, pub(crate) environments: Option, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 6a6bef52a5..e6d7448dfe 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -2483,6 +2483,11 @@ pub struct ThreadSettingsSnapshot { pub service_tier: Option, pub approval_policy: AskForApproval, pub approvals_reviewer: ApprovalsReviewer, + /// Original selection before managed policy resolved the effective reviewer. + /// Older histories contain only `approvals_reviewer`. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub requested_approvals_reviewer: Option, pub permission_profile: PermissionProfile, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] diff --git a/codex-rs/state/src/extract.rs b/codex-rs/state/src/extract.rs index 2b22fd5d78..35358a62b1 100644 --- a/codex-rs/state/src/extract.rs +++ b/codex-rs/state/src/extract.rs @@ -745,6 +745,7 @@ mod tests { service_tier: None, approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, + requested_approvals_reviewer: None, permission_profile: permission_profile.clone(), active_permission_profile: None, cwd: cwd.clone().try_into().expect("absolute settings cwd"), diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index ecee9645d4..1aab982144 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -265,6 +265,7 @@ async fn new_config( explicit_permission_profile_mode: false, custom_permission_profiles: Vec::new(), approvals_reviewer: ApprovalsReviewer::User, + requested_approvals_reviewer: None, enforce_residency: Constrained::allow_any(/*initial_value*/ None), hide_agent_reasoning: false, show_raw_agent_reasoning: false, diff --git a/codex-rs/thread-store/src/thread_metadata_sync.rs b/codex-rs/thread-store/src/thread_metadata_sync.rs index c62bfe1bea..6f1a7d1791 100644 --- a/codex-rs/thread-store/src/thread_metadata_sync.rs +++ b/codex-rs/thread-store/src/thread_metadata_sync.rs @@ -750,6 +750,7 @@ mod tests { service_tier: None, approval_policy: AskForApproval::Never, approvals_reviewer: ApprovalsReviewer::User, + requested_approvals_reviewer: None, permission_profile: permission_profile.clone(), active_permission_profile: None, cwd: cwd.clone().try_into().expect("absolute settings cwd"), diff --git a/codex-rs/tui/src/app/misalignment_policy.rs b/codex-rs/tui/src/app/misalignment_policy.rs index eece358c3b..7eb6f5b58a 100644 --- a/codex-rs/tui/src/app/misalignment_policy.rs +++ b/codex-rs/tui/src/app/misalignment_policy.rs @@ -171,7 +171,6 @@ impl App { params.runtime_workspace_roots = Some(config.permissions.user_visible_workspace_roots().to_vec()); params.approval_policy = Some(config.permissions.approval_policy.value().into()); - params.approvals_reviewer = Some(config.approvals_reviewer.into()); params.sandbox_policy = sandbox_policy; params.permissions = permissions; } diff --git a/codex-rs/tui/src/app/safety_buffering.rs b/codex-rs/tui/src/app/safety_buffering.rs index 72da56bfcd..659bcbacaf 100644 --- a/codex-rs/tui/src/app/safety_buffering.rs +++ b/codex-rs/tui/src/app/safety_buffering.rs @@ -62,7 +62,6 @@ impl App { cwd, active_permission_profile, approval_policy, - approvals_reviewer, model: turn_model, effort, collaboration_mode, @@ -217,7 +216,6 @@ impl App { }; let retry_thread_id = started.session.thread_id; *approval_policy = started.session.approval_policy; - *approvals_reviewer = Some(started.session.approvals_reviewer); *active_permission_profile = started.session.active_permission_profile.clone(); self.detach_current_thread_for_navigation(app_server, Some(retry_thread_id)) diff --git a/codex-rs/tui/src/app/tests/misalignment_policy_tests.rs b/codex-rs/tui/src/app/tests/misalignment_policy_tests.rs index 04e554de99..29ced74fcc 100644 --- a/codex-rs/tui/src/app/tests/misalignment_policy_tests.rs +++ b/codex-rs/tui/src/app/tests/misalignment_policy_tests.rs @@ -252,7 +252,7 @@ async fn misalignment_continuation_requires_current_review_and_submits_once() -> runtime_workspace_roots: (!server_owned) .then(|| config.permissions.user_visible_workspace_roots().to_vec()), approval_policy: (!server_owned).then_some(AskForApproval::OnRequest), - approvals_reviewer: (!server_owned).then(|| config.approvals_reviewer.into()), + approvals_reviewer: None, sandbox_policy: (!server_owned && !preserve) .then(|| config.legacy_sandbox_policy().into()), cyber_access_program: rollout_enabled.then_some(if daybreak { diff --git a/codex-rs/tui/src/app/tests/startup_defaults_tests.rs b/codex-rs/tui/src/app/tests/startup_defaults_tests.rs index bed5ce30b0..ea537b9f80 100644 --- a/codex-rs/tui/src/app/tests/startup_defaults_tests.rs +++ b/codex-rs/tui/src/app/tests/startup_defaults_tests.rs @@ -517,8 +517,20 @@ requires_openai_auth = false serde_json::json!({"cwd": server_config.cwd.display().to_string(), "includeLayers": true}) ], ); - if choice == "saved" { - assert_eq!(started.session.approvals_reviewer, ApprovalsReviewer::User); + if matches!(choice, "saved" | "embedded_saved") { + let expected_turn_policy = if choice == "saved" { + AskForApproval::OnRequest + } else { + AskForApproval::Never + }; + assert_eq!( + started.session.approvals_reviewer, + if choice == "saved" { + ApprovalsReviewer::User + } else { + ApprovalsReviewer::AutoReview + } + ); let (mut app, _events, mut ops) = make_test_app_with_channels().await; app.app_server_target = target; app.config.permissions.approval_policy = @@ -530,7 +542,7 @@ requires_openai_auth = false .await?; assert_eq!( app.config.permissions.approval_policy.value(), - AskForApproval::OnRequest.to_core() + expected_turn_policy.to_core() ); app.chat_widget.insert_str("first turn"); app.chat_widget @@ -538,8 +550,11 @@ requires_openai_auth = false app.submit_thread_op(&mut server, thread_id, next_user_turn_op(&mut ops)) .await?; let turn = &recorded_params(&requests, "turn/start")[0]; - assert_eq!(turn["approvalsReviewer"], "user"); - assert_eq!(turn["approvalPolicy"], "on-request"); + assert!(turn["approvalsReviewer"].is_null()); + assert_eq!( + turn["approvalPolicy"], + serde_json::json!(expected_turn_policy) + ); } server.shutdown().await?; proxy.await??; diff --git a/codex-rs/tui/src/app/user_turn_submission.rs b/codex-rs/tui/src/app/user_turn_submission.rs index 229d21e22b..cdbda63973 100644 --- a/codex-rs/tui/src/app/user_turn_submission.rs +++ b/codex-rs/tui/src/app/user_turn_submission.rs @@ -120,6 +120,7 @@ pub(super) async fn try_submit_user_turn( }); let selected_active = selected_profile.map(|profile| ActivePermissionProfile::new(profile.profile_id.clone())); + // Ordinary turns inherit the server reviewer; echoing its effective value loses the request. let (turn_approval_policy, turn_approvals_reviewer) = if let Some(profile) = selected_profile { ( profile.approval_policy, @@ -130,17 +131,10 @@ pub(super) async fn try_submit_user_turn( { ( Some(config.permissions.approval_policy.value().into()), - Some(config.approvals_reviewer.into()), + approvals_reviewer.map(Into::into), ) } else { - ( - Some(*approval_policy), - Some( - approvals_reviewer - .unwrap_or(config.approvals_reviewer) - .into(), - ), - ) + (Some(*approval_policy), approvals_reviewer.map(Into::into)) }; // Only explicit choices may override the saved server profile. let explicit_profile = app diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 78f5ab59f1..38ada25c33 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -1274,6 +1274,12 @@ impl AppServerSession { // Active-session config has already adopted the server's roots. Fork does // not restore saved roots when omitted, so preserve this explicit selection. params.runtime_workspace_roots = Some(config.workspace_roots.clone()); + if selected_profile.is_none() { + params.approvals_reviewer = None; + if let Some(overrides) = params.config.as_mut() { + overrides.remove("approvals_reviewer"); + } + } } if self.thread_params_mode() == ThreadParamsMode::Remote && permission_mode == ForkPermissionMode::InheritSaved