From c9b19deb09c1841ce7acc33ddb96276030936a29 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Sun, 23 Aug 2026 10:01:24 +0000 Subject: [PATCH] Distinguish Guardian review threads from subagents (#40221) ## Why Guardian reviews were reported with the generic `subagent` thread source, making them indistinguishable from other delegated threads in persisted metadata and analytics. ## What changed - Add the `guardian_review` thread source to the core protocol and app-server schema. - Assign it to Guardian reviewer sessions and propagate the configured source through turn metadata, rollouts, and analytics. - Continue treating Guardian reviews like subagents for paginated history and trusted-provenance checks. ## Testing - Cover protocol round trips, delegate source selection, emitted analytics, request metadata, and persisted rollout metadata. GitOrigin-RevId: 07480c122715812874c9d3b48bb39fc5c86b2367 --- .../analytics/src/analytics_client_tests.rs | 23 ++++++++++-------- codex-rs/analytics/src/events.rs | 2 +- codex-rs/analytics/src/facts.rs | 2 ++ codex-rs/analytics/src/reducer.rs | 2 +- .../app-server-exports-stable.json.zst | Bin 137293 -> 137300 bytes .../src/protocol/v2/tests.rs | 1 + .../src/protocol/v2/thread_data.rs | 3 +++ codex-rs/core/src/codex_delegate.rs | 9 +++++-- codex-rs/core/src/codex_delegate_tests.rs | 9 +++++-- codex-rs/core/src/session/mod.rs | 3 ++- codex-rs/core/src/session/session.rs | 2 +- codex-rs/core/src/session/tests.rs | 10 +++----- codex-rs/core/src/turn_metadata.rs | 6 ++++- codex-rs/core/tests/suite/guardian_review.rs | 21 ++++++++++++++-- .../tests/suite/subagent_notifications.rs | 6 +++++ codex-rs/protocol/src/protocol.rs | 3 +++ 16 files changed, 74 insertions(+), 28 deletions(-) diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index dfde8cf3e1..56ecf981d7 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -3232,6 +3232,7 @@ fn subagent_thread_started_review_serializes_expected_shape() { client_version: "1.0.0".to_string(), model: "gpt-5".to_string(), ephemeral: false, + thread_source: Some(ThreadSource::Subagent), subagent_source: SubAgentSource::Review, created_at: 123, }, @@ -3284,6 +3285,7 @@ fn subagent_thread_started_thread_spawn_serializes_thread_lineage() { client_version: "1.0.0".to_string(), model: "gpt-5".to_string(), ephemeral: true, + thread_source: Some(ThreadSource::Subagent), subagent_source: SubAgentSource::ThreadSpawn { parent_thread_id, depth: 1, @@ -3323,6 +3325,7 @@ fn subagent_thread_started_memory_consolidation_serializes_expected_shape() { client_version: "1.0.0".to_string(), model: "gpt-5".to_string(), ephemeral: false, + thread_source: Some(ThreadSource::Subagent), subagent_source: SubAgentSource::MemoryConsolidation, created_at: 125, }, @@ -3350,12 +3353,14 @@ fn subagent_thread_started_other_serializes_expected_shape() { client_version: "1.0.0".to_string(), model: "gpt-5".to_string(), ephemeral: false, + thread_source: Some(ThreadSource::GuardianReview), subagent_source: SubAgentSource::Other("guardian".to_string()), created_at: 126, }, )); let payload = serde_json::to_value(&event).expect("serialize other subagent event"); + assert_eq!(payload["event_params"]["thread_source"], "guardian_review"); assert_eq!(payload["event_params"]["subagent_source"], "guardian"); assert_eq!(payload["event_params"]["parent_thread_id"], json!(null)); } @@ -3376,6 +3381,7 @@ fn subagent_thread_started_other_serializes_explicit_parent_thread_id() { client_version: "1.0.0".to_string(), model: "gpt-5".to_string(), ephemeral: false, + thread_source: Some(ThreadSource::GuardianReview), subagent_source: SubAgentSource::Other("guardian".to_string()), created_at: 126, }, @@ -3407,6 +3413,7 @@ async fn subagent_thread_started_publishes_without_initialize() { client_version: "1.0.0".to_string(), model: "gpt-5".to_string(), ephemeral: false, + thread_source: Some(ThreadSource::Subagent), subagent_source: SubAgentSource::Review, created_at: 127, }, @@ -3427,7 +3434,7 @@ async fn subagent_thread_started_publishes_without_initialize() { } #[tokio::test] -async fn subagent_events_keep_thread_originator_with_explicit_turn_connection() { +async fn guardian_events_keep_thread_source_and_originator_with_explicit_turn_connection() { let mut reducer = AnalyticsReducer::default(); let mut events = Vec::new(); let parent_thread_id = @@ -3483,13 +3490,8 @@ async fn subagent_events_keep_thread_originator_with_explicit_turn_connection() client_version: "1.0.0".to_string(), model: "gpt-5".to_string(), ephemeral: false, - subagent_source: SubAgentSource::ThreadSpawn { - parent_thread_id, - depth: 1, - agent_path: None, - agent_nickname: None, - agent_role: None, - }, + thread_source: Some(ThreadSource::GuardianReview), + subagent_source: SubAgentSource::Other("guardian".to_string()), created_at: 130, }, )), @@ -3546,8 +3548,8 @@ async fn subagent_events_keep_thread_originator_with_explicit_turn_connection() }; let params = &event.event_params; assert_eq!(params.session_id, "session-root"); - assert_eq!(params.thread_source, Some(ThreadSource::Subagent)); - assert_eq!(params.subagent_source.as_deref(), Some("thread_spawn")); + assert_eq!(params.thread_source, Some(ThreadSource::GuardianReview)); + assert_eq!(params.subagent_source.as_deref(), Some("guardian")); assert_eq!( params.parent_thread_id.as_deref(), Some("44444444-4444-4444-4444-444444444444") @@ -3625,6 +3627,7 @@ async fn subagent_tool_items_inherit_parent_connection_metadata() { client_version: "1.0.0".to_string(), model: "gpt-5".to_string(), ephemeral: false, + thread_source: Some(ThreadSource::Subagent), subagent_source: SubAgentSource::Review, created_at: 128, }, diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index a06fb6f05d..6b7c159109 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -1449,7 +1449,7 @@ pub(crate) fn subagent_thread_started_event_request( runtime: current_runtime_metadata(), model: input.model, ephemeral: input.ephemeral, - thread_source: Some(ThreadSource::Subagent), + thread_source: input.thread_source, initialization_mode: ThreadInitializationMode::New, subagent_source: Some(subagent_source_name(&input.subagent_source)), parent_thread_id: input.parent_thread_id, diff --git a/codex-rs/analytics/src/facts.rs b/codex-rs/analytics/src/facts.rs index 3ca5b3327c..e240f6c7b1 100644 --- a/codex-rs/analytics/src/facts.rs +++ b/codex-rs/analytics/src/facts.rs @@ -28,6 +28,7 @@ use codex_protocol::protocol::HookSource; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SkillScope; use codex_protocol::protocol::SubAgentSource; +use codex_protocol::protocol::ThreadSource; use codex_protocol::protocol::TokenUsage; use codex_protocol::request_permissions::RequestPermissionsResponse; use serde::Serialize; @@ -388,6 +389,7 @@ pub struct SubAgentThreadStartedInput { pub client_version: String, pub model: String, pub ephemeral: bool, + pub thread_source: Option, pub subagent_source: SubAgentSource, pub created_at: u64, } diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index 59434ad086..226bf54540 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -1069,7 +1069,7 @@ impl AnalyticsReducer { .metadata .get_or_insert_with(|| ThreadMetadataState { session_id: input.session_id.clone(), - thread_source: Some(ThreadSource::Subagent), + thread_source: input.thread_source.clone(), initialization_mode: ThreadInitializationMode::New, subagent_source: Some(subagent_source_name(&input.subagent_source)), parent_thread_id, 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 6357817af7b47edd153385f5eaba3460eca9ce03..bd468c2323eeacd94805f4f1a53860c4244872af 100644 GIT binary patch delta 4590 zcmV%C-O~L~a z(+aQ(0SF5OO8`m$NC019B#sM3Q*|U2s_m;btr|3z%I6a>&8m723m0eEdVT5{`(_HM zy+*QGOybVkeR>t!&cc@kxFw*DYl0(6TWx43bOf1%wj4a3$Y zPv7q7d%#mstXnc!8Lb?G(I-{Er8O87oYTCfvb6s?+7(w7&@1xo7E(@WKbJ7WU4m!o zu7@=>Nh|OIl^@c5}JH|CYL>95}Q?Gp-^B9 z6WbC3E8!>zJi))STkRaB0^w*c6oIPB13_P~WJeXY^-e@>QqF&r^j^tcIL~RQ`S}Bd zEvTanSxO460aG9lNI-z#0D(Xhkplu^nH^{P697U(MIvJ$U<3w%e?c%15Cnq&7(oyO zK@bE-5ClO$h9ME866r3C0kq%9$F`xr!FAP~`?@2Rd5--RH7Nb&w8c=6A{x%p>q%r`|Q8V?lVU*J(;Q|8z3uZ%O~0K{eC0MXQXE!V@Bn zjTp#|5~>sx>Wd#jS+s{j0D~bkL=q>#@Ml-#k7t^hXJcgMf<$Fh@4Re&(0A^lFXlWA zWz}sV$B!lK309$V$g^PoygX#f3oyu%i=tnN5>Y8rkU*# ztdic?W@{=v@?bWXpz7Jmkk1o=01lvSv?*Zy^}eZ6$dCb5P^P96S|OkzinUMDZ`2dH zOIV=h=m23~tEpVT+A=sCuqFJ@>b3}y#z!?6P3M1_YsDWa@>T6L#too61yxg)Z;or? zy4iBU>{+bie|V^a@j)g8T+H7_zazt9MGoG2+9(mU*&ORIu%``K>cLdtlN6{@zqw)Z zAq5*+W5(RiUKg-Fqy23j-@ZxK!b>q6*Fro5pi7uQ0dBKd*NXPX0g_IqTaE{)>*vFu zf9S`o^w&nDm!v$=er=HJ0hmef?A__$6eX}LpiIN_;g zY8#z6f77Nq+l%my>GblwRj%%-tn2U(Wr@?cUZPto1$&o8?e25)DINHi+X8hfuVKg( zea*{TF~2LRdcfa=+HKoh&b~VrE#`fAq{YqT&nABx2$#wZ?fMYmN6R$$iXLu`tqjbO zgurAn;IJ5gCBhalkf`k53$NuR<3>=PhxGDge}*gFb!ox*JT%fa9t1jKEmn;< zjCA07QQrRtxS3Kc&1bN$od|zT%1ZN~fVbdsH#hH3!5eE3x9mcK@SWp>%-|;a6gMSR zo-RPLd|KzIkg$(~H&uX>Vx9RgEmllv|SCU3y?jf8-90p=nm~?_>oTt{>H1q^Juefo3QiD1y=? z#)JyUERNLVI-i;AdQK1|u;*#yp%YI{m&;`tb9*&EpOK473;HA1cTH~fDlF-Vyw12b z+IE}o2}|8V&%p&{KxJMRMmY7TpaN_9-yqi-ZDdnYcLsM(IV%~%%h?*wX^;M8f9bpD ztKIq$8Z9NLy52r>Gi%_~7UeP>tW>?Z0~m_?NR4B?A2rutQ^@?N>29p}B2%o%uCF1s zC>9+H;L%oCJ-zV*c4bGb5*zPlVwBKmoNR0Gdn z#`UwGz-IvG;f0|J3qRfL*g$^@${0_=#4i@7Xgd4om=ji$y>dj(5 z31%;om!I$IkbdWpgijg(cZF?CneTQ9)MwXgz7?V*lZII#3Jz;({}K+FyiGYN??N?X z8x~Fg$?H|26k1MT%5-r-u|S49G;Yb!lqq^c$8F1+H)kl5|37iV%p@k z>2dXk*Tr~Y&ppBL8CZH4f6JOd7dj}>1)XPWzB4S)56BaU^Lc*ZJKo$z5n-j><963K)czOeZUO=x6w5B_&3V!D;0^0yVI{Y?YoV zD3yOgaKBA>#K{%(^d#RhP1rKzG&u3y67vR!cc6rnD1*`F=*4s(*!-@$ zkj>s?4DqX#9qEn{So50f{^%B$%BsC^|${z*x{Y(%$Am3V}!~jmgM|MsP12apZ$u zy$%y>_4{9*z+HvRe@8>27DGo+Xl+M6ym>#RuqpF=kQK7)MGRK!`4ue)8voxJX<0HWy;jFx(^R+<2x$~Gz3rS&Q*|I|5sN>< zEfg^+Y0*;-{l0sV+FuF#xdJ=JGp z2f5=$SX2Yjc*nDgwivQT6R+UGWyTuYpcirjn?Jmgz42L6Ll{_sCG(mX1jX^4Jbi$x zZVs%@vTd!_zDFm#iP(o;S_WG_^vO)~OZWbwRrb0sl=pi6?#} z0@vyKph>-y8hpD*>T_=wcfc*O)cNary9jIz)V{!S>w- zA>?B~!72ly?;Fr0F*i?PuWLQ%+yIC_Y|;? z7${)#*x?eJzarAXf1DmgUA->C2d+r72)WYK@eczk$SVWtv@F=!Cf23bk zuRi5`Q!0Y{xY(_>byX7qk{<)O2TDd=YL?=}%PVXE?()BbpjCEQ^#G}i1IJMko{v-* zGas*j!KJwdyu$*XD=1szaRS|;STrl>8$32`IHVE#y>udJCZzCb5kb^5bQjsw(5BS; z)i})2;CZV^_%?eV6a=k!z4sijBY`-L|}!70tl;C4%%vUuvaN?wRW5i zWCd4to-y#D0Z5#sXeBD9IUXC*60wSgtGo|+ND$rJCx~ZM-uVWAg+h-99uCQiWoV|xwbFvUXU(F=2xRu!v11(MiA==C|o%R+PW%n zl{dSRN>CH*OC;dMe@2XvWb;a{CZeo;<=HH0gBw+|3qAu+E_5>q3e~q*{4sCK^yC9t zG0~eQqkY|WmPyskn}4llop+8LfK4p`Cy_2mfm^B8~2Fp^jX=rTGIW=IPaNbh!6e;LfuL1dDuG6Dwa5SK!* z1?Zzteh$5gx#ev`gLOTh%aolWnqg32htNbCWX4-JZ53$X+;?NU!gXHDxaBo(G| zW;`BjIXo;xFS^|CgauZW_-sOjfXB^WYNG4%{c$=8rgSs%7BhaJDiW~}DSnle@*j78 zVSFuN#B+Xqe+l|B?N^;GhbNeM$R6ANFOoWNeDnJToydf83eri^w9<%aS+21Flqzv` z&Izg&os2pwzf>VZKp{5E{)E`iAm>1BVY6V^_!@iIE14;x$>lw1Eg!k9v?S+UnsD|A z(kl`bt6o9ulzkHs65~o`U1aq?IS}n(GG#+xYF7`sf3oZibyI}u9SW&|yW-Rp?TXJtMiYB&b5t6?-vvq!*gidx#<^hNB01t!zRr4xC&eF3fOmqdj$ z)NXvTh*E+#a7V?`J??n5d%32RT6RJdNtgs@kujf8r6P5Rv)cuHO3|hAPs=V+;c+?E zt$H99f08h=Q_lr6zR|=$qv$;N{*W&W-6VV|r);pDwQ-o$hHprP-dS+!!C~dx&!Gfl zqNo+12t(e~6UDJn6umBTfONr~5&=;h#!7!TVK%^jz25&oX%fnJ^E@|-E_nPlu*@0Y zz`+nQY?ZJ~5{kLcNo{3(G zo8hPq7u#`B;)kciu=G6x=Ly{Au9rSBl@~<5#xHW>4Sznx9KMD0TWiQ!YL8@^0TYv% Y(s7~{m^yJvtXrE*F%ddg2)2ueC$}oYnE(I) delta 4583 zcmVfb_x~%m_aMZ|`>MDx`$er^bG#QP!ZPqWH06!b7cd|K4VyREf3m@LN%%+* zl&`h^sFuHeAH09sZ6 zQ=pNNZH~sGSRDVp?yye}5{9Wc8OFjn3TE*SN2%rk(sXK!D%?c|a5q1Oif_9cTIz00M(WB4Z$61O|aYe=rab1cLw>K@bE% z5ClgM1VKQCArYby=`M@`li$e4wqL)&Ue%m;x+8{n4t)hRD1Fwn#SV}n3|OM4dhvjr z@WxJVU`oQGq=H2FcPjr!YLE z#(vd!pZihU0LCS>hs~ke_sQC4sT0~+Sp5Dc28JWQZ6)hk-u^g4(phB5F$ZS);S=Z| zDDWcv+w0^dL7r%yHkf(*Tb?viBZyE&FPH)R}l19Jm2Xfg*@*3Wn3w}>*JYbUIzqA48=ED5b! z@B;!F4L$z~w#p8gP$LD}fmw&h0iTlV=`ot~Qyxrf3O~-0%Cbzq*A@ctrDB zHYcRCe>i;Y@vCFv*7 z4jAtj;@eEijpi2Xs~W*y->%V&55W6rBAc2z2#4Q1bFprE&*=3@K5uUss3RJo{#n4z` zpFHQxu-CYyAdwrJl3a^vG2Gh>r zIl{(o<>ug`4N%?E#YvpPQ%Ha{UT%=ri#9rx6!zdwD9%a_@N)jc2eoJUvh?!#f5W#P zgvMqPR7qcx_Ij+MW!WL@eU3cDEAU=SG?&`PM)_Kxe6BM z13lwau?g!Yu*3B3LRx`rDl~4NXvU^_@58dCyEnw$AWd~_8H-Xi!j|S}e~nXtEj@U~ zmg7#^lL9!eX|D4svK-b0*=h^|d7Y*ZW{^ie5-Lw#8L?^_5e~k1D55n-v6?30Al!9C zIqs@8QOqZL4G?77)E1f)G3vwk_>>vQ2qp!_WC9<0pJ(Cpp52!rO0a@?M z7`IikGV}woXO9m4kFSMWA&1LR(F$0F#>^HgIP`VsWQ7_d1+iyE6LA=LT$M^^DCU%( zLU6xLh+pJFJ93h}5l!we2sb!!+7i+RvO~lsqoOvDLuZhg9D9nfe>R3I1DT3g8OXz* z7WD`LrBy0pe#pO%TSXn+0p^>KRJ}lD02kFSHCu;JJ!~)usWSrOz{!fKhTQU-_7Itm ztwfm zIfJ3II=LR1^oGigKvQ3`Rf>qfk&{+#5S_AQu?KH)Gf0Y1NQ-UrTp(2?p^&}hGtq=X zuGgSRW{`SOe<$4J|!4d@T%p-`g(G6YzP>KnAE8>GJ z_RNptx=OX}F(P*K2ydTJ2N~*jQ5uM$Jl3Y-Ug_h*NQi?Vuh|cL8`M(2I|-6`-J5PU z4H;X#YH;TY>{Ov@10V|uAV92g5G2CoNX=#2QDTJCf8}}$BTzrpKR1vAnjjH!a_ub2 zW>^~Ng65>T@x|NyG18n8^Ng<^VlGDQ#jpATAdR0>ZH0Qhx?@dWmpTg4=m&( zV;EuQfB&l(&29~y)GT(`w3sH?K`|hPd~k9OOIS96w-^r{IwgI@ZjtLt2LM!dJNh{@ z*+FuR@;q!Ic9odvlxfw(B!qbX>nA5u?Pu~8PC^bn>`22{mq_1~!}50?XMYJlbn~Lv zgY_6a^Dwmv)0z6*S06rah0}!i$Z$>brGO<`e;64j98*Q z>dNjAy8~klytN&K=T{QK0&)PTQl32$n}0*qcLDs@Nu9kI88~2iPgc7vLU-Vi7Okea zJS5y!2*r6|O=}f2pJ8@?ToX5M`Q9%Tw_Ncr<`$}aMKZ0D8Z`gUKJc|G>ZEIsnxfp$ ze^4We3GmW7rnIE0`^*4&QireW?INl*(5e!XiME9__ap)*YGZLR4@5p59oydzLdeIm0B4$e z1l*2nPDx9k+`!{vLJ|e}8L_VM8ByhWfB%|Sz)(sDpR$kG*r}63vQxl5BF}>jx|J?6%J z`5k)j9HDHDfC+Sm0-{+#-{7%r!{M;l@9l}CtdLqji`b?f-n+9xz}sHdbu9vOHm@kOFDnZV00+BGCh2C4 znHIko3BKx;NHoJp2ZP;o9;P8GXLJkFB?4<)6l7Snq-d+vhrNQq)k> z)NoZMKeO9{qk9J2JV-2V0sPAdmjSZPKyg^uP#6b5(YHtagITk@hSu}NJNI4?GOemVP0_p{HY+p{#aOy-2k=!-aO=TiLK=F3_mC9U z#r4XI-{Az**D$}rFi#$mmqwYUekH<9iQ-JE0XJ?J{eN7SNxF<>MwP(fqX~K#SFIII z+b>3HF|AJs>R{GtA&k>Q7yx}jvck2;K*7J;A&kv&qE1Ld@ zkpn;f5}fekI4IDU`PZC62nJ=jyg{!+E2MnVE|@S}=sDHXT;a<7(3Vw^uDp?zRDzmd zS0VvFHbRUfn^$l(e-VA{E7N948{DXxUGN!rbD^6_P`19s+K+j8rY9rN3KP8{811V> zXPIzn-jvm97J28E0FeCxFo<+X0NnD0S0)4|UEp6cAKUCmwkPYavHh{0vUlbZ`Gt8G zSin#Mb1SP}NEeRFg^|QEK$p?kFhgjmoOmas%2<|8FOyWge-SW9XT206EXcbIsb9z7sl5TMm*=& zmw+$RepT6We<*^PBi%#XzapvQj&JU3(CL{_h$5XNAuEj-i{;83pj26|&izoW5|dH= z_DeuA1QcQ&>`#bg1v!Uh3;PfZ+h?$c4M=8+_@TTfisd8mN=rt+^9X0p_q-wzuId%U zKEH1wHDO%cuSAqk^& z>N#n~e>WOgXq4dxU##+_l$)d?CBuf(uX*gOW4Mo0=$$X89vs%=ehy(e6Gg2Wk1*t| zAyJ%piK5q93y?3kQzD>{!#JbwCd_X9uh&ByC{6PDZg$U&qI-1E9Dbtrsb)B;tBCD* z3-QC|Vpw|5z)=FXxf`fYjO>EQUHPRtc*846F^3N${Z=)1mh>Z`8Qd}H$vZ-{0#i~> RiPhR<^1aZ(La<%BJOQ|ujST<* diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 2154c9cb7f..472856bffb 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -144,6 +144,7 @@ fn thread_sources_round_trip_as_scalar_labels() { for (source, label) in [ (ThreadSource::User, "user"), (ThreadSource::Subagent, "subagent"), + (ThreadSource::GuardianReview, "guardian_review"), ( ThreadSource::Feature("automation".to_string()), "automation", diff --git a/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs b/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs index b59078c67f..d538c8b7e1 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/thread_data.rs @@ -101,6 +101,7 @@ impl From for CoreThreadHistoryMode { pub enum ThreadSource { User, Subagent, + GuardianReview, Feature(String), MemoryConsolidation, } @@ -135,6 +136,7 @@ impl From for ThreadSource { match value { CoreThreadSource::User => ThreadSource::User, CoreThreadSource::Subagent => ThreadSource::Subagent, + CoreThreadSource::GuardianReview => ThreadSource::GuardianReview, CoreThreadSource::Feature(feature) => ThreadSource::Feature(feature), CoreThreadSource::MemoryConsolidation => ThreadSource::MemoryConsolidation, } @@ -146,6 +148,7 @@ impl From for CoreThreadSource { match value { ThreadSource::User => CoreThreadSource::User, ThreadSource::Subagent => CoreThreadSource::Subagent, + ThreadSource::GuardianReview => CoreThreadSource::GuardianReview, ThreadSource::Feature(feature) => CoreThreadSource::Feature(feature), ThreadSource::MemoryConsolidation => CoreThreadSource::MemoryConsolidation, } diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 0b4ff1ac46..70c9fda438 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -80,7 +80,8 @@ pub(crate) async fn run_codex_thread_interactive( warnings: Vec::new(), }; let session_source = SessionSource::SubAgent(subagent_source.clone()); - let extensions = if crate::guardian::is_guardian_reviewer_source(&session_source) { + let is_guardian_reviewer = crate::guardian::is_guardian_reviewer_source(&session_source); + let extensions = if is_guardian_reviewer { codex_extension_api::empty_extension_registry() } else { Arc::clone(&parent_session.services.extensions) @@ -107,7 +108,11 @@ pub(crate) async fn run_codex_thread_interactive( session_source, forked_from_thread_id, parent_thread_id: Some(parent_session.thread_id), - thread_source: Some(ThreadSource::Subagent), + thread_source: Some(if is_guardian_reviewer { + ThreadSource::GuardianReview + } else { + ThreadSource::Subagent + }), originator: parent_ctx.originator.clone(), agent_control: parent_session.services.agent_control.clone(), dynamic_tools: Vec::new(), diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index 2de63f4fde..f79ff413fb 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -229,12 +229,13 @@ async fn guardian_delegates_do_not_inherit_parent_extensions() { .services .extensions = Arc::new(extensions.build()); - for (subagent_source, expected_thread_starts) in [ + for (subagent_source, expected_thread_starts, expected_thread_source) in [ ( SubAgentSource::Other(crate::guardian::GUARDIAN_REVIEWER_NAME.to_string()), 0, + ThreadSource::GuardianReview, ), - (SubAgentSource::Review, 1), + (SubAgentSource::Review, 1, ThreadSource::Subagent), ] { let mut config = parent_ctx.config.as_ref().clone(); config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); @@ -263,6 +264,10 @@ async fn guardian_delegates_do_not_inherit_parent_extensions() { expected_thread_starts ); assert_eq!(thread_starts.load(Ordering::SeqCst), expected_thread_starts); + assert_eq!( + session.thread_config_snapshot().await.thread_source, + Some(expected_thread_source) + ); io.shutdown_and_wait() .await .expect("delegate session should shut down"); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 583d7451ed..78520c375d 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1323,7 +1323,7 @@ impl Session { ThreadHistoryMode::Paginated ) && matches!( session_configuration.thread_source.as_ref(), - Some(ThreadSource::Subagent) + Some(ThreadSource::Subagent | ThreadSource::GuardianReview) ), ) }; @@ -4230,6 +4230,7 @@ pub(crate) fn emit_subagent_session_started( client_version, model: thread_config.model, ephemeral: thread_config.ephemeral, + thread_source: thread_config.thread_source, subagent_source, created_at, }); diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index b5441ca3de..5c27987a31 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -732,7 +732,7 @@ impl Session { ThreadHistoryMode::Paginated ) && matches!( session_configuration.thread_source.as_ref(), - Some(ThreadSource::Subagent) + Some(ThreadSource::Subagent | ThreadSource::GuardianReview) ); if let InitialHistory::Forked(items) = &mut initial_history { Self::assign_missing_rollout_response_item_ids(items); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index cadbb868ce..b977ccb9ef 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4954,6 +4954,7 @@ async fn emit_subagent_session_started_includes_fork_lineage_and_originator() { let child_thread_id = ThreadId::new(); let mut session_configuration = make_session_configuration_for_tests().await; session_configuration.forked_from_thread_id = Some(forked_from_thread_id); + session_configuration.thread_source = Some(ThreadSource::GuardianReview); emit_subagent_session_started( &analytics_events_client, @@ -4965,13 +4966,7 @@ async fn emit_subagent_session_started_includes_fork_lineage_and_originator() { child_thread_id, Some(parent_thread_id), session_configuration.thread_config_snapshot(Vec::new()), - SubAgentSource::ThreadSpawn { - parent_thread_id, - depth: 1, - agent_path: None, - agent_nickname: None, - agent_role: None, - }, + SubAgentSource::Other(crate::guardian::GUARDIAN_REVIEWER_NAME.to_string()), ); let event = timeout(Duration::from_secs(1), async { @@ -4995,6 +4990,7 @@ async fn emit_subagent_session_started_includes_fork_lineage_and_originator() { .await .expect("subagent initialization analytics should be emitted"); + assert_eq!(event["event_params"]["thread_source"], "guardian_review"); assert_eq!( event["event_params"]["parent_thread_id"], parent_thread_id.to_string() diff --git a/codex-rs/core/src/turn_metadata.rs b/codex-rs/core/src/turn_metadata.rs index ec652ee520..8be5c1705f 100644 --- a/codex-rs/core/src/turn_metadata.rs +++ b/codex-rs/core/src/turn_metadata.rs @@ -292,7 +292,11 @@ impl TurnMetadataState { } match &self.thread_source { // Desktop create/fork/send lacks trusted app-server provenance; fail closed. - Some(ThreadSource::Subagent | ThreadSource::MemoryConsolidation) => false, + Some( + ThreadSource::Subagent + | ThreadSource::GuardianReview + | ThreadSource::MemoryConsolidation, + ) => false, Some(ThreadSource::Feature(feature)) => { !matches!(feature.as_str(), "system" | "title") && !feature.starts_with("ambient") } diff --git a/codex-rs/core/tests/suite/guardian_review.rs b/codex-rs/core/tests/suite/guardian_review.rs index 0398d2cd9d..6e6cbf244e 100644 --- a/codex-rs/core/tests/suite/guardian_review.rs +++ b/codex-rs/core/tests/suite/guardian_review.rs @@ -42,6 +42,7 @@ use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::ThreadSettingsOverrides; +use codex_protocol::protocol::ThreadSource; use codex_protocol::protocol::TurnAbortReason; use codex_protocol::user_input::UserInput; use core_test_support::fs_wait; @@ -398,6 +399,14 @@ async fn guardian_session_prewarms_and_is_reused_for_first_review( Some("guardian") ); assert_eq!(guardian_review["model"].as_str(), Some(expected_model)); + for request in [guardian_prewarm, &guardian_review] { + let metadata: serde_json::Value = serde_json::from_str( + request["client_metadata"]["x-codex-turn-metadata"] + .as_str() + .expect("guardian turn metadata"), + )?; + assert_eq!(metadata["thread_source"], "guardian_review"); + } assert_eq!( guardian_review["client_metadata"]["thread_id"].as_str(), Some(guardian_thread_id) @@ -442,10 +451,18 @@ async fn guardian_session_prewarms_and_is_reused_for_first_review( .await .expect("guardian trunk rollout path"); test.codex.shutdown_and_wait().await?; - let guardian_context_windows = fs::read_to_string(guardian_rollout_path)? + let guardian_rollout = fs::read_to_string(guardian_rollout_path)? .lines() .map(serde_json::from_str::) - .collect::>>()? + .collect::>>()?; + assert_eq!( + guardian_rollout.iter().find_map(|line| match &line.item { + RolloutItem::SessionMeta(meta) => meta.meta.thread_source.as_ref(), + _ => None, + }), + Some(&ThreadSource::GuardianReview) + ); + let guardian_context_windows = guardian_rollout .into_iter() .filter_map(|line| match line.item { RolloutItem::EventMsg(EventMsg::TurnStarted(event)) => Some(event.model_context_window), diff --git a/codex-rs/core/tests/suite/subagent_notifications.rs b/codex-rs/core/tests/suite/subagent_notifications.rs index ee6697e5e0..484cf3e079 100644 --- a/codex-rs/core/tests/suite/subagent_notifications.rs +++ b/codex-rs/core/tests/suite/subagent_notifications.rs @@ -977,6 +977,12 @@ async fn spawned_child_receives_forked_parent_context( let child_request = wait_for_request_with_model(&child_request_log, REQUESTED_MODEL).await?; assert!(child_request.body_contains_text(TURN_0_FORK_PROMPT)); let child_body = child_request.body_json(); + let child_metadata: serde_json::Value = serde_json::from_str( + child_body["client_metadata"]["x-codex-turn-metadata"] + .as_str() + .expect("child turn metadata"), + )?; + assert_eq!(child_metadata["thread_source"], "subagent"); let original_parent_turn_id = parent_body["client_metadata"]["turn_id"] .as_str() .expect("legacy spawn parent turn id"); diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index e88eca7d41..cd20f787c4 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -2604,6 +2604,7 @@ pub enum SessionSource { pub enum ThreadSource { User, Subagent, + GuardianReview, Feature(String), MemoryConsolidation, } @@ -2613,6 +2614,7 @@ impl ThreadSource { match self { ThreadSource::User => "user", ThreadSource::Subagent => "subagent", + ThreadSource::GuardianReview => "guardian_review", ThreadSource::Feature(feature) => feature, ThreadSource::MemoryConsolidation => "memory_consolidation", } @@ -2646,6 +2648,7 @@ impl FromStr for ThreadSource { match value { "user" => Ok(ThreadSource::User), "subagent" => Ok(ThreadSource::Subagent), + "guardian_review" => Ok(ThreadSource::GuardianReview), "memory_consolidation" => Ok(ThreadSource::MemoryConsolidation), other => Ok(ThreadSource::Feature(other.to_string())), }