9p+|B
zs9VbaMRW3n?!>uyy>*hgYN>V*Gtta(h~M3W5C5K?wNYZJS5~*)L>&*U_056`fAM2s
zuw2|Tm(OitrhE2c=xyKni~{$8dSk23mRBF+5vldY)Vw4wcArG8Qw37r&*fFW)KS$L12QLw6D
z8t*H-Esij2R=&4#L3-i3)^tlnC){{^Gza3TKSRPUcAKz_d-1+(N8(DTkZ`8u%I!*R
zl8`bP)5V{C-Ld<&pC}{GC%_c&mwT_JFjuH+7OTd)61&$}FI#aDwNt9ccdtzZjP>7|
z`^uoW!fi_k2?S4qg%B)Qa0mf{LkOsXAYu^U2<8t-bbc&X2_a_BrJ|pEy9MYTZc^LfuI2kHloL
zXSfi8rYI>6*yC1W^v4L^tt7oSv-?&m4hZbDX^E(hA;)(lXtw;Nx(vc-1;?>tC-1X^bwET2rz5QXbB%s4pXdv`R0lD^!^3#0
zBCIotwe*-2sVs~v{AcSPz_#%3${3@YZoh`Of{g)G0tY4Ls%;IB%O#_({zrd-dYHrg
zS22M490%ZWpUvPS!@457V^Xj#Lo7;V~3=lLz&=+1fJ@;fUx?N@2^(S|js58FA2SHtQnkmpT54
zg0g@yEa~UrMdc|Sb*ChsLHpw5IZ!!?vWUL?A5Lao_Pq%FhK6C_Jj{J|q^_yYZPbQx
zlZ=WY+I$TkWQX4DGT&T=KS-p(_wx6ZkEupF6oi}(YGw@F7hRRDQA8iTk(rp%G*n2U
zViv^;`TE`~>TF5rsm?%RK;+$3w?02)X1qM;DUq_{9
z+}4!&vNAGa{>DZ5N=~asyG(1$qSz%>zXJ(OBVTq0iVwDVXbowNY0X$Dy2R=s?zn)i
zzimo-PT|mj*cqq=*q95p20jNmt=R^gPTQn{Zn`6KRvJarFyL{dX=HcOho`(~qF*!o
zU6YRzTh7LVH|qo#>V2<+lLuN!u2EZcCHZ8{w8EbC=RKbL(pxDiLUmC|th$~||7Cp*
z|50dv9G%?ua*fSQrTy1ixI@0rF)A5icz$-?p(;~+;5KmV=zc#N*Fg@H;Cr-H1hP+u
zAK(sp=ZcQHQ)$yU-rk+Uw2OZfcH=T&jyj*Lb&rfFPP5wI|Fyh8k~uW9Cp3{{-Jyey
zLvSnK#PbLM|E#u4Yr$COr5^>RSLo5AGqd--g{d{@^j4&P%X{BW!K~k#(dHq-LddzIF3eg5kUg^
z#JsJK_P6r#@&u3D&~{I6|E4?SgEUKY=nz1y!QCeC=?}KIpQcBT9)K|elM`-lMXvtB
z=RqR}hI2~96>mytyO;08DxCrlsN@T-!{^tKWWTRFP@Idi?M*WC80*e1^pP{ORdlMy
zhJM#(pLQ&DoCwK=>N}X4@)!r^RaIZwR10+O_>)>KeeQFgwVZ9F@Y##dl{wFzqx$xv
z@M|xU%QZ$hh8wZq3@Zc)-yL_pr}ew20#{hBOvK%vbG5`cWfMi&!&MpPGvKuosNT}8
zQpqh{(|qtlzn}h!MG5^S6eo
z%dK~}bxftUy=Y9dNd9}C%Wjv}GK#zF-4MSE3i>L6>Mq=J#Y~n{HQhi`?7nD?6dO~J?RP#3IVM>OE6AfU!PHh
z@k-mb{{GL+l>x28;`lPRHY~MWHw6)35YjWCxE2!yMMz5LQ%ZM|7B(;GqQ1WV
z727kEQ4A`^+l_c7t)Woy<7ZCj-H<|{EGAs~H+gc|&vkgZJoouYX*rqWE`(ob8T7
zC`;`Yr|orM{w8-C
zR#>T1<;wV?)(pj8IyAjo$ZcQi#{C+;2`nZ$J3ZGabzo+ki6GZSYHMqE<6T`{Tdx16
zhXCrozWmzrp|DVc@oWI-zuV!HVd;+-@14}&>C`^`W;b*Y^S89mO3zSFkNLjFt-V$q
z*lkg$WY^3c<5ec0an^EVFu_xKU*8r+($tM;p$jc1r%F}f`A5UH9*yRP@%B|TQfXDq
z*k?^~X=uKpb)Euvf+d2{)_^uLcWz#Th(!DkDp1rs(QCIBuOvL%>{(^C1?(h-=c*iGmA6n;paiq1iObsbh#Pr~_t;s(()`20E*8%~f5Q|%r_9_Te`TxMv{>uB
zvytyte)5eb{Oxe6ytPI&lZ&$J|7M^Q5v9&eb_ly}Y%?))
zGnooovo`8epiuqo8eXH|FcPKhN5Ye#9NmKM{s{t%7LB&f?8oM@j{4qN6>{92R+^gY
zsNGFe*RLK6z~uF^-GlML!XYivx7@RduvDSDJ};}IfSTQF3_<7jxx!nkA>D=fdZ*yD
z+6L8_$Du{$up2SiyErqAk)UCo>z)3td=0Z*w)R
zck(tRKDFlA^cj3vvJl~!xdKhbriNXn8p3m}vY%`gp9En0YE=stMKtn;+>Fr$at+9d
zW>ZMUKpGPn#bVKq(tN-qqJaHrUHPXVrF4OMI=UPgIT`u}6Rl~TjPHZnv|wdg?Jrj@
z)Zh6@J|0Y9q*a;gx~iUbX*q57)QGx{{+Se?>-U((O3>5fE-6t;9?)nhB%U4!4iO>u
zki6=9jN(4V;OC#~CFvw6|M`BS~qH#~-xD6q-R*MlAP6P`>{NLMsmkDAd-%@8<
zSst^zlao9N*;m3Z2I1$ljre=NjZ|mLfAsp>c8Ry3e#P}&PmZs&F#YCUZ{8p06rUm|
z?7&B)y()Q$<_;o@FhjF=R75-TU3-FXQ7vNueO?RkBWSDC(|jAV>
zA};v3=26D5_QdK2dK=wYEVvmP*7w>WX=#4!r753tbEoUTPywf_#%@4ORh0{PX4c}a
z+5qnq>?Ip3(OJMGpUyP|+FcE;jD+dahY_DvruYgG7ZYuzF(uzm>Qu#q;n&qbx37)7
zc3(1+Wj;|Vtb*`w387Gph0bsJQv%@xLRVRNbZLp=&FnDmviyCfpl4iAuxVMEsmB(R
z9Z6~)e%VTue;P$>;es@m$A%a&Nb&;nbw4!$gMvt9YxR_z!IMo>_0#_T=cAM{P9?rK
z5>A405bh&d8U59<-S%mX!Sj_JR)KD*fSRPRjbe4#=Ua*P*xIvbEbr&>%4PK(uUxUp$Y5`h2AznZ}*@`8%B*#0^pc{-Yy;)0LOAtx%9GQ@>(tqifF`-E&Q)Yg@;}G
z6Iw^(1BLyg;GuM1l-Iu-h}QlMSO33MM*cs$)7ME{8br2lh6=yp-NU4JfNDYt|yukXWjIV2P
zjULUtyNkABc447-d5=%I**cwCVkY*yl5Xhx{1>+*m!(*mP6pXR$f;Z9>^L<7Q^$+n
zq#&V&?({X^SlI3^(VOQP+(N%hbIh}UmK6o2nV90bSSG)6;S$p
zV%+XKo{cNwyDtoxS~C=E`yOtNh%IG{9j6Vsd4pzfcxh0sQPf%X>NM9n3)yzl^OGoz
zQpjq}rLbKfy~%6Fd0U
zQ%b9Zx2(4^Y`s*7wO4CMzB8VYO);kUY)7spz(BCWDPPVyIdA)mo!wgpH>B!>tAxES
zor9S|vcv?^wio>s-Z4VkK%${30gHMQn!~Dl`q1QtBdW(IEnGh5-!urOI
zvjAPiO-6fxGWT!?EAC2Xpe|u&fmlB0(Wt|ZqOZK@P5B`*IUn8?tc@O=mm|@d+@*n5T4w
z_BMy_YOm@Wd}m+BOuDqc?oQeC3oM=u(@l9jg-%(AJO5~dT|>Y9?(wuoRuW0!t)y<1
zy;(>^_HT?=sja_V43Mfet|OaVB{iSlkEUc)P!xOHp`x6Tt=+0)&KB9))$G5sT#}h^
zq(@6!fI9!SaMldI^2iJK+*pJ}$eUV$W@0m>8qK}muuD6K+uo{|1Flv3Mcjl|cY5Ni
zD!-W;#QID)3v@+jW5)jJJMzZQ8dD)d3YH%W#ZAj}EvEEL!{z10euu1I&8<_g}>4`U&jh#2XxM
zAIK))=hq#42C}2cIi;ajtNpSqR;Wq%Thr6CLA>hCF06}c0C`{#_yS7i6(-eY+`T=P
zYccZEbC`Rkd}*c3{_IK;r{LZ)kdEgB(J!#bg3VB*PguO%=%~pYj)ok-0;SNZO#x0M
z%$;<5W2E+7nj8<-Zk&E4w5+cymfzdIIrM6Z7pwEUS`9gBy|th7SdZj2@YXYi4UeeO)zENuoCZ=E$|y_JAP>d>x$;BNu>Ie
z*@j@#{7xb|$Bh)2IT%O^X6ywrTHLST$u^_GBz+x-=<
z(8l~S^5P08jLhIqOii7go6|8gR1#1E3nThyqDMzQ=Sm`Wlt6P0&J26RhW9r!_~iU&
z`&F>C3s@@G*6s2=ves_??bh0w@tn9s)pi2|^E$Eu#0@Y}-7t{?5%55hIOr^tu@0Z~
ztY$(#*+5{rzF-qANt$+=>8M01T}zC~j*xBe=#S24#zp=O;ymx9ilH|S3Qr23)C~>l
zCJaAXW_uOfs^TZ5`IcALarN^7j~x4uMM7G*a^pz5bbrFfafa`OGo_KbsZ3l6I)zwJ
z(mbqROT-Pb!CWk(o)S(OpC};0AP88t@DCj&5sW=wuKP&5!atx))asXIB)xbKfd1ET^2@@<715uYzY$M?DhG0Xmr|aEVoEIb5$-jPv%NC_a@u9_`r50zbv^oiVmw=I_Auj
z8)q$czCzgugg+%BjTJkCXMY+wqe1v1(kV_#9DKuvmPGDW@%i}plG*gVZgvVF2$Ac4
zrN1z+l0g4(3igJxOqF8beWyB72kEKxx3f02VyirCa1BnDC@TCYa!yy^GOCB_&
zBZ~y~%%&@<&o@LFc=_!nOvLBR7uO>H#vA%vZ*sYBvM;p2c-ko`Q*Ot{?&t2?Ls;6S
zSyt)IzL0N(O4qceUYN2OXmzgbg=F0&rR}QGO20KJzVKRAT}0LubHx&mdm5ZrSJVwJvQwcO2>hD#%I*{
z)d*|iTXA6j6Jay=FIUN5?bWuJDRr)~=&SN`OBy3Q@o4$DSyK%d%aZ0I$(SE4!Iy46
zgwt}qmBA#UXixssgZ|f|_VHa_4qPARethrT8=EFOJfqpSCx421&f*=gbZWAoB(x?Y
z!<{L6kuoPI*wBhHCeve2NSapZ%A{Y-xc%Hg#2Rg%YHxC#m3Bhp+0w8zFOCmnL0h!-
zps#fin~$SWwKZ4@WXCdG(a>QmAYFz($59GAN|Ez&HBosNvUDYDG``_D46hymuZ
z7?ox3qn2Hs0~icu(b8V)ZO%4ujzn}VfSN4mT(3cJP$
z_sw<4vZQu!w=36cUPRy9)7?;;Iv*q+P}hW?gB2M`4mJvQ;M_+JHHC4d$$6!g>P6J<
zRi$K`g!#6r2XYp1lwFf3y-~F8du+tSgB&rw&1$S5T+nWJnYW0?H>SIl
z5`ZQQEH9<%*@n8K9tQuUy+6jkKL#OT_n-CY3Y5U~HRL?Df4E?R;uG{6=lL1$t*#H7
zjw-jcYpjVv;f=0)k`XUY%Iz5nYe}M)U7k8Y&AAn(Ws?2dc>+S*6q4{bpTq^2yc18$3
zCGzMV=^#5Tw5pL!s(5pWygd?e-JOKq4MNYR)e5}7eGN>Z_I*#VM1!)>aeoK9ANt96
z3c5eKSLl%SvvabDSK&G_`S~!h3P3Qh$o+X4$QPC=RBZ8Dz->R2A(P6vbxULyQ*7ee
z=oX>`!A<%jS}!*AgQ^ncD-rY_apcf)KAn(|aJdW;5fVa3iQJ6kMhnZ{f_>oq7Z+}G
zd|qeUXc&0S=hM2yx;`h}Bxo1|uUzl091BcM*Q=I{mPUBx$uO}W*bs%jhP#J>CDzKc
z@;WT-pI%E_tE#G6(N}Lxb2ezV0uQ^N5J9N^cyH1Z;dwr#sqMaVm5~0YP7j0#0s9#9
zzqjstc5_)Q9b@_6j@}|FCDr?|iDZk-LlFlK-rWT!032-qunMM=6W*(8BpN%;B-J=jLjj+Dl2Y-~^>k3*O>O9A*DANG^a
z8F_bY#PMfoX=!@;B9{{Jmo=%cD<*&tW#n9hnOzW1p#3*
zqU`=0Ixsl+FgZnjN-hun8y0YX4P&9Db*ZXKb0;Qom~boBS8lQ<3+HQQk}v5GsG)7E
zQJQ%NBsQ#TR*uV#z2vE>se98Uw-aEbnzHgSu;jpm@42~$Q#f<8v%jqOL;@4^oN&GWX6*hUuSWg}nVwifSuoD>pH7CB*h6Y$#bG0BC#*py_XHZjM(wtA$*)
zN6UNX3s2~yzOsXmf*16G!Rc>-9tx#&dnyHgK0M}ebnMb`)yf#$Y;
ze`kTF7utCCxl}MrL+H=7((o5{>sxUzaNU!%czWU2<036a2VtQWeW@=}k*d~^9su9W
zfp-9u8@*#yeZSI2C;6mv;G5@>_8s{idBDIi)!I_2!t&P3w9o|@Y@v27_IcxPvm-=a)Cc0GoSec0G7Lak_-cjUt&QF!l*FJfBzq#G%V
zd;4+^zh=`C5GcC#|64aoz8+#)hvi=I)(Pfb74zc(%WT$`#E6Jlv8)!jFDxLov>O*P
z1FUG0JpAykfIQ7@JWpmYnSHtZOJ}KmQy-@jOz3iSXoIzGq0(w1ACrJy5Z*`Q07YE1
z`rdYIelqTZ7Oisy46xmnUA+CWu9|4gubT5WgS|3j(`2r|eOwUYqIjz4q+fRYy?7ve
zft+n7(222@zk2F=kWU~Sz%Xl+>PBMkByt)bV9Yl3hq9e3N5uIIyLRP9l)&K?3^*UN
z#Y2|7_DZU&va?q^g0Ls@6$F5#0!8jY-CQNM)qyS)equj^bd3iR7~kLLqo=-!=4p`a
z8=`ei2TlQwxMv-#VtDp&OAv6>nL5fDq5|sS@;K24fRpM)`6ImlQlctCm~NqU9IgUh
zANyxh1nuZk%=knv7C0Pu{HwLM3$(VIo035;71Jlvq6MfZ6;!Rn?&XkfRl>P?SL~7|
z9u3=tsew`yjzv@y^tFAaoj->2ajE!#Cd$8$v~Mi}TG^v(pyx4ZToRYt@K$xnzOeAu
znZRqj=@|@$g2(cheX{x=W@+p&_Ufz~_C{1M=JqR%(Odp8E6UfeL8AoAR;)A*sUH<3hCyBBrZS)8BK
z!A@z`?oTaexYN_xL`1lZ0jx3niRL{eEvPHU&ET2pf<6J@H_a$C<0ElHJWy
zHK4!UAFTQFBp_k8ZwgSG2w@sII)9(i-<_XpH;&lPen#xhUuS!8qUpq(V!Hx=bJ(`h
z)6`2~N-?ev711O0S#nA~dydQHAy7Z2ua|Nt!cV=dYB-EtevXBT;{WYCAehKYCnBTKWr;$yd`Dxxfk;T}wj!m61BtFv6>=X(T@AIS3K
zSM_SX-CZkN(4kC-+%N*t)UXor>s2VKJUh#VMU2l(wMQkxwrc#CLfI>bsr?KVACPog
ztJdrUL9C~PN$>#tnL&)jKB~Mioc3UYNlI}W-oM5!%)G@*zsPSq`|%y9yZweZ$#_Io
zfGt9cEqq7NE@Y-UnS?2E;$ziMynzFefj&Krd=kWO@$~ue1srz#>HEe|zR|`#mv4G+
z-KSm(F`q>+t}XdMamkr4xiik`
z*4P%_I}t8`qHfTsmw});
zaV`xdi9%HKeZ5D|F5Xr=yV!kuRt**i-0F=^84>=Fb@1xF=gU1)MCu2fg`J@5yg4%W
z8YY3eM8qf0TcI5q!S?)Ic#y-QdST?ZwcgG`+zxRD6LlKNXx66*}O(!Rw7=Am76H>AZU!fqk)w`l={_a){orEzXB5s?`>uF`8T
zAKF;3HDi#9Y|%pXZe#)W+jL+Js*p!&YwOHdZ>I*uYa^zuHH!b*Pk1{`D6rydg2NCr
z5sQ{XBg)AxCecT{8uMPMEUcWm1N2>?2?}jw(`q!EZ14g4TG-yspJlJ=e$^EdKYm8v
zk32TN?YSf0GO9Y;Af`D@Ks7ZjPO|3qb!-g2o#%nyUtqk%UX*&(V>4&}x_cnYp4ZR=
z!OeS740(SmMo=2iGvDz=@Y3VgQ(-2nOA1n+qzs{v`&Vl3;-3IcQx#4r3yfw~{o`&H
z^GTs9?|6Kb>{nlMM;o=pU75Oeo<{FoK6Z~Bn?Dbg*VCtKpTJAfs7+SaRI3idmd&mF
z*ihe*xW)`QdGkH!f{=t_QTph6f$PUTW_?ROsg1eDqFRU`7@8DrydzvzR!Tcda+sz+
zu6tVA3Nd+1bL$pHFfClQ1^5Ljdj%vC7XdO^=b_s@LT0a-_)PlezFKbqJJM%Pc#CiE=#uE~D0iUx`
zU7DL95t8p)xnEO9NGpr$IrKpyH_SH8o7jU4xb^D
zNtivh;Wz%A4S5BHuP?c#i-!AI;^qdtA^3*67)n6=2u~%aHC2(2;7=D6QhON_11_4{WGyrt;7OC}fyC)$=Ftu+%0s|vX4b@{nHx*r>z
zhe&_PrtU&itW_uN#%0(N#5M_KU$RhSPbokB{Lp!ZIn6r8%av_a`Pi6*jRi)m5x(>j
zSCi8Fr;SxMM6y-WjNpUsr`q0iYAX$byjL({3KdQ
zQ^)-PTwdI)$Fr)N@`Z51#-=>U#=eLz)=qPFiI0^#OffA4WJ*MsDj}?rL&+3?HV;c4
zmNRE}R|LDSUmsY=m~Nuk2$C}BP{4#2f=fC0F5O4th;&An3f*eOgM3kbGt)hbIr6a-5sLn4_RPk$UJU7*q9r0QN`vR~3W2G$uOX
zDLjJ}b2U*!m4G^-xOaZ#LFc3G4YTK|J^6ZEtaMGeHuab){{M>XM=dN2Sv2}xscQ{k!5(@`+PRIXvXY|hg
zPgn5SBcWO*Zt*#W?YK7;XHa$0LLXWi_DiE-xmS;-GB7-on6P-PewNk8b)pTv~$@xO%r40>$t5}=QYNqZ-oPgmyaxw9ey+OO+NJy5^nr~V?-2KG
zi@grQ?~)=cGuM>oW!%V@y{`AIdpy?OAxRVz$GKKK_I_9>nTEO8=^nNYEwuYbMBq$A
zIY4@kL_0(St5(h=BNUzmBQN9KkvBR}N_TW=QA>V~mu-I@f7#I_t~;ywbvg+U*!AE1
zwgEs8#XWG_|9`c4)MH(+krZlTNIie#%dW?cl7&wowJMpX4`4-jCxG{wbd#5t$T*u2
zW^ptIz$L9#r@aQ|F#S8|9ETX
z1PI%i6?^_muDF@ro<2utTtU=%EAxv8D?Sm|eU+9L2hBH{
z2Pa!~(w^{-h4quCMG%o_sEDw5s5XG|RZyd@ISNS5nI#w1a;3E;Jh}be%zJHHzH4@F
z3mYexiHi_szmSIxclLN>s;g9vDD~=u(L4tkM5#_RT#i~?7VIr9Prrb)wKn>jp2kZt
z7^dqbdF!S&uWxt&IT9!Bb7GaRVbx9MlT}s$OOt-9RC8M
z>}FMiDCu4;Vop4|v@zCbKyk#$K;61t8(l5HgV1ris9d|f_yhLJ@jdTChil7Gx`13R
zsv=T5O(B^gwuNv`892vqv0~
Date: Tue, 27 Oct 2026 01:16:43 +0800
Subject: [PATCH 15/31] test(skills): keep the diagnostics story typed against
the provider state
The gallery merges arbitrary context keys at runtime through its state shim, but the shared provider type only exposes ExtensionState fields, so passing skills/skillDiagnostics as initialState failed the webview type-check and the memoized webview build. Type the story state explicitly as the provider initialState with a comment explaining the gallery-only fields.
---
webview-ui/playwright/gallery/stories.tsx | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/webview-ui/playwright/gallery/stories.tsx b/webview-ui/playwright/gallery/stories.tsx
index c44f2e2c66..70996c077f 100644
--- a/webview-ui/playwright/gallery/stories.tsx
+++ b/webview-ui/playwright/gallery/stories.tsx
@@ -469,8 +469,14 @@ export const stories: Record = {
message: "Missing required 'description' field",
},
]
+ // The gallery's ExtensionStateContext shim merges arbitrary keys, while the real
+ // provider's state type does not expose the skills diagnostics fields the settings
+ // panel reads; the cast keeps the story typed without changing production state.
+ const initialState = { cwd: "/workspace", skills, skillDiagnostics } as unknown as NonNullable<
+ React.ComponentProps["initialState"]
+ >
return (
-
+
From 691d79fcc716a1f013d114a41224502b20102e96 Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 03:35:37 +0800
Subject: [PATCH 16/31] fix(skills): publish through a symlinked SKILL.md and
remove only the empty new directory
Review follow-ups: updateSkillModes resolves the discovered path with fs.realpath before the temp+rename publish, so a symlinked SKILL.md is updated through the link instead of being replaced by a regular file (unresolvable paths fall back to the discovered path and surface the real write failure). createSkill failure cleanup no longer recursively removes the value mkdir(recursive) reports: it removes only this call own skill directory with rmdir while empty, so an ancestor (.roo / skills) can never take sibling skills with it. Tests cover the empty-only removal, a failed empty-only removal (original error surfaces), a pre-existing directory, and mkdir reporting an ancestor with a sibling skill preserved. SkillsManager.spec.ts 116 passed.
---
src/services/skills/SkillsManager.ts | 18 +++++--
.../skills/__tests__/SkillsManager.spec.ts | 48 +++++++++++++++++--
2 files changed, 60 insertions(+), 6 deletions(-)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index 895a06891d..44cdcad378 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -756,12 +756,15 @@ Add your skill instructions here.
`
const skillContent = matter.stringify(body, frontmatter, SKILL_YAML_DUMP_OPTIONS)
// Publish the SKILL.md atomically, and leave no half-created skill directory behind
- // when persistence fails.
+ // when persistence fails. mkdir(recursive) can report the first ancestor it created
+ // (.roo or skills), so a recursive removal of that value could delete unrelated
+ // skills: only this call's own skill directory is removed, and only while it is
+ // empty. Any created ancestor stays behind like the empty directory it is.
try {
await this.writeSkillFileAtomically(skillMdPath, skillContent)
} catch (error) {
if (createdDir) {
- await fs.rm(createdDir, { recursive: true, force: true }).catch(() => {})
+ await fs.rmdir(skillDir).catch(() => {})
}
throw error
}
@@ -1095,7 +1098,16 @@ Add your skill instructions here.
// values from being reflowed into folded block scalars, keeping the
// file stable (see issue #859).
const newContent = matter.stringify(body, frontmatter, SKILL_YAML_DUMP_OPTIONS)
- await this.writeSkillFileAtomically(skill.path, newContent)
+ // Resolve the link target first: publishing through the temp+rename must update the file
+ // the skill was discovered through, not replace a symlinked SKILL.md with a regular file.
+ let targetPath = skill.path
+ try {
+ targetPath = await fs.realpath(skill.path)
+ } catch {
+ // Unresolvable path (already missing, or a platform without realpath): keep the
+ // discovered path and let the write report the real failure.
+ }
+ await this.writeSkillFileAtomically(targetPath, newContent)
// Refresh skills list
await this.discoverSkills()
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index ff76bba98b..8c194871fc 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -2329,13 +2329,54 @@ Instructions`)
mockWriteFile.mockRejectedValueOnce(Object.assign(new Error("ENOSPC"), { code: "ENOSPC" }))
mockUnlink.mockResolvedValue(undefined)
mockRm.mockResolvedValue(undefined)
+ mockRmdir.mockResolvedValue(undefined)
await expect(skillsManager.createSkill("failing-skill", "global", "Description")).rejects.toThrow("ENOSPC")
- // The partially written temp file and the directory this call created are removed;
- // the original persistence error is what the caller sees.
+ // The partially written temp file is removed and the directory this call created is
+ // removed only while empty (rmdir, never a recursive rm that could take ancestors
+ // reported by mkdir(recursive) with unrelated skills); the original persistence
+ // error is what the caller sees.
expect(mockUnlink).toHaveBeenCalledWith(expect.stringMatching(/\.SKILL\.md\.[^/\\]+\.tmp$/))
- expect(mockRm).toHaveBeenCalledWith(createdDir, { recursive: true, force: true })
+ expect(mockRmdir).toHaveBeenCalledWith(p(GLOBAL_ROO_DIR, "skills", "failing-skill"))
+ expect(mockRm).not.toHaveBeenCalled()
+ })
+
+ it("leaves a non-empty skill directory alone when the empty-only removal fails", async () => {
+ mockDirectoryExists.mockResolvedValue(false)
+ mockRealpath.mockImplementation(async (p: string) => p)
+ mockReaddir.mockResolvedValue([])
+ mockFileExists.mockResolvedValue(false)
+ mockMkdir.mockResolvedValue(p(GLOBAL_ROO_DIR, "skills", "busy-skill"))
+ mockWriteFile.mockRejectedValueOnce(Object.assign(new Error("ENOSPC"), { code: "ENOSPC" }))
+ mockUnlink.mockResolvedValue(undefined)
+ mockRmdir.mockRejectedValueOnce(Object.assign(new Error("ENOTEMPTY"), { code: "ENOTEMPTY" }))
+
+ // A failed cleanup (the temp file survived, another skill appeared, ...) must not
+ // mask or replace the persistence failure.
+ await expect(skillsManager.createSkill("busy-skill", "global", "Description")).rejects.toThrow("ENOSPC")
+ expect(mockRmdir).toHaveBeenCalledWith(p(GLOBAL_ROO_DIR, "skills", "busy-skill"))
+ expect(mockRm).not.toHaveBeenCalled()
+ })
+
+ it("never recursively removes an ancestor reported by recursive mkdir", async () => {
+ mockDirectoryExists.mockResolvedValue(false)
+ mockRealpath.mockImplementation(async (p: string) => p)
+ mockReaddir.mockResolvedValue([])
+ mockFileExists.mockResolvedValue(false)
+ // mkdir(recursive) reports the first ancestor it created; removing that value
+ // recursively could take the sibling skill directories under it with it.
+ mockMkdir.mockResolvedValue(p(GLOBAL_ROO_DIR, "skills"))
+ mockWriteFile.mockRejectedValueOnce(Object.assign(new Error("ENOSPC"), { code: "ENOSPC" }))
+ mockUnlink.mockResolvedValue(undefined)
+ mockRm.mockResolvedValue(undefined)
+ mockRmdir.mockResolvedValue(undefined)
+
+ await expect(skillsManager.createSkill("isolated-skill", "global", "Description")).rejects.toThrow("ENOSPC")
+
+ // Only this skill's directory is considered, and only for an empty-only removal.
+ expect(mockRmdir).toHaveBeenCalledWith(p(GLOBAL_ROO_DIR, "skills", "isolated-skill"))
+ expect(mockRm).not.toHaveBeenCalled()
})
it("does not remove a pre-existing skill directory when persistence fails", async () => {
@@ -2352,6 +2393,7 @@ Instructions`)
await expect(skillsManager.createSkill("owned-skill", "global", "Description")).rejects.toThrow("EACCES")
expect(mockRm).not.toHaveBeenCalled()
+ expect(mockRmdir).not.toHaveBeenCalled()
expect(mockUnlink).toHaveBeenCalledWith(expect.stringMatching(/\.SKILL\.md\.[^/\\]+\.tmp$/))
})
From 7163cfd89421ab344c58d2f7c02a650032f0e12d Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 03:35:37 +0800
Subject: [PATCH 17/31] test(webview): type the skills initial state and keep
the visual test screenshot-only
Review follow-ups: the provider initial-state contract now types skills/skillDiagnostics and seeds both provider values from initialState (empty-array fallbacks), so the gallery story passes them without a double assertion. The behaviour assertions (diagnostic text, locations, valid skill row) already live in the SkillsSettings Vitest suite; the visual test now only mounts the deterministic story, checks bounded layout and pins the screenshot. SkillsSettings.spec.tsx 22 passed; the visual test passes with the unchanged baseline.
---
webview-ui/playwright/gallery/stories.tsx | 8 +-------
.../settings/__tests__/SkillsSettings.visual.tsx | 14 +++-----------
webview-ui/src/context/ExtensionStateContext.tsx | 10 ++++++++--
3 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/webview-ui/playwright/gallery/stories.tsx b/webview-ui/playwright/gallery/stories.tsx
index 70996c077f..c44f2e2c66 100644
--- a/webview-ui/playwright/gallery/stories.tsx
+++ b/webview-ui/playwright/gallery/stories.tsx
@@ -469,14 +469,8 @@ export const stories: Record
= {
message: "Missing required 'description' field",
},
]
- // The gallery's ExtensionStateContext shim merges arbitrary keys, while the real
- // provider's state type does not expose the skills diagnostics fields the settings
- // panel reads; the cast keeps the story typed without changing production state.
- const initialState = { cwd: "/workspace", skills, skillDiagnostics } as unknown as NonNullable<
- React.ComponentProps["initialState"]
- >
return (
-
+
diff --git a/webview-ui/src/components/settings/__tests__/SkillsSettings.visual.tsx b/webview-ui/src/components/settings/__tests__/SkillsSettings.visual.tsx
index b1eff02240..27330ab8d7 100644
--- a/webview-ui/src/components/settings/__tests__/SkillsSettings.visual.tsx
+++ b/webview-ui/src/components/settings/__tests__/SkillsSettings.visual.tsx
@@ -2,21 +2,13 @@ import { expect, test } from "../../../../playwright/coverage-fixture"
import { collectBoundedLayoutFailures } from "../../../../playwright/layout-contracts"
import { mountedStory } from "../../../../playwright/mounted-story"
-test("shows skill load diagnostics with paths, locations and messages", async ({ mount, page }) => {
+test("renders the diagnostics story deterministically", async ({ mount, page }) => {
await page.setViewportSize({ width: 520, height: 480 })
const component = mountedStory(await mount("skills-settings-diagnostics"))
const story = component.getByTestId("skills-settings-story")
- const alert = story.getByRole("alert")
-
- await expect(alert).toBeVisible()
- // The diagnostics are rendered verbatim: path, optional line:column, message.
- await expect(alert).toContainText("/workspace/.roo/skills/broken-skill/SKILL.md:3:12")
- await expect(alert).toContainText("Invalid YAML frontmatter: unexpected end of the stream")
- await expect(alert).toContainText("/home/user/.roo/skills/legacy-mode/SKILL.md")
- await expect(alert).toContainText("Missing required 'description' field")
- // The valid skill next to the diagnostics still renders its own row.
- await expect(story).toContainText("diagnostics-demo")
+ // Behavioural assertions for the diagnostics alert live in the SkillsSettings Vitest
+ // suite; this test only pins the deterministic state and its screenshot.
await expect(story).toHaveScreenshot("skills-settings-diagnostics.png")
await expect.poll(() => story.evaluate(collectBoundedLayoutFailures)).toEqual([])
})
diff --git a/webview-ui/src/context/ExtensionStateContext.tsx b/webview-ui/src/context/ExtensionStateContext.tsx
index ed7af1fd2d..1aef01e2d9 100644
--- a/webview-ui/src/context/ExtensionStateContext.tsx
+++ b/webview-ui/src/context/ExtensionStateContext.tsx
@@ -284,6 +284,10 @@ export const createInitialExtensionState = (): ExtensionState => ({
type ExtensionStateProviderInitialState = Partial & {
routerModels?: RouterModels
+ // The skill state lives outside ExtensionState; the fixture and tests seed it here so the
+ // initial-state contract covers every value the provider exposes.
+ skills?: SkillMetadata[]
+ skillDiagnostics?: SkillDiagnostic[]
}
export const ExtensionStateContextProvider: React.FC<{
@@ -319,8 +323,10 @@ export const ExtensionStateContextProvider: React.FC<{
global: {},
},
)
- const [skills, setSkills] = useState([])
- const [skillDiagnostics, setSkillDiagnostics] = useState([])
+ const [skills, setSkills] = useState(() => initialState?.skills ?? [])
+ const [skillDiagnostics, setSkillDiagnostics] = useState(
+ () => initialState?.skillDiagnostics ?? [],
+ )
const [rules, setRules] = useState([])
const [includeTaskHistoryInEnhance, setIncludeTaskHistoryInEnhance] = useState(
() => initialState?.includeTaskHistoryInEnhance ?? true,
From 304c647553b42700203d091e46c83c44a21922de Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 03:54:06 +0800
Subject: [PATCH 18/31] fix(skills): propagate a failed realpath resolution
before the atomic publish
Review follow-up: updateSkillModes no longer falls back to the discovered path when realpath fails at publish time; publishing through the unresolved path could replace a symlink with a regular file or write somewhere other than the target the caller saw. Added a test where discovery succeeds but the resolution fails: the exact error propagates and no write/rename happens. SkillsManager.spec.ts 117 passed.
---
src/services/skills/SkillsManager.ts | 10 ++-----
.../skills/__tests__/SkillsManager.spec.ts | 29 +++++++++++++++++++
2 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index 44cdcad378..6dab743cca 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -1100,13 +1100,9 @@ Add your skill instructions here.
const newContent = matter.stringify(body, frontmatter, SKILL_YAML_DUMP_OPTIONS)
// Resolve the link target first: publishing through the temp+rename must update the file
// the skill was discovered through, not replace a symlinked SKILL.md with a regular file.
- let targetPath = skill.path
- try {
- targetPath = await fs.realpath(skill.path)
- } catch {
- // Unresolvable path (already missing, or a platform without realpath): keep the
- // discovered path and let the write report the real failure.
- }
+ // A resolution failure propagates: publishing through the unresolved path could replace a
+ // symlink with a regular file (or write somewhere other than the target the caller saw).
+ const targetPath = await fs.realpath(skill.path)
await this.writeSkillFileAtomically(targetPath, newContent)
// Refresh skills list
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index 8c194871fc..d1b9d704ac 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -2534,6 +2534,35 @@ Body here.`
expect(parsed2.data.description).toBe(description)
})
+ it("propagates a realpath failure instead of writing through the unresolved path", async () => {
+ const skillDir = p(globalSkillsDir, "unresolvable-skill")
+ const skillMd = p(skillDir, "SKILL.md")
+ mockDirectoryExists.mockImplementation(async (dir: string) => dir === globalSkillsDir)
+ mockReaddir.mockImplementation(async (dir: string) =>
+ dir === globalSkillsDir ? ["unresolvable-skill"] : [],
+ )
+ mockStat.mockImplementation(async (pathArg: string) => {
+ if (pathArg === skillDir) {
+ return { isDirectory: () => true }
+ }
+ throw enoent()
+ })
+ mockFileExists.mockImplementation(async (file: string) => file === skillMd)
+ mockReadFile.mockResolvedValue("---\nname: unresolvable-skill\ndescription: A skill\n---\nInstructions")
+
+ await skillsManager.discoverSkills()
+ // The discovered path resolves, but the publish-time resolution fails: the write must
+ // not fall back to the unresolved (potentially symlinked) path.
+ const resolutionError = Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" })
+ mockRealpath.mockRejectedValueOnce(resolutionError)
+
+ await expect(skillsManager.updateSkillModes("unresolvable-skill", "global", ["code"])).rejects.toBe(
+ resolutionError,
+ )
+ expect(mockWriteFile).not.toHaveBeenCalled()
+ expect(mockRename).not.toHaveBeenCalled()
+ })
+
it("should throw error for invalid skill name", async () => {
await expect(skillsManager.createSkill("Invalid-Name", "global", "Description")).rejects.toThrow(
"Skill name must be lowercase letters/numbers/hyphens only",
From 3f0010f21123a90799dd87342ff9f43be8c2dfdc Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 04:30:15 +0800
Subject: [PATCH 19/31] fix(skills): keep SKILL.md publishes inside the skills
roots and track temp sidecars
Review follow-ups: updateSkillModes resolves the SKILL.md target and rejects it when the real path falls outside every configured skills root for the source, so a direct skill-directory symlink cannot publish outside the selected folder. Atomic writes track their temp sidecar in the manager while in flight, dispose() removes any still-tracked sidecar, and initialize() sweeps stale sidecars left by a previous host under the known skills roots. Added focused tests for createSkill/updateSkillModes rename rejection (temp removed, original error rethrown), outside-root refusal (no write/rename), and dispose sidecar cleanup. SkillsManager.spec.ts 121 passed; tsc clean.
---
src/services/skills/SkillsManager.ts | 75 ++++++++++++++++
.../skills/__tests__/SkillsManager.spec.ts | 87 +++++++++++++++++++
2 files changed, 162 insertions(+)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index 6dab743cca..578dfdaa8c 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -110,16 +110,62 @@ export class SkillsManager {
// Runs discovery passes one at a time, so an older pass can't finish after a newer one
private discoveryQueue: Promise = Promise.resolve()
private isDisposed = false
+ // Atomic SKILL.md writes in flight, tracked so dispose() can remove temp sidecars if the
+ // extension host stops between the staged write and the rename.
+ private activeTempFiles: Set = new Set()
constructor(provider: ClineProvider) {
this.providerRef = new WeakRef(provider)
}
async initialize(): Promise {
+ // A previous extension-host stop can leave a staged SKILL.md temp sidecar behind; the
+ // in-memory tracking cannot survive that, so sweep the known skills directories once.
+ try {
+ await this.sweepStaleSkillTempFiles()
+ } catch {
+ // Stale-cleanup is best effort; discovery and watcher setup still run.
+ }
await this.discoverSkills()
await this.setupFileWatchers()
}
+ /**
+ * Remove SKILL.md temp sidecars left by a previous host: the name pattern is the one
+ * writeSkillFileAtomically generates, and only the skill directories under the known skills
+ * roots are scanned.
+ */
+ private async sweepStaleSkillTempFiles(): Promise {
+ const tempPattern = /^\..*\.SKILL\.md\..*\.tmp$/
+ const roots = (await this.getSkillsDirectories()).map((entry) => entry.dir)
+ for (const root of roots) {
+ let entries: unknown[]
+ try {
+ entries = await fs.readdir(root, { withFileTypes: true })
+ } catch {
+ continue
+ }
+ const directories = [root]
+ for (const entry of entries) {
+ const dirent = entry as { name?: string; isDirectory?: () => boolean }
+ if (!dirent?.isDirectory?.()) continue
+ directories.push(path.join(root, String(dirent.name)))
+ }
+ for (const directory of directories) {
+ let files: string[]
+ try {
+ files = await fs.readdir(directory)
+ } catch {
+ continue
+ }
+ for (const file of files) {
+ if (!tempPattern.test(file)) continue
+ await fs.unlink(path.join(directory, file)).catch(() => {})
+ }
+ }
+ }
+ }
+
/**
* Discover all skills from global and project directories.
* Supports both generic skills (skills/) and mode-specific skills (skills-{mode}/).
@@ -675,6 +721,7 @@ export class SkillsManager {
directory,
`.${path.basename(filePath)}.${Date.now()}.${Math.random().toString(36).slice(2)}.tmp`,
)
+ this.activeTempFiles.add(temporaryPath)
try {
await fs.writeFile(temporaryPath, content, "utf-8")
await fs.rename(temporaryPath, filePath)
@@ -683,6 +730,8 @@ export class SkillsManager {
// failure is the one the caller needs to see.
await fs.unlink(temporaryPath).catch(() => {})
throw error
+ } finally {
+ this.activeTempFiles.delete(temporaryPath)
}
}
@@ -1063,6 +1112,26 @@ Add your skill instructions here.
* @param source - Where the skill is located ("global" or "project")
* @param newModeSlugs - New mode slugs (undefined/empty = any mode)
*/
+ /**
+ * The resolved SKILL.md target must stay inside one of the configured skills roots for the
+ * source: a direct skill-directory symlink can point anywhere, and publishing through it
+ * would write outside the user selected skills folder.
+ */
+ private async assertTargetWithinSkillsRoots(targetPath: string, source: "global" | "project"): Promise {
+ const roots = (await this.getSkillsDirectories())
+ .filter((entry) => entry.source === source)
+ .map((entry) => entry.dir)
+ const inside = async (root: string): Promise => {
+ const resolvedRoot = await fs.realpath(root).catch(() => root)
+ const relative = path.relative(resolvedRoot, targetPath)
+ return relative.length > 0 && !relative.startsWith("..") && !path.isAbsolute(relative)
+ }
+ for (const root of roots) {
+ if (await inside(root)) return
+ }
+ throw new Error(`Refusing to update SKILL.md outside the ${source} skills directories: ${targetPath}`)
+ }
+
async updateSkillModes(name: string, source: "global" | "project", newModeSlugs?: string[]): Promise {
// Find any skill with this name and source (regardless of current mode)
let skill: SkillMetadata | undefined
@@ -1103,6 +1172,7 @@ Add your skill instructions here.
// A resolution failure propagates: publishing through the unresolved path could replace a
// symlink with a regular file (or write somewhere other than the target the caller saw).
const targetPath = await fs.realpath(skill.path)
+ await this.assertTargetWithinSkillsRoots(targetPath, source)
await this.writeSkillFileAtomically(targetPath, newContent)
// Refresh skills list
@@ -1284,6 +1354,11 @@ Add your skill instructions here.
async dispose(): Promise {
this.isDisposed = true
+ // Any sidecar still tracked is a write that never reached its rename.
+ for (const temporaryPath of this.activeTempFiles) {
+ await fs.unlink(temporaryPath).catch(() => {})
+ }
+ this.activeTempFiles.clear()
this.disposables.forEach((d) => d.dispose())
this.disposables = []
this.containerWatchers.forEach((w) => w.dispose())
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index d1b9d704ac..02372f88b7 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -2379,6 +2379,26 @@ Instructions`)
expect(mockRm).not.toHaveBeenCalled()
})
+ it("removes the temp file and rethrows the original error when the create rename fails", async () => {
+ mockDirectoryExists.mockResolvedValue(false)
+ mockRealpath.mockImplementation(async (p: string) => p)
+ mockReaddir.mockResolvedValue([])
+ mockFileExists.mockResolvedValue(false)
+ mockMkdir.mockResolvedValue(p(GLOBAL_ROO_DIR, "skills", "rename-fails"))
+ mockWriteFile.mockResolvedValue(undefined)
+ const renameError = Object.assign(new Error("EEXIST: rename failed"), { code: "EEXIST" })
+ mockRename.mockRejectedValueOnce(renameError)
+ mockUnlink.mockResolvedValue(undefined)
+ mockRmdir.mockResolvedValue(undefined)
+
+ await expect(skillsManager.createSkill("rename-fails", "global", "Description")).rejects.toBe(renameError)
+
+ const temporaryPath = String(mockWriteFile.mock.calls[0][0])
+ expect(temporaryPath).toMatch(/\.SKILL\.md\.[^/\\]+\.tmp$/)
+ expect(mockUnlink).toHaveBeenCalledWith(temporaryPath)
+ expect(skillsManager["activeTempFiles"].size).toBe(0)
+ })
+
it("does not remove a pre-existing skill directory when persistence fails", async () => {
mockDirectoryExists.mockResolvedValue(false)
mockRealpath.mockImplementation(async (p: string) => p)
@@ -2563,6 +2583,64 @@ Body here.`
expect(mockRename).not.toHaveBeenCalled()
})
+ it("removes the temp file and rethrows the original error when the update rename fails", async () => {
+ const skillDir = p(globalSkillsDir, "rename-update")
+ const skillMd = p(skillDir, "SKILL.md")
+ mockDirectoryExists.mockImplementation(async (dir: string) => dir === globalSkillsDir)
+ mockReaddir.mockImplementation(async (dir: string) => (dir === globalSkillsDir ? ["rename-update"] : []))
+ mockStat.mockImplementation(async (pathArg: string) => {
+ if (pathArg === skillDir) {
+ return { isDirectory: () => true }
+ }
+ throw enoent()
+ })
+ mockFileExists.mockImplementation(async (file: string) => file === skillMd)
+ mockReadFile.mockResolvedValue("---\nname: rename-update\ndescription: A skill\n---\nInstructions")
+ mockRealpath.mockImplementation(async (pathArg: string) => pathArg)
+ mockWriteFile.mockResolvedValue(undefined)
+ const renameError = Object.assign(new Error("EIO: rename failed"), { code: "EIO" })
+ mockRename.mockRejectedValueOnce(renameError)
+ mockUnlink.mockResolvedValue(undefined)
+
+ await skillsManager.discoverSkills()
+
+ await expect(skillsManager.updateSkillModes("rename-update", "global", ["code"])).rejects.toBe(renameError)
+
+ const temporaryPath = String(mockWriteFile.mock.calls[0][0])
+ expect(temporaryPath).toMatch(/\.SKILL\.md\.[^/\\]+\.tmp$/)
+ expect(mockUnlink).toHaveBeenCalledWith(temporaryPath)
+ expect(skillsManager["activeTempFiles"].size).toBe(0)
+ })
+
+ it("refuses to update a SKILL.md that resolves outside the skills roots", async () => {
+ const skillDir = p(globalSkillsDir, "escaping")
+ const skillMd = p(skillDir, "SKILL.md")
+ const outsidePath = p("/outside/root", "SKILL.md")
+ mockDirectoryExists.mockImplementation(async (dir: string) => dir === globalSkillsDir)
+ mockReaddir.mockImplementation(async (dir: string) => (dir === globalSkillsDir ? ["escaping"] : []))
+ mockStat.mockImplementation(async (pathArg: string) => {
+ if (pathArg === skillDir) {
+ return { isDirectory: () => true }
+ }
+ throw enoent()
+ })
+ mockFileExists.mockImplementation(async (file: string) => file === skillMd)
+ mockReadFile.mockResolvedValue("---\nname: escaping\ndescription: A skill\n---\nInstructions")
+ // Discovery saw the root path, but the publish-time resolution points outside every
+ // configured skills root: a direct skill-dir symlink must not let the write escape.
+ mockRealpath.mockImplementation(async (pathArg: string) => (pathArg === skillMd ? outsidePath : pathArg))
+ mockWriteFile.mockResolvedValue(undefined)
+ mockRename.mockResolvedValue(undefined)
+
+ await skillsManager.discoverSkills()
+
+ await expect(skillsManager.updateSkillModes("escaping", "global", ["code"])).rejects.toThrow(
+ /Refusing to update SKILL\.md outside the global skills directories/,
+ )
+ expect(mockWriteFile).not.toHaveBeenCalled()
+ expect(mockRename).not.toHaveBeenCalled()
+ })
+
it("should throw error for invalid skill name", async () => {
await expect(skillsManager.createSkill("Invalid-Name", "global", "Description")).rejects.toThrow(
"Skill name must be lowercase letters/numbers/hyphens only",
@@ -3967,4 +4045,13 @@ Instructions`)
})
})
})
+ it("removes tracked temp sidecars on dispose", async () => {
+ mockUnlink.mockResolvedValue(undefined)
+ skillsManager["activeTempFiles"].add("/mock/skills/foo/.SKILL.md.123.abc.tmp")
+
+ await skillsManager.dispose()
+
+ expect(mockUnlink).toHaveBeenCalledWith("/mock/skills/foo/.SKILL.md.123.abc.tmp")
+ expect(skillsManager["activeTempFiles"].size).toBe(0)
+ })
})
From 29019356f962e235222957138781b6b0543c93bf Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 04:54:43 +0800
Subject: [PATCH 20/31] fix(skills): exclusive staging, pre-rename
revalidation, and cancellation-safe init
Review follow-ups: staged SKILL.md temp files are created with flag wx so a pre-created symlink at the generated name cannot be followed, and updateSkillModes re-resolves the target immediately before the rename and re-validates it against the source skills roots. The rename-rejection tests now also assert the wx flag, that create cleanup removes only its own directory (rmdir, no recursive rm), and that the existing SKILL.md is not replaced. initialize() is idempotent/cancellation-safe: it tracks the initialization promise and stops after the stale sweep or discovery if the manager was disposed, and setupFileWatchers/watchDirectory return early when disposed (test: initialize after dispose starts no watchers). SkillsManager.spec.ts 122 passed; tsc clean.
---
src/services/skills/SkillsManager.ts | 48 ++++++++++++++-----
.../skills/__tests__/SkillsManager.spec.ts | 15 ++++++
2 files changed, 50 insertions(+), 13 deletions(-)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index 578dfdaa8c..a15b8780ab 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -118,16 +118,25 @@ export class SkillsManager {
this.providerRef = new WeakRef(provider)
}
- async initialize(): Promise {
- // A previous extension-host stop can leave a staged SKILL.md temp sidecar behind; the
- // in-memory tracking cannot survive that, so sweep the known skills directories once.
- try {
- await this.sweepStaleSkillTempFiles()
- } catch {
- // Stale-cleanup is best effort; discovery and watcher setup still run.
- }
- await this.discoverSkills()
- await this.setupFileWatchers()
+ private initialization: Promise | null = null
+
+ initialize(): Promise {
+ if (this.initialization) return this.initialization
+ this.initialization = (async () => {
+ // A previous extension-host stop can leave a staged SKILL.md temp sidecar behind; the
+ // in-memory tracking cannot survive that, so sweep the known skills directories once.
+ try {
+ await this.sweepStaleSkillTempFiles()
+ } catch {
+ // Stale-cleanup is best effort; discovery and watcher setup still run.
+ }
+ // Disposal can land during the sweep: stop before touching discovery or watchers.
+ if (this.isDisposed) return
+ await this.discoverSkills()
+ if (this.isDisposed) return
+ await this.setupFileWatchers()
+ })()
+ return this.initialization
}
/**
@@ -715,7 +724,11 @@ export class SkillsManager {
* must not leave a truncated SKILL.md that the loader would silently misparse.
* The temp file is removed when the write or rename fails.
*/
- private async writeSkillFileAtomically(filePath: string, content: string): Promise {
+ private async writeSkillFileAtomically(
+ filePath: string,
+ content: string,
+ validateTarget?: (targetPath: string) => Promise,
+ ): Promise {
const directory = path.dirname(filePath)
const temporaryPath = path.join(
directory,
@@ -723,7 +736,11 @@ export class SkillsManager {
)
this.activeTempFiles.add(temporaryPath)
try {
- await fs.writeFile(temporaryPath, content, "utf-8")
+ // Exclusive creation: a pre-created symlink at the generated name cannot be followed.
+ await fs.writeFile(temporaryPath, content, { encoding: "utf-8", flag: "wx" })
+ // Re-validate the final target immediately before the rename: the target (or an
+ // ancestor) can change between the caller's resolution and this publish.
+ await validateTarget?.(filePath)
await fs.rename(temporaryPath, filePath)
} catch (error) {
// Best effort: a partial temp file must not be left behind. The original
@@ -1173,7 +1190,10 @@ Add your skill instructions here.
// symlink with a regular file (or write somewhere other than the target the caller saw).
const targetPath = await fs.realpath(skill.path)
await this.assertTargetWithinSkillsRoots(targetPath, source)
- await this.writeSkillFileAtomically(targetPath, newContent)
+ await this.writeSkillFileAtomically(targetPath, newContent, async (target) => {
+ const freshTarget = await fs.realpath(target).catch(() => target)
+ await this.assertTargetWithinSkillsRoots(freshTarget, source)
+ })
// Refresh skills list
await this.discoverSkills()
@@ -1264,6 +1284,7 @@ Add your skill instructions here.
}
private async setupFileWatchers(): Promise {
+ if (this.isDisposed) return
// Skip if test environment is detected or VSCode APIs are not available
if (process.env.NODE_ENV === "test" || !vscode.workspace.createFileSystemWatcher) {
return
@@ -1303,6 +1324,7 @@ Add your skill instructions here.
}
private watchDirectory(dirPath: string): void {
+ if (this.isDisposed) return
const watcher = this.createSkillsWatcher(dirPath)
if (watcher) {
this.disposables.push(watcher)
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index 02372f88b7..b10edc903e 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -2608,7 +2608,12 @@ Body here.`
const temporaryPath = String(mockWriteFile.mock.calls[0][0])
expect(temporaryPath).toMatch(/\.SKILL\.md\.[^/\\]+\.tmp$/)
+ expect(mockWriteFile.mock.calls[0][2]).toEqual(expect.objectContaining({ flag: "wx" }))
expect(mockUnlink).toHaveBeenCalledWith(temporaryPath)
+ // The existing SKILL.md was never replaced: the only write went to the temp sidecar and
+ // the failed rename never committed.
+ expect(mockWriteFile).toHaveBeenCalledTimes(1)
+ expect(mockRename).toHaveBeenCalledWith(temporaryPath, skillMd)
expect(skillsManager["activeTempFiles"].size).toBe(0)
})
@@ -4045,6 +4050,16 @@ Instructions`)
})
})
})
+ it("does not start watchers when initialization runs after disposal", async () => {
+ const createWatcher = vi.mocked(vscode.workspace.createFileSystemWatcher)
+ await skillsManager.dispose()
+ createWatcher.mockClear()
+
+ await skillsManager.initialize()
+
+ expect(createWatcher).not.toHaveBeenCalled()
+ })
+
it("removes tracked temp sidecars on dispose", async () => {
mockUnlink.mockResolvedValue(undefined)
skillsManager["activeTempFiles"].add("/mock/skills/foo/.SKILL.md.123.abc.tmp")
From 30e0e01962f8c679c1359eea04f13f3eda4275ec Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 05:15:22 +0800
Subject: [PATCH 21/31] fix(skills): match the real sidecar name, guard watcher
setup, and only clean staged files this write owns
Review follow-ups: the stale-sidecar sweep pattern now matches the generated name (.SKILL.md...tmp) with a restart-cleanup test; a focused watcher-guard test runs setupFileWatchers under NODE_ENV=development, then disposes and asserts no watcher is created again; the atomic writer cleans up a staged sidecar unless exclusive creation failed with EEXIST (an existing file at the generated name belongs to someone else), with a test that preserves it. SkillsManager.spec.ts 125 passed.
---
src/services/skills/SkillsManager.ts | 16 ++++--
.../skills/__tests__/SkillsManager.spec.ts | 51 +++++++++++++++++++
2 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index a15b8780ab..761466def8 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -145,7 +145,7 @@ export class SkillsManager {
* roots are scanned.
*/
private async sweepStaleSkillTempFiles(): Promise {
- const tempPattern = /^\..*\.SKILL\.md\..*\.tmp$/
+ const tempPattern = /^\.SKILL\.md\..*\.tmp$/
const roots = (await this.getSkillsDirectories()).map((entry) => entry.dir)
for (const root of roots) {
let entries: unknown[]
@@ -735,17 +735,25 @@ export class SkillsManager {
`.${path.basename(filePath)}.${Date.now()}.${Math.random().toString(36).slice(2)}.tmp`,
)
this.activeTempFiles.add(temporaryPath)
+ // Only a write that actually created the sidecar through exclusive creation may remove it:
+ // when wx fails because the path already exists, that file belongs to someone else.
+ let ownsTemporaryPath = false
try {
// Exclusive creation: a pre-created symlink at the generated name cannot be followed.
await fs.writeFile(temporaryPath, content, { encoding: "utf-8", flag: "wx" })
+ ownsTemporaryPath = true
// Re-validate the final target immediately before the rename: the target (or an
// ancestor) can change between the caller's resolution and this publish.
await validateTarget?.(filePath)
await fs.rename(temporaryPath, filePath)
} catch (error) {
- // Best effort: a partial temp file must not be left behind. The original
- // failure is the one the caller needs to see.
- await fs.unlink(temporaryPath).catch(() => {})
+ // Best effort: a partial temp file must not be left behind, but an EEXIST failure means
+ // exclusive creation never acquired the path and the existing file belongs to someone
+ // else. Any other failure may have created a partial sidecar and is cleaned up; the
+ // original failure is the one the caller needs to see.
+ if (ownsTemporaryPath || (error as { code?: string })?.code !== "EEXIST") {
+ await fs.unlink(temporaryPath).catch(() => {})
+ }
throw error
} finally {
this.activeTempFiles.delete(temporaryPath)
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index b10edc903e..d181a7e1f2 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -2379,6 +2379,24 @@ Instructions`)
expect(mockRm).not.toHaveBeenCalled()
})
+ it("preserves an existing file when exclusive staging fails because the name exists", async () => {
+ mockDirectoryExists.mockResolvedValue(false)
+ mockRealpath.mockImplementation(async (p: string) => p)
+ mockReaddir.mockResolvedValue([])
+ mockFileExists.mockResolvedValue(false)
+ mockMkdir.mockResolvedValue(p(GLOBAL_ROO_DIR, "skills", "exists"))
+ const existsError = Object.assign(new Error("EEXIST: file already exists"), { code: "EEXIST" })
+ mockWriteFile.mockRejectedValueOnce(existsError)
+ mockUnlink.mockResolvedValue(undefined)
+
+ await expect(skillsManager.createSkill("exists", "global", "Description")).rejects.toBe(existsError)
+
+ // wx failed before this write owned the path: the existing file must not be unlinked.
+ expect(mockUnlink).not.toHaveBeenCalled()
+ expect(mockRename).not.toHaveBeenCalled()
+ expect(skillsManager["activeTempFiles"].size).toBe(0)
+ })
+
it("removes the temp file and rethrows the original error when the create rename fails", async () => {
mockDirectoryExists.mockResolvedValue(false)
mockRealpath.mockImplementation(async (p: string) => p)
@@ -4050,6 +4068,39 @@ Instructions`)
})
})
})
+ it("stops creating watchers once the manager is disposed", async () => {
+ const createWatcher = vi.mocked(vscode.workspace.createFileSystemWatcher)
+ const originalNodeEnv = process.env.NODE_ENV
+ process.env.NODE_ENV = "development"
+ try {
+ createWatcher.mockClear()
+ await skillsManager["setupFileWatchers"]()
+ expect(createWatcher).toHaveBeenCalled()
+
+ createWatcher.mockClear()
+ await skillsManager.dispose()
+ await skillsManager["setupFileWatchers"]()
+ expect(createWatcher).not.toHaveBeenCalled()
+ } finally {
+ process.env.NODE_ENV = originalNodeEnv
+ }
+ })
+
+ it("sweeps a staged sidecar left by a previous host", async () => {
+ const staleName = ".SKILL.md.123.abc.tmp"
+ mockReaddir.mockImplementation(async (_dir: string, options?: { withFileTypes?: boolean }) => {
+ if (options?.withFileTypes) {
+ return []
+ }
+ return [staleName]
+ })
+ mockUnlink.mockResolvedValue(undefined)
+
+ await skillsManager["sweepStaleSkillTempFiles"]()
+
+ expect(mockUnlink).toHaveBeenCalledWith(p(globalSkillsDir, staleName))
+ })
+
it("does not start watchers when initialization runs after disposal", async () => {
const createWatcher = vi.mocked(vscode.workspace.createFileSystemWatcher)
await skillsManager.dispose()
From 08b2447db091aef6fcd81808231da95ea38f47b5 Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 05:28:15 +0800
Subject: [PATCH 22/31] ci: retrigger platform tests after the vitest worker
teardown flake
The previous run executed all 3934 tests successfully but vitest reported EnvironmentTeardownError (onUserConsoleLog pending) in grace-retry-errors.spec.ts during worker shutdown, failing the job. No source change.
From 1100ade9cf8c6c480fe08ba86873d62fb2d13648 Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 05:50:05 +0800
Subject: [PATCH 23/31] fix(skills): narrow the sidecar pattern and serialize
sweep/write per directory
Review follow-ups: the stale-sidecar sweep only matches the writer-generated name (dot, SKILL.md, numeric timestamp, base36 suffix, .tmp), and a static per-directory queue shared by every SkillsManager instance wraps both the sweep and the atomic write create/validate/rename path so cleanup cannot unlink a sidecar another instance is writing. SkillsManager.spec.ts 125 passed.
---
src/services/skills/SkillsManager.ts | 102 +++++++++++++++++----------
1 file changed, 64 insertions(+), 38 deletions(-)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index 761466def8..1d9773e2a2 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -113,6 +113,10 @@ export class SkillsManager {
// Atomic SKILL.md writes in flight, tracked so dispose() can remove temp sidecars if the
// extension host stops between the staged write and the rename.
private activeTempFiles: Set = new Set()
+ // Per-directory serialization for SKILL.md sidecar cleanup and writes. The queue is static so
+ // every SkillsManager instance in this process shares it: a sweep cannot unlink a sidecar that
+ // another instance is still writing, and writes cannot interleave their validate/rename steps.
+ private static directoryQueues: Map> = new Map()
constructor(provider: ClineProvider) {
this.providerRef = new WeakRef(provider)
@@ -145,7 +149,7 @@ export class SkillsManager {
* roots are scanned.
*/
private async sweepStaleSkillTempFiles(): Promise {
- const tempPattern = /^\.SKILL\.md\..*\.tmp$/
+ const tempPattern = /^\.SKILL\.md\.\d+\.[a-z0-9]+\.tmp$/
const roots = (await this.getSkillsDirectories()).map((entry) => entry.dir)
for (const root of roots) {
let entries: unknown[]
@@ -161,16 +165,18 @@ export class SkillsManager {
directories.push(path.join(root, String(dirent.name)))
}
for (const directory of directories) {
- let files: string[]
- try {
- files = await fs.readdir(directory)
- } catch {
- continue
- }
- for (const file of files) {
- if (!tempPattern.test(file)) continue
- await fs.unlink(path.join(directory, file)).catch(() => {})
- }
+ await this.withDirectoryLock(directory, async () => {
+ let files: string[]
+ try {
+ files = await fs.readdir(directory)
+ } catch {
+ return
+ }
+ for (const file of files) {
+ if (!tempPattern.test(file)) continue
+ await fs.unlink(path.join(directory, file)).catch(() => {})
+ }
+ })
}
}
}
@@ -724,40 +730,60 @@ export class SkillsManager {
* must not leave a truncated SKILL.md that the loader would silently misparse.
* The temp file is removed when the write or rename fails.
*/
+ private async withDirectoryLock(directory: string, operation: () => Promise): Promise {
+ const key = path.resolve(directory)
+ const previous = SkillsManager.directoryQueues.get(key) ?? Promise.resolve()
+ const run = previous.then(operation)
+ const tracked = run.then(
+ () => undefined,
+ () => undefined,
+ )
+ SkillsManager.directoryQueues.set(key, tracked)
+ try {
+ return await run
+ } finally {
+ if (SkillsManager.directoryQueues.get(key) === tracked) {
+ SkillsManager.directoryQueues.delete(key)
+ }
+ }
+ }
+
private async writeSkillFileAtomically(
filePath: string,
content: string,
validateTarget?: (targetPath: string) => Promise,
): Promise {
const directory = path.dirname(filePath)
- const temporaryPath = path.join(
- directory,
- `.${path.basename(filePath)}.${Date.now()}.${Math.random().toString(36).slice(2)}.tmp`,
- )
- this.activeTempFiles.add(temporaryPath)
- // Only a write that actually created the sidecar through exclusive creation may remove it:
- // when wx fails because the path already exists, that file belongs to someone else.
- let ownsTemporaryPath = false
- try {
- // Exclusive creation: a pre-created symlink at the generated name cannot be followed.
- await fs.writeFile(temporaryPath, content, { encoding: "utf-8", flag: "wx" })
- ownsTemporaryPath = true
- // Re-validate the final target immediately before the rename: the target (or an
- // ancestor) can change between the caller's resolution and this publish.
- await validateTarget?.(filePath)
- await fs.rename(temporaryPath, filePath)
- } catch (error) {
- // Best effort: a partial temp file must not be left behind, but an EEXIST failure means
- // exclusive creation never acquired the path and the existing file belongs to someone
- // else. Any other failure may have created a partial sidecar and is cleaned up; the
- // original failure is the one the caller needs to see.
- if (ownsTemporaryPath || (error as { code?: string })?.code !== "EEXIST") {
- await fs.unlink(temporaryPath).catch(() => {})
+ await this.withDirectoryLock(directory, async () => {
+ const temporaryPath = path.join(
+ directory,
+ `.${path.basename(filePath)}.${Date.now()}.${Math.random().toString(36).slice(2)}.tmp`,
+ )
+ this.activeTempFiles.add(temporaryPath)
+ // Only a write that actually created the sidecar through exclusive creation may remove
+ // it: when wx fails because the path already exists, that file belongs to someone else.
+ let ownsTemporaryPath = false
+ try {
+ // Exclusive creation: a pre-created symlink at the generated name cannot be followed.
+ await fs.writeFile(temporaryPath, content, { encoding: "utf-8", flag: "wx" })
+ ownsTemporaryPath = true
+ // Re-validate the final target immediately before the rename: the target (or an
+ // ancestor) can change between the caller's resolution and this publish.
+ await validateTarget?.(filePath)
+ await fs.rename(temporaryPath, filePath)
+ } catch (error) {
+ // Best effort: a partial temp file must not be left behind, but an EEXIST failure
+ // means exclusive creation never acquired the path and the existing file belongs to
+ // someone else. Any other failure may have created a partial sidecar and is cleaned
+ // up; the original failure is the one the caller needs to see.
+ if (ownsTemporaryPath || (error as { code?: string })?.code !== "EEXIST") {
+ await fs.unlink(temporaryPath).catch(() => {})
+ }
+ throw error
+ } finally {
+ this.activeTempFiles.delete(temporaryPath)
}
- throw error
- } finally {
- this.activeTempFiles.delete(temporaryPath)
- }
+ })
}
async createSkill(
From ca68406ec3becf7e8f4a3615d7c9af4048cc11a4 Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 06:13:06 +0800
Subject: [PATCH 24/31] fix(skills): bound the stale sweep by age and key the
directory queue by realpath
Review follow-ups: sweepStaleSkillTempFiles only removes matching sidecars older than ten minutes so a fresh file owned by a live write in another process is left alone; withDirectoryLock resolves the directory through fs.realpath before deriving its queue key (fallback path.resolve) so symlink aliases share one lock. Tests cover the fresh-sidecar survival and alias serialization. SkillsManager.spec.ts 126 passed; tsc clean.
---
src/services/skills/SkillsManager.ts | 14 ++++-
.../skills/__tests__/SkillsManager.spec.ts | 51 ++++++++++++++++++-
2 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index 1d9773e2a2..07d11159c9 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -93,6 +93,10 @@ function isHiddenEntry(name: string): boolean {
return name.startsWith(".")
}
+// Stale-sidecar sweep: a matching temp file younger than this may belong to a live write in
+// another process, so only older sidecars are treated as abandoned by a previous host.
+const SKILL_TEMP_SIDECAR_STALE_MS = 10 * 60 * 1000
+
export class SkillsManager {
private skills: Map = new Map()
private diagnostics: SkillDiagnostic[] = []
@@ -174,7 +178,11 @@ export class SkillsManager {
}
for (const file of files) {
if (!tempPattern.test(file)) continue
- await fs.unlink(path.join(directory, file)).catch(() => {})
+ const stalePath = path.join(directory, file)
+ const stats = await fs.stat(stalePath).catch(() => undefined)
+ // Leave fresh sidecars alone: they may belong to a write in another process.
+ if (!stats || Date.now() - stats.mtimeMs < SKILL_TEMP_SIDECAR_STALE_MS) continue
+ await fs.unlink(stalePath).catch(() => {})
}
})
}
@@ -731,7 +739,9 @@ export class SkillsManager {
* The temp file is removed when the write or rename fails.
*/
private async withDirectoryLock(directory: string, operation: () => Promise): Promise {
- const key = path.resolve(directory)
+ // Normalize through realpath first so a symlinked alias shares the same queue key; fall
+ // back to the resolved path when the directory cannot be resolved (e.g. not created yet).
+ const key = await fs.realpath(directory).catch(() => path.resolve(directory))
const previous = SkillsManager.directoryQueues.get(key) ?? Promise.resolve()
const run = previous.then(operation)
const tracked = run.then(
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index d181a7e1f2..c86a4a080b 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -4086,19 +4086,66 @@ Instructions`)
}
})
- it("sweeps a staged sidecar left by a previous host", async () => {
+ it("sweeps only a staged sidecar left by a previous host", async () => {
const staleName = ".SKILL.md.123.abc.tmp"
+ const freshName = ".SKILL.md.456.def.tmp"
mockReaddir.mockImplementation(async (_dir: string, options?: { withFileTypes?: boolean }) => {
if (options?.withFileTypes) {
return []
}
- return [staleName]
+ return [staleName, freshName]
+ })
+ mockStat.mockImplementation(async (pathArg: string) => {
+ if (pathArg.endsWith(staleName)) {
+ // Older than the stale threshold: abandoned by a previous host.
+ return { mtimeMs: Date.now() - 60 * 60 * 1000 }
+ }
+ if (pathArg.endsWith(freshName)) {
+ return { mtimeMs: Date.now() }
+ }
+ throw enoent()
})
mockUnlink.mockResolvedValue(undefined)
await skillsManager["sweepStaleSkillTempFiles"]()
expect(mockUnlink).toHaveBeenCalledWith(p(globalSkillsDir, staleName))
+ // A fresh matching sidecar may belong to a live write and must survive.
+ expect(mockUnlink).not.toHaveBeenCalledWith(p(globalSkillsDir, freshName))
+ })
+
+ it("shares one directory queue across symlink aliases", async () => {
+ const order: string[] = []
+ let releaseFirst!: () => void
+ const firstGate = new Promise((resolve) => {
+ releaseFirst = resolve
+ })
+ const alias = p("/alias", "skills")
+ mockRealpath.mockImplementation(async (pathArg: string) => (pathArg === alias ? globalSkillsDir : pathArg))
+
+ const first = skillsManager["withDirectoryLock"](alias, async () => {
+ order.push("first:start")
+ await firstGate
+ order.push("first:end")
+ })
+ try {
+ // The lock key is resolved through realpath asynchronously before the operation runs.
+ await vi.waitFor(() => expect(order).toEqual(["first:start"]))
+ const second = skillsManager["withDirectoryLock"](globalSkillsDir, async () => {
+ order.push("second")
+ })
+ await Promise.resolve()
+ // The alias resolves to the same real directory, so the second operation waits.
+ expect(order).toEqual(["first:start"])
+
+ releaseFirst()
+ await Promise.all([first, second])
+ expect(order).toEqual(["first:start", "first:end", "second"])
+ } finally {
+ // Never leave the shared queue holding a gate if an assertion above fails.
+ releaseFirst()
+ await first.catch(() => undefined)
+ }
})
it("does not start watchers when initialization runs after disposal", async () => {
From 741a280a0b2f1b822480e46f90e57c4ae04e726e Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 06:28:20 +0800
Subject: [PATCH 25/31] fix(skills): fail closed on pre-rename resolution and
make queued writes disposal-aware
Review follow-ups: the pre-rename realpath has no fallback to the stale path (the error propagates, the owned sidecar is removed, and the rename is skipped), the update rename-rejection test asserts the existing SKILL.md and directory survive, and queued writes reject when the manager is disposed before the operation starts and re-check disposal after acquiring the directory lock and before staging/validation/rename. Descriptor-relative publication is tracked in #2203. SkillsManager.spec.ts 128 passed; tsc clean.
---
src/services/skills/SkillsManager.ts | 13 ++++-
.../skills/__tests__/SkillsManager.spec.ts | 56 +++++++++++++++++++
2 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index 07d11159c9..418a13e42e 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -743,7 +743,11 @@ export class SkillsManager {
// back to the resolved path when the directory cannot be resolved (e.g. not created yet).
const key = await fs.realpath(directory).catch(() => path.resolve(directory))
const previous = SkillsManager.directoryQueues.get(key) ?? Promise.resolve()
- const run = previous.then(operation)
+ const run = previous.then(async () => {
+ // A manager disposed while this operation waited must not run at all.
+ if (this.isDisposed) throw new Error("SkillsManager is disposed")
+ return operation()
+ })
const tracked = run.then(
() => undefined,
() => undefined,
@@ -774,12 +778,15 @@ export class SkillsManager {
// it: when wx fails because the path already exists, that file belongs to someone else.
let ownsTemporaryPath = false
try {
+ if (this.isDisposed) throw new Error("SkillsManager is disposed")
// Exclusive creation: a pre-created symlink at the generated name cannot be followed.
await fs.writeFile(temporaryPath, content, { encoding: "utf-8", flag: "wx" })
ownsTemporaryPath = true
+ if (this.isDisposed) throw new Error("SkillsManager is disposed")
// Re-validate the final target immediately before the rename: the target (or an
// ancestor) can change between the caller's resolution and this publish.
await validateTarget?.(filePath)
+ if (this.isDisposed) throw new Error("SkillsManager is disposed")
await fs.rename(temporaryPath, filePath)
} catch (error) {
// Best effort: a partial temp file must not be left behind, but an EEXIST failure
@@ -1235,7 +1242,9 @@ Add your skill instructions here.
const targetPath = await fs.realpath(skill.path)
await this.assertTargetWithinSkillsRoots(targetPath, source)
await this.writeSkillFileAtomically(targetPath, newContent, async (target) => {
- const freshTarget = await fs.realpath(target).catch(() => target)
+ // Fail closed: a resolution failure must propagate so the staged temp write is removed
+ // and the rename is skipped, rather than publishing through a stale path.
+ const freshTarget = await fs.realpath(target)
await this.assertTargetWithinSkillsRoots(freshTarget, source)
})
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index c86a4a080b..ca2f33f4f0 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -2632,9 +2632,53 @@ Body here.`
// the failed rename never committed.
expect(mockWriteFile).toHaveBeenCalledTimes(1)
expect(mockRename).toHaveBeenCalledWith(temporaryPath, skillMd)
+ // The existing SKILL.md and its directory remain untouched.
+ expect(mockReadFile).toHaveBeenCalledWith(skillMd, "utf-8")
+ expect(mockWriteFile).not.toHaveBeenCalledWith(skillMd, expect.anything(), expect.anything())
+ expect(mockRmdir).not.toHaveBeenCalled()
+ expect(mockRm).not.toHaveBeenCalled()
expect(skillsManager["activeTempFiles"].size).toBe(0)
})
+ it("fails closed when the pre-rename target resolution fails", async () => {
+ const skillDir = p(globalSkillsDir, "resolution-fails")
+ const skillMd = p(skillDir, "SKILL.md")
+ mockDirectoryExists.mockImplementation(async (dir: string) => dir === globalSkillsDir)
+ mockReaddir.mockImplementation(async (dir: string) => (dir === globalSkillsDir ? ["resolution-fails"] : []))
+ mockStat.mockImplementation(async (pathArg: string) => {
+ if (pathArg === skillDir) {
+ return { isDirectory: () => true }
+ }
+ throw enoent()
+ })
+ mockFileExists.mockImplementation(async (file: string) => file === skillMd)
+ mockReadFile.mockResolvedValue("---\nname: resolution-fails\ndescription: A skill\n---\nInstructions")
+ // The first resolution succeeds; the immediate pre-rename resolution fails and must
+ // propagate instead of publishing through the stale path.
+ let resolutions = 0
+ mockRealpath.mockImplementation(async (pathArg: string) => {
+ if (pathArg === skillMd) {
+ resolutions += 1
+ if (resolutions === 1) {
+ return skillMd
+ }
+ throw Object.assign(new Error("EIO: resolution failed"), { code: "EIO" })
+ }
+ return pathArg
+ })
+ mockWriteFile.mockResolvedValue(undefined)
+ mockUnlink.mockResolvedValue(undefined)
+
+ await skillsManager.discoverSkills()
+
+ await expect(skillsManager.updateSkillModes("resolution-fails", "global", ["code"])).rejects.toThrow(
+ "EIO: resolution failed",
+ )
+ const temporaryPath = String(mockWriteFile.mock.calls[0][0])
+ expect(mockUnlink).toHaveBeenCalledWith(temporaryPath)
+ expect(mockRename).not.toHaveBeenCalled()
+ })
+
it("refuses to update a SKILL.md that resolves outside the skills roots", async () => {
const skillDir = p(globalSkillsDir, "escaping")
const skillMd = p(skillDir, "SKILL.md")
@@ -4158,6 +4202,18 @@ Instructions`)
expect(createWatcher).not.toHaveBeenCalled()
})
+ it("rejects a queued write once the manager is disposed", async () => {
+ await skillsManager.dispose()
+ mockWriteFile.mockResolvedValue(undefined)
+ mockUnlink.mockResolvedValue(undefined)
+
+ await expect(
+ skillsManager["writeSkillFileAtomically"](p(globalSkillsDir, "SKILL.md"), "content"),
+ ).rejects.toThrow("SkillsManager is disposed")
+ expect(mockWriteFile).not.toHaveBeenCalled()
+ expect(mockRename).not.toHaveBeenCalled()
+ })
+
it("removes tracked temp sidecars on dispose", async () => {
mockUnlink.mockResolvedValue(undefined)
skillsManager["activeTempFiles"].add("/mock/skills/foo/.SKILL.md.123.abc.tmp")
From 38844c7d1a439c97dfe8249ea0c4f003e160c9c5 Mon Sep 17 00:00:00 2001
From: easonLiangWorldedtech
<136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Tue, 27 Oct 2026 06:50:38 +0800
Subject: [PATCH 26/31] test(skills): pin the directory-queue disposal guard
directly
Review follow-up: block one withDirectoryLock operation, queue a second, dispose the manager, then release the first; the queued operation rejects with SkillsManager is disposed and its callback never runs. The end-to-end writeSkillFileAtomically disposal test stays. SkillsManager.spec.ts 129 passed.
---
.../skills/__tests__/SkillsManager.spec.ts | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index ca2f33f4f0..15f5d3db47 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -4214,6 +4214,33 @@ Instructions`)
expect(mockRename).not.toHaveBeenCalled()
})
+ it("never runs a queued directory operation after the manager is disposed", async () => {
+ let releaseFirst!: () => void
+ const firstGate = new Promise((resolve) => {
+ releaseFirst = resolve
+ })
+ let firstStarted = false
+ let secondRan = false
+
+ const first = skillsManager["withDirectoryLock"](globalSkillsDir, async () => {
+ firstStarted = true
+ await firstGate
+ })
+ await vi.waitFor(() => expect(firstStarted).toBe(true))
+
+ // The second operation is queued behind the first and must be rejected after disposal
+ // without running its callback.
+ const second = skillsManager["withDirectoryLock"](globalSkillsDir, async () => {
+ secondRan = true
+ })
+ await skillsManager.dispose()
+ releaseFirst()
+
+ await first
+ await expect(second).rejects.toThrow("SkillsManager is disposed")
+ expect(secondRan).toBe(false)
+ })
+
it("removes tracked temp sidecars on dispose", async () => {
mockUnlink.mockResolvedValue(undefined)
skillsManager["activeTempFiles"].add("/mock/skills/foo/.SKILL.md.123.abc.tmp")
From bc96d5bb79cbd7e11926b8640e991804df3ce23b Mon Sep 17 00:00:00 2001
From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Wed, 4 Nov 2026 03:56:00 +0800
Subject: [PATCH 27/31] fix(skills): move stacked JSDoc blocks and localize the
containment error
- SkillsManager: the createSkill / writeSkillFileAtomically and updateSkillModes /
assertTargetWithinSkillsRoots doc blocks were stacked above the wrong methods;
each block now sits on the method it describes.
- The outside-root rejection is user visible, so it goes through the skills
namespace instead of a hard-coded English string: errors.outside_skills_root
with {{source}} and {{path}}. Added to all 18 shipped locales, following the
same pattern as the symlink-hardening messages, plus a locale test that keeps
the key and its placeholders in every bundle.
---
.../__tests__/skills-error-locales.spec.ts | 33 +++++++++++++++
src/i18n/locales/ca/skills.json | 3 +-
src/i18n/locales/de/skills.json | 3 +-
src/i18n/locales/en/skills.json | 3 +-
src/i18n/locales/es/skills.json | 3 +-
src/i18n/locales/fr/skills.json | 3 +-
src/i18n/locales/hi/skills.json | 3 +-
src/i18n/locales/id/skills.json | 3 +-
src/i18n/locales/it/skills.json | 3 +-
src/i18n/locales/ja/skills.json | 3 +-
src/i18n/locales/ko/skills.json | 3 +-
src/i18n/locales/nl/skills.json | 3 +-
src/i18n/locales/pl/skills.json | 3 +-
src/i18n/locales/pt-BR/skills.json | 3 +-
src/i18n/locales/ru/skills.json | 3 +-
src/i18n/locales/tr/skills.json | 3 +-
src/i18n/locales/vi/skills.json | 3 +-
src/i18n/locales/zh-CN/skills.json | 3 +-
src/i18n/locales/zh-TW/skills.json | 3 +-
src/services/skills/SkillsManager.ts | 42 +++++++++----------
.../skills/__tests__/SkillsManager.spec.ts | 1 +
21 files changed, 91 insertions(+), 39 deletions(-)
create mode 100644 src/i18n/__tests__/skills-error-locales.spec.ts
diff --git a/src/i18n/__tests__/skills-error-locales.spec.ts b/src/i18n/__tests__/skills-error-locales.spec.ts
new file mode 100644
index 0000000000..9a09c91de5
--- /dev/null
+++ b/src/i18n/__tests__/skills-error-locales.spec.ts
@@ -0,0 +1,33 @@
+import fs from "fs"
+import path from "path"
+
+const localesDir = path.join(__dirname, "..", "locales")
+
+const languages = fs
+ .readdirSync(localesDir, { withFileTypes: true })
+ .filter((dirent) => dirent.isDirectory() && !dirent.name.startsWith("."))
+ .map((dirent) => dirent.name)
+
+function loadSkillErrors(language: string): Record {
+ return JSON.parse(fs.readFileSync(path.join(localesDir, language, "skills.json"), "utf8")).errors
+}
+
+describe("skills:errors.outside_skills_root locales", () => {
+ it("covers every shipped locale", () => {
+ expect(languages.length).toBeGreaterThan(0)
+ for (const language of languages) {
+ expect(
+ loadSkillErrors(language).outside_skills_root,
+ `outside_skills_root missing in ${language}`,
+ ).toBeTruthy()
+ }
+ })
+
+ it("keeps the source and path placeholders in every locale", () => {
+ for (const language of languages) {
+ const message = loadSkillErrors(language).outside_skills_root
+ expect(message, `{{source}} placeholder missing in ${language}`).toContain("{{source}}")
+ expect(message, `{{path}} placeholder missing in ${language}`).toContain("{{path}}")
+ }
+ })
+})
diff --git a/src/i18n/locales/ca/skills.json b/src/i18n/locales/ca/skills.json
index 69f87c690c..7053710f59 100644
--- a/src/i18n/locales/ca/skills.json
+++ b/src/i18n/locales/ca/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "No s'ha trobat l'habilitat \"{{name}}\"",
"container_skill_read_only": "L'habilitat \"{{name}}\" prové d'una carpeta d'habilitats enllaçada simbòlicament ({{path}}) i no es pot moure ni eliminar aquí. Canvia-la a la seva carpeta d'origen.",
"symlink_escapes_skill": "No es pot moure l'habilitat: l'enllaç simbòlic {{link}} apunta fora de la carpeta de l'habilitat i es trencaria a la nova ubicació.",
- "move_restore_failed": "No s'ha pogut moure l'skill ({{error}}) i la carpeta original no s'ha pogut restaurar a {{path}}. El teu skill està a salvo a {{backupPath}}; torna'l a moure manualment."
+ "move_restore_failed": "No s'ha pogut moure l'skill ({{error}}) i la carpeta original no s'ha pogut restaurar a {{path}}. El teu skill està a salvo a {{backupPath}}; torna'l a moure manualment.",
+ "outside_skills_root": "No es pot actualitzar SKILL.md fora dels directoris d'habilitats {{source}}: {{path}}"
}
}
diff --git a/src/i18n/locales/de/skills.json b/src/i18n/locales/de/skills.json
index 1093bcc1f0..9ce73e11cf 100644
--- a/src/i18n/locales/de/skills.json
+++ b/src/i18n/locales/de/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Skill \"{{name}}\" nicht gefunden",
"container_skill_read_only": "Skill \"{{name}}\" stammt aus einem per Symlink eingebundenen Skills-Ordner ({{path}}) und kann hier nicht verschoben oder gelöscht werden. Ändere ihn in seinem Quellordner.",
"symlink_escapes_skill": "Skill kann nicht verschoben werden: Der Symlink {{link}} zeigt aus dem Skill-Ordner heraus und würde am neuen Ort nicht mehr funktionieren.",
- "move_restore_failed": "Das Verschieben des Skills ist fehlgeschlagen ({{error}}) und der ursprüngliche Ordner konnte nicht nach {{path}} wiederhergestellt werden. Dein Skill ist in {{backupPath}} sicher; verschiebe ihn manuell zurück."
+ "move_restore_failed": "Das Verschieben des Skills ist fehlgeschlagen ({{error}}) und der ursprüngliche Ordner konnte nicht nach {{path}} wiederhergestellt werden. Dein Skill ist in {{backupPath}} sicher; verschiebe ihn manuell zurück.",
+ "outside_skills_root": "SKILL.md kann nicht außerhalb der {{source}}-Skill-Verzeichnisse aktualisiert werden: {{path}}"
}
}
diff --git a/src/i18n/locales/en/skills.json b/src/i18n/locales/en/skills.json
index 78c708e93e..ea1f96fa59 100644
--- a/src/i18n/locales/en/skills.json
+++ b/src/i18n/locales/en/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Skill \"{{name}}\" not found",
"container_skill_read_only": "Skill \"{{name}}\" comes from a symlinked skills folder ({{path}}) and can't be moved or deleted here. Change it in its source folder.",
"symlink_escapes_skill": "Can't move the skill: the symlink {{link}} points outside the skill folder and would break at the new location.",
- "move_restore_failed": "Moving the skill failed ({{error}}) and its original folder couldn't be restored to {{path}}. Your skill is safe in {{backupPath}}; move it back manually."
+ "move_restore_failed": "Moving the skill failed ({{error}}) and its original folder couldn't be restored to {{path}}. Your skill is safe in {{backupPath}}; move it back manually.",
+ "outside_skills_root": "Refusing to update SKILL.md outside the {{source}} skills directories: {{path}}"
}
}
diff --git a/src/i18n/locales/es/skills.json b/src/i18n/locales/es/skills.json
index cb8a821a00..7129854959 100644
--- a/src/i18n/locales/es/skills.json
+++ b/src/i18n/locales/es/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "No se encontró la habilidad \"{{name}}\"",
"container_skill_read_only": "La habilidad \"{{name}}\" proviene de una carpeta de habilidades enlazada simbólicamente ({{path}}) y no se puede mover ni eliminar aquí. Cámbiala en su carpeta de origen.",
"symlink_escapes_skill": "No se puede mover la habilidad: el enlace simbólico {{link}} apunta fuera de la carpeta de la habilidad y se rompería en la nueva ubicación.",
- "move_restore_failed": "No se pudo mover el skill ({{error}}) y su carpeta original no se pudo restaurar en {{path}}. Tu skill está a salvo en {{backupPath}}; muévelo de vuelta manualmente."
+ "move_restore_failed": "No se pudo mover el skill ({{error}}) y su carpeta original no se pudo restaurar en {{path}}. Tu skill está a salvo en {{backupPath}}; muévelo de vuelta manualmente.",
+ "outside_skills_root": "No se puede actualizar SKILL.md fuera de los directorios de habilidades {{source}}: {{path}}"
}
}
diff --git a/src/i18n/locales/fr/skills.json b/src/i18n/locales/fr/skills.json
index a013c2b1e2..fb4b08d94a 100644
--- a/src/i18n/locales/fr/skills.json
+++ b/src/i18n/locales/fr/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Compétence \"{{name}}\" introuvable",
"container_skill_read_only": "La compétence \"{{name}}\" provient d'un dossier de compétences lié par un lien symbolique ({{path}}) et ne peut pas être déplacée ni supprimée ici. Modifie-la dans son dossier source.",
"symlink_escapes_skill": "Impossible de déplacer la compétence : le lien symbolique {{link}} pointe hors du dossier de la compétence et serait cassé au nouvel emplacement.",
- "move_restore_failed": "Le déplacement du skill a échoué ({{error}}) et son dossier d'origine n'a pas pu être restauré dans {{path}}. Ton skill est en sécurité dans {{backupPath}} ; remets-le en place manuellement."
+ "move_restore_failed": "Le déplacement du skill a échoué ({{error}}) et son dossier d'origine n'a pas pu être restauré dans {{path}}. Ton skill est en sécurité dans {{backupPath}} ; remets-le en place manuellement.",
+ "outside_skills_root": "Impossible de mettre à jour SKILL.md en dehors des dossiers de compétences {{source}} : {{path}}"
}
}
diff --git a/src/i18n/locales/hi/skills.json b/src/i18n/locales/hi/skills.json
index 66e06d81df..9cdc711929 100644
--- a/src/i18n/locales/hi/skills.json
+++ b/src/i18n/locales/hi/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "स्किल \"{{name}}\" नहीं मिला",
"container_skill_read_only": "स्किल \"{{name}}\" एक सिमलिंक किए गए स्किल फ़ोल्डर ({{path}}) से आता है और इसे यहाँ से न तो हटाया जा सकता है और न ही स्थानांतरित किया जा सकता है। इसे इसके मूल फ़ोल्डर में बदलें।",
"symlink_escapes_skill": "स्किल स्थानांतरित नहीं किया जा सकता: सिमलिंक {{link}} स्किल फ़ोल्डर के बाहर इंगित करता है और नए स्थान पर टूट जाएगा।",
- "move_restore_failed": "स्किल को मूव करना विफल रहा ({{error}}) और उसका मूल फ़ोल्डर {{path}} पर वापस नहीं लाया जा सका। तुम्हारी स्किल {{backupPath}} में सुरक्षित है; उसे मैन्युअल रूप से वापस ले जाओ।"
+ "move_restore_failed": "स्किल को मूव करना विफल रहा ({{error}}) और उसका मूल फ़ोल्डर {{path}} पर वापस नहीं लाया जा सका। तुम्हारी स्किल {{backupPath}} में सुरक्षित है; उसे मैन्युअल रूप से वापस ले जाओ।",
+ "outside_skills_root": "{{source}} स्किल निर्देशिकाओं के बाहर SKILL.md को अपडेट करने से मना किया गया: {{path}}"
}
}
diff --git a/src/i18n/locales/id/skills.json b/src/i18n/locales/id/skills.json
index a7616aed1d..aa179c32b1 100644
--- a/src/i18n/locales/id/skills.json
+++ b/src/i18n/locales/id/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Skill \"{{name}}\" tidak ditemukan",
"container_skill_read_only": "Skill \"{{name}}\" berasal dari folder skill yang di-symlink ({{path}}) dan tidak bisa dipindahkan atau dihapus di sini. Ubah di folder sumbernya.",
"symlink_escapes_skill": "Tidak bisa memindahkan skill: symlink {{link}} mengarah ke luar folder skill dan akan rusak di lokasi baru.",
- "move_restore_failed": "Gagal memindahkan skill ({{error}}) dan folder aslinya tidak dapat dipulihkan ke {{path}}. Skill kamu aman di {{backupPath}}; pindahkan kembali secara manual."
+ "move_restore_failed": "Gagal memindahkan skill ({{error}}) dan folder aslinya tidak dapat dipulihkan ke {{path}}. Skill kamu aman di {{backupPath}}; pindahkan kembali secara manual.",
+ "outside_skills_root": "Menolak memperbarui SKILL.md di luar direktori skill {{source}}: {{path}}"
}
}
diff --git a/src/i18n/locales/it/skills.json b/src/i18n/locales/it/skills.json
index e34c1055b5..8a69e28627 100644
--- a/src/i18n/locales/it/skills.json
+++ b/src/i18n/locales/it/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Skill \"{{name}}\" non trovata",
"container_skill_read_only": "La skill \"{{name}}\" proviene da una cartella di skill collegata tramite symlink ({{path}}) e non può essere spostata o eliminata qui. Modificala nella sua cartella di origine.",
"symlink_escapes_skill": "Impossibile spostare la skill: il symlink {{link}} punta fuori dalla cartella della skill e non funzionerebbe nella nuova posizione.",
- "move_restore_failed": "Lo spostamento della skill non è riuscito ({{error}}) e la cartella originale non è stata ripristinata in {{path}}. La tua skill è al sicuro in {{backupPath}}; spostala indietro manualmente."
+ "move_restore_failed": "Lo spostamento della skill non è riuscito ({{error}}) e la cartella originale non è stata ripristinata in {{path}}. La tua skill è al sicuro in {{backupPath}}; spostala indietro manualmente.",
+ "outside_skills_root": "Impossibile aggiornare SKILL.md al di fuori delle cartelle delle skill {{source}}: {{path}}"
}
}
diff --git a/src/i18n/locales/ja/skills.json b/src/i18n/locales/ja/skills.json
index c70cf80959..ab67fd5b1e 100644
--- a/src/i18n/locales/ja/skills.json
+++ b/src/i18n/locales/ja/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "スキル「{{name}}」が見つかりません",
"container_skill_read_only": "スキル「{{name}}」はシンボリックリンクされたスキルフォルダー({{path}})にあるため、ここでは移動も削除もできません。元のフォルダーで変更してください。",
"symlink_escapes_skill": "スキルを移動できません:シンボリックリンク {{link}} がスキルフォルダーの外を指しているため、新しい場所では壊れてしまいます。",
- "move_restore_failed": "スキルの移動に失敗し({{error}})、元のフォルダーを {{path}} に復元できませんでした。スキルは {{backupPath}} に安全に保存されています。手動で元に戻してください。"
+ "move_restore_failed": "スキルの移動に失敗し({{error}})、元のフォルダーを {{path}} に復元できませんでした。スキルは {{backupPath}} に安全に保存されています。手動で元に戻してください。",
+ "outside_skills_root": "{{source}} スキルディレクトリの外にある SKILL.md は更新できません:{{path}}"
}
}
diff --git a/src/i18n/locales/ko/skills.json b/src/i18n/locales/ko/skills.json
index ee78643b2b..bef5bdff56 100644
--- a/src/i18n/locales/ko/skills.json
+++ b/src/i18n/locales/ko/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "스킬 \"{{name}}\"을(를) 찾을 수 없습니다",
"container_skill_read_only": "스킬 \"{{name}}\"은(는) 심볼릭 링크된 스킬 폴더({{path}})에 있어 여기서 이동하거나 삭제할 수 없습니다. 원본 폴더에서 변경하세요.",
"symlink_escapes_skill": "스킬을 이동할 수 없습니다: 심볼릭 링크 {{link}}이(가) 스킬 폴더 바깥을 가리키므로 새 위치에서 깨집니다.",
- "move_restore_failed": "스킬 이동에 실패했고({{error}}) 원래 폴더를 {{path}}(으)로 복원하지 못했습니다. 스킬은 {{backupPath}}에 안전하게 있으니 직접 다시 옮겨 주세요."
+ "move_restore_failed": "스킬 이동에 실패했고({{error}}) 원래 폴더를 {{path}}(으)로 복원하지 못했습니다. 스킬은 {{backupPath}}에 안전하게 있으니 직접 다시 옮겨 주세요.",
+ "outside_skills_root": "{{source}} 스킬 디렉터리 밖의 SKILL.md는 업데이트할 수 없습니다: {{path}}"
}
}
diff --git a/src/i18n/locales/nl/skills.json b/src/i18n/locales/nl/skills.json
index 05a17f9768..6609ba44b8 100644
--- a/src/i18n/locales/nl/skills.json
+++ b/src/i18n/locales/nl/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Vaardigheid \"{{name}}\" niet gevonden",
"container_skill_read_only": "Vaardigheid \"{{name}}\" komt uit een via symlink gekoppelde vaardighedenmap ({{path}}) en kan hier niet worden verplaatst of verwijderd. Wijzig hem in de bronmap.",
"symlink_escapes_skill": "Kan de vaardigheid niet verplaatsen: de symlink {{link}} wijst naar buiten de vaardighedenmap en zou op de nieuwe locatie niet meer werken.",
- "move_restore_failed": "Het verplaatsen van de skill is mislukt ({{error}}) en de oorspronkelijke map kon niet worden hersteld naar {{path}}. Je skill staat veilig in {{backupPath}}; zet hem handmatig terug."
+ "move_restore_failed": "Het verplaatsen van de skill is mislukt ({{error}}) en de oorspronkelijke map kon niet worden hersteld naar {{path}}. Je skill staat veilig in {{backupPath}}; zet hem handmatig terug.",
+ "outside_skills_root": "Kan SKILL.md niet bijwerken buiten de {{source}}-vaardighedensmappen: {{path}}"
}
}
diff --git a/src/i18n/locales/pl/skills.json b/src/i18n/locales/pl/skills.json
index 0e583164b5..1c506e8671 100644
--- a/src/i18n/locales/pl/skills.json
+++ b/src/i18n/locales/pl/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Nie znaleziono umiejętności \"{{name}}\"",
"container_skill_read_only": "Umiejętność \"{{name}}\" pochodzi z folderu umiejętności podłączonego dowiązaniem symbolicznym ({{path}}) i nie można jej tutaj przenieść ani usunąć. Zmień ją w folderze źródłowym.",
"symlink_escapes_skill": "Nie można przenieść umiejętności: dowiązanie symboliczne {{link}} wskazuje poza folder umiejętności i przestałoby działać w nowej lokalizacji.",
- "move_restore_failed": "Przeniesienie skilla nie powiodło się ({{error}}), a oryginalnego folderu nie udało się przywrócić do {{path}}. Twój skill jest bezpieczny w {{backupPath}}; przenieś go z powrotem ręcznie."
+ "move_restore_failed": "Przeniesienie skilla nie powiodło się ({{error}}), a oryginalnego folderu nie udało się przywrócić do {{path}}. Twój skill jest bezpieczny w {{backupPath}}; przenieś go z powrotem ręcznie.",
+ "outside_skills_root": "Nie można zaktualizować SKILL.md poza katalogami umiejętności {{source}}: {{path}}"
}
}
diff --git a/src/i18n/locales/pt-BR/skills.json b/src/i18n/locales/pt-BR/skills.json
index 1a838e6bf3..a9d143b2c6 100644
--- a/src/i18n/locales/pt-BR/skills.json
+++ b/src/i18n/locales/pt-BR/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Habilidade \"{{name}}\" não encontrada",
"container_skill_read_only": "A habilidade \"{{name}}\" vem de uma pasta de habilidades vinculada por link simbólico ({{path}}) e não pode ser movida nem excluída aqui. Altere-a na pasta de origem.",
"symlink_escapes_skill": "Não é possível mover a habilidade: o link simbólico {{link}} aponta para fora da pasta da habilidade e quebraria no novo local.",
- "move_restore_failed": "Falha ao mover a skill ({{error}}) e a pasta original não pôde ser restaurada em {{path}}. Sua skill está segura em {{backupPath}}; mova-a de volta manualmente."
+ "move_restore_failed": "Falha ao mover a skill ({{error}}) e a pasta original não pôde ser restaurada em {{path}}. Sua skill está segura em {{backupPath}}; mova-a de volta manualmente.",
+ "outside_skills_root": "Não é possível atualizar o SKILL.md fora das pastas de habilidades {{source}}: {{path}}"
}
}
diff --git a/src/i18n/locales/ru/skills.json b/src/i18n/locales/ru/skills.json
index 18ceff9470..f4cdcead37 100644
--- a/src/i18n/locales/ru/skills.json
+++ b/src/i18n/locales/ru/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Навык \"{{name}}\" не найден",
"container_skill_read_only": "Навык \"{{name}}\" находится в папке навыков, подключённой через символическую ссылку ({{path}}), и его нельзя переместить или удалить здесь. Измени его в исходной папке.",
"symlink_escapes_skill": "Не удалось переместить навык: символическая ссылка {{link}} указывает за пределы папки навыка и сломается в новом месте.",
- "move_restore_failed": "Не удалось переместить навык ({{error}}), и исходную папку не получилось восстановить в {{path}}. Твой навык в безопасности в {{backupPath}}; верни его вручную."
+ "move_restore_failed": "Не удалось переместить навык ({{error}}), и исходную папку не получилось восстановить в {{path}}. Твой навык в безопасности в {{backupPath}}; верни его вручную.",
+ "outside_skills_root": "Нельзя обновить SKILL.md вне каталогов навыков {{source}}: {{path}}"
}
}
diff --git a/src/i18n/locales/tr/skills.json b/src/i18n/locales/tr/skills.json
index 0235bd8c19..6b5f4c1441 100644
--- a/src/i18n/locales/tr/skills.json
+++ b/src/i18n/locales/tr/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "\"{{name}}\" becerisi bulunamadı",
"container_skill_read_only": "\"{{name}}\" becerisi sembolik bağlantılı bir beceri klasöründen ({{path}}) geliyor ve burada taşınamaz veya silinemez. Kaynak klasöründe değiştir.",
"symlink_escapes_skill": "Beceri taşınamıyor: {{link}} sembolik bağlantısı beceri klasörünün dışını gösteriyor ve yeni konumda bozulur.",
- "move_restore_failed": "Skill taşınamadı ({{error}}) ve orijinal klasörü {{path}} konumuna geri yüklenemedi. Skill'in {{backupPath}} içinde güvende; onu elle geri taşı."
+ "move_restore_failed": "Skill taşınamadı ({{error}}) ve orijinal klasörü {{path}} konumuna geri yüklenemedi. Skill'in {{backupPath}} içinde güvende; onu elle geri taşı.",
+ "outside_skills_root": "{{source}} beceri klasörlerinin dışındaki SKILL.md güncellenemez: {{path}}"
}
}
diff --git a/src/i18n/locales/vi/skills.json b/src/i18n/locales/vi/skills.json
index 8c15253dbb..aaf358bcf3 100644
--- a/src/i18n/locales/vi/skills.json
+++ b/src/i18n/locales/vi/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "Không tìm thấy kỹ năng \"{{name}}\"",
"container_skill_read_only": "Kỹ năng \"{{name}}\" đến từ một thư mục kỹ năng được liên kết tượng trưng ({{path}}) nên không thể di chuyển hoặc xóa ở đây. Hãy thay đổi nó trong thư mục nguồn.",
"symlink_escapes_skill": "Không thể di chuyển kỹ năng: liên kết tượng trưng {{link}} trỏ ra ngoài thư mục kỹ năng và sẽ bị hỏng ở vị trí mới.",
- "move_restore_failed": "Không thể di chuyển skill ({{error}}) và không thể khôi phục thư mục gốc về {{path}}. Skill của bạn vẫn an toàn trong {{backupPath}}; hãy di chuyển lại thủ công."
+ "move_restore_failed": "Không thể di chuyển skill ({{error}}) và không thể khôi phục thư mục gốc về {{path}}. Skill của bạn vẫn an toàn trong {{backupPath}}; hãy di chuyển lại thủ công.",
+ "outside_skills_root": "Không thể cập nhật SKILL.md nằm ngoài các thư mục kỹ năng {{source}}: {{path}}"
}
}
diff --git a/src/i18n/locales/zh-CN/skills.json b/src/i18n/locales/zh-CN/skills.json
index 4ae1d9f960..c7a7954650 100644
--- a/src/i18n/locales/zh-CN/skills.json
+++ b/src/i18n/locales/zh-CN/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "未找到技能 \"{{name}}\"",
"container_skill_read_only": "技能 \"{{name}}\" 来自通过符号链接引入的技能文件夹({{path}}),无法在此移动或删除。请在其源文件夹中修改。",
"symlink_escapes_skill": "无法移动技能:符号链接 {{link}} 指向技能文件夹之外,移动后会失效。",
- "move_restore_failed": "移动技能失败({{error}}),且无法将原文件夹恢复到 {{path}}。你的技能安全保存在 {{backupPath}} 中,请手动将其移回。"
+ "move_restore_failed": "移动技能失败({{error}}),且无法将原文件夹恢复到 {{path}}。你的技能安全保存在 {{backupPath}} 中,请手动将其移回。",
+ "outside_skills_root": "无法更新位于 {{source}} 技能目录之外的 SKILL.md:{{path}}"
}
}
diff --git a/src/i18n/locales/zh-TW/skills.json b/src/i18n/locales/zh-TW/skills.json
index 9dd272f5a0..1c551df290 100644
--- a/src/i18n/locales/zh-TW/skills.json
+++ b/src/i18n/locales/zh-TW/skills.json
@@ -14,6 +14,7 @@
"skill_not_found": "找不到技能「{{name}}」",
"container_skill_read_only": "技能「{{name}}」來自透過符號連結引入的技能資料夾({{path}}),無法在此移動或刪除。請在其來源資料夾中修改。",
"symlink_escapes_skill": "無法移動技能:符號連結 {{link}} 指向技能資料夾之外,移動後會失效。",
- "move_restore_failed": "移動技能失敗({{error}}),且無法將原資料夾還原到 {{path}}。你的技能安全保存在 {{backupPath}} 中,請手動將其移回。"
+ "move_restore_failed": "移動技能失敗({{error}}),且無法將原資料夾還原到 {{path}}。你的技能安全保存在 {{backupPath}} 中,請手動將其移回。",
+ "outside_skills_root": "無法更新位於 {{source}} 技能目錄之外的 SKILL.md:{{path}}"
}
}
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index 418a13e42e..bbdf3276d8 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -724,20 +724,6 @@ export class SkillsManager {
}
}
- /**
- * Create a new skill
- * @param name - Skill name (must be valid per agentskills.io spec)
- * @param source - "global" or "project"
- * @param description - Skill description
- * @param modeSlugs - Optional mode restrictions (undefined/empty = any mode)
- * @returns Path to created SKILL.md file
- */
- /**
- * Write one SKILL.md atomically: stage the content in a same-directory temp file,
- * await the complete write, then rename it over the target. A crash or failed write
- * must not leave a truncated SKILL.md that the loader would silently misparse.
- * The temp file is removed when the write or rename fails.
- */
private async withDirectoryLock(directory: string, operation: () => Promise): Promise {
// Normalize through realpath first so a symlinked alias shares the same queue key; fall
// back to the resolved path when the directory cannot be resolved (e.g. not created yet).
@@ -762,6 +748,12 @@ export class SkillsManager {
}
}
+ /**
+ * Write one SKILL.md atomically: stage the content in a same-directory temp file,
+ * await the complete write, then rename it over the target. A crash or failed write
+ * must not leave a truncated SKILL.md that the loader would silently misparse.
+ * The temp file is removed when the write or rename fails.
+ */
private async writeSkillFileAtomically(
filePath: string,
content: string,
@@ -803,6 +795,14 @@ export class SkillsManager {
})
}
+ /**
+ * Create a new skill
+ * @param name - Skill name (must be valid per agentskills.io spec)
+ * @param source - "global" or "project"
+ * @param description - Skill description
+ * @param modeSlugs - Optional mode restrictions (undefined/empty = any mode)
+ * @returns Path to created SKILL.md file
+ */
async createSkill(
name: string,
source: "global" | "project",
@@ -1174,12 +1174,6 @@ Add your skill instructions here.
return undefined
}
- /**
- * Update the mode associations for a skill by modifying its SKILL.md frontmatter.
- * @param name - Skill name
- * @param source - Where the skill is located ("global" or "project")
- * @param newModeSlugs - New mode slugs (undefined/empty = any mode)
- */
/**
* The resolved SKILL.md target must stay inside one of the configured skills roots for the
* source: a direct skill-directory symlink can point anywhere, and publishing through it
@@ -1197,9 +1191,15 @@ Add your skill instructions here.
for (const root of roots) {
if (await inside(root)) return
}
- throw new Error(`Refusing to update SKILL.md outside the ${source} skills directories: ${targetPath}`)
+ throw new Error(t("skills:errors.outside_skills_root", { source, path: targetPath }))
}
+ /**
+ * Update the mode associations for a skill by modifying its SKILL.md frontmatter.
+ * @param name - Skill name
+ * @param source - Where the skill is located ("global" or "project")
+ * @param newModeSlugs - New mode slugs (undefined/empty = any mode)
+ */
async updateSkillModes(name: string, source: "global" | "project", newModeSlugs?: string[]): Promise {
// Find any skill with this name and source (regardless of current mode)
let skill: SkillMetadata | undefined
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index 15f5d3db47..20b91ae3ec 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -147,6 +147,7 @@ vi.mock("../../../i18n", () => ({
"skills:errors.already_exists": `Skill "${params?.name}" already exists at ${params?.path}`,
"skills:errors.not_found": `Skill "${params?.name}" not found in ${params?.source}${params?.modeInfo}`,
"skills:errors.move_restore_failed": `Moving the skill failed (${params?.error}) and its original folder couldn't be restored to ${params?.path}. Your skill is safe in ${params?.backupPath}; move it back manually.`,
+ "skills:errors.outside_skills_root": `Refusing to update SKILL.md outside the ${params?.source} skills directories: ${params?.path}`,
}
return translations[key] || key
},
From 0ee05df6d824350686423ab83d9ff09961a61fb1 Mon Sep 17 00:00:00 2001
From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Wed, 4 Nov 2026 04:17:26 +0800
Subject: [PATCH 28/31] fix(skills): keep SKILL.md mode bits through the atomic
rewrite and fix the zh-CN delay label
- writeSkillFileAtomically: a rename replaces the target inode, so the replacement
got the process default mode. A user-restricted SKILL.md (0600) came back readable
by other local users after an unrelated mode-restriction edit. The existing target's
mode is read right before the rename and applied to the staged file with chmod.
- zh-CN autoApprove.write.delayLabel described a pre-write inspection window; the
delay applies after the write so diagnostics can run. Aligned with the en/zh-TW
wording, with a locale test pinning the post-write phrasing.
---
src/services/skills/SkillsManager.ts | 8 ++++
.../skills/__tests__/SkillsManager.spec.ts | 47 +++++++++++++++++++
.../i18n/__tests__/write-delay-labels.spec.ts | 21 +++++++++
.../src/i18n/locales/zh-CN/settings.json | 2 +-
4 files changed, 77 insertions(+), 1 deletion(-)
create mode 100644 webview-ui/src/i18n/__tests__/write-delay-labels.spec.ts
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index bbdf3276d8..c43fd7d618 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -779,6 +779,14 @@ export class SkillsManager {
// ancestor) can change between the caller's resolution and this publish.
await validateTarget?.(filePath)
if (this.isDisposed) throw new Error("SkillsManager is disposed")
+ // rename replaces the target's inode, so the replacement must carry the target's
+ // permission bits: a user-restricted SKILL.md (e.g. 0600) would otherwise come back
+ // with the process default mode and become readable by other local users. A target
+ // that does not exist yet (createSkill) has no mode to preserve and keeps the default.
+ const existingTarget = await fs.stat(filePath).catch(() => undefined)
+ if (existingTarget && typeof existingTarget.mode === "number") {
+ await fs.chmod(temporaryPath, existingTarget.mode & 0o777)
+ }
await fs.rename(temporaryPath, filePath)
} catch (error) {
// Best effort: a partial temp file must not be left behind, but an EEXIST failure
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index 20b91ae3ec..28b7a26b07 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -20,6 +20,7 @@ const {
mockLstat,
mockSymlink,
mockUnlink,
+ mockChmod,
} = vi.hoisted(() => ({
mockReadlink: vi.fn(),
mockCp: vi.fn(),
@@ -38,6 +39,7 @@ const {
mockRmdir: vi.fn(),
mockSymlink: vi.fn(),
mockUnlink: vi.fn(),
+ mockChmod: vi.fn(),
}))
// Platform-agnostic test paths
@@ -73,6 +75,7 @@ vi.mock("fs/promises", () => ({
readlink: mockReadlink,
symlink: mockSymlink,
unlink: mockUnlink,
+ chmod: mockChmod,
},
stat: mockStat,
readFile: mockReadFile,
@@ -88,6 +91,7 @@ vi.mock("fs/promises", () => ({
readlink: mockReadlink,
symlink: mockSymlink,
unlink: mockUnlink,
+ chmod: mockChmod,
}))
// Mock os module
@@ -2573,6 +2577,49 @@ Body here.`
expect(parsed2.data.description).toBe(description)
})
+ it("preserves the existing SKILL.md permission bits through the atomic rewrite", async () => {
+ // rename replaces the target's inode: without an explicit chmod the new file gets
+ // the process default mode, so a user-restricted SKILL.md (0600) would come back
+ // readable by other local users after an unrelated mode-restriction edit.
+ const skillDir = p(globalSkillsDir, "mode-preserved-skill")
+ const skillMd = p(skillDir, "SKILL.md")
+ const originalContent = `---
+name: mode-preserved-skill
+description: 'Mode preserved'
+---
+
+Body.`
+
+ mockDirectoryExists.mockImplementation(async (dir: string) => dir === globalSkillsDir)
+ mockRealpath.mockImplementation(async (pathArg: string) => pathArg)
+ mockReaddir.mockImplementation(async (dir: string) =>
+ dir === globalSkillsDir ? ["mode-preserved-skill"] : [],
+ )
+ mockStat.mockImplementation(async (pathArg: string) => {
+ if (pathArg === skillDir) {
+ return { isDirectory: () => true }
+ }
+ if (pathArg === skillMd) {
+ // 0600 regular file: the mode the rewrite must keep.
+ return { isDirectory: () => false, mode: 0o100600 }
+ }
+ throw new Error("Not found")
+ })
+ mockFileExists.mockImplementation(async (file: string) => file === skillMd)
+ mockReadFile.mockResolvedValue(originalContent)
+ mockWriteFile.mockResolvedValue(undefined)
+
+ await skillsManager.discoverSkills()
+ expect(skillsManager.getAllSkills()).toHaveLength(1)
+ await skillsManager.updateSkillModes("mode-preserved-skill", "global", ["code"])
+
+ const temporaryPath = mockWriteFile.mock.calls[0][0] as string
+ expect(mockChmod).toHaveBeenCalledWith(temporaryPath, 0o600)
+ // The mode has to be in place before the rename publishes the file.
+ expect(mockChmod.mock.invocationCallOrder[0]).toBeLessThan(mockRename.mock.invocationCallOrder[0])
+ expect(mockRename).toHaveBeenCalledWith(temporaryPath, skillMd)
+ })
+
it("propagates a realpath failure instead of writing through the unresolved path", async () => {
const skillDir = p(globalSkillsDir, "unresolvable-skill")
const skillMd = p(skillDir, "SKILL.md")
diff --git a/webview-ui/src/i18n/__tests__/write-delay-labels.spec.ts b/webview-ui/src/i18n/__tests__/write-delay-labels.spec.ts
new file mode 100644
index 0000000000..569dd2e563
--- /dev/null
+++ b/webview-ui/src/i18n/__tests__/write-delay-labels.spec.ts
@@ -0,0 +1,21 @@
+import en from "../locales/en/settings.json"
+import zhCn from "../locales/zh-CN/settings.json"
+import zhTw from "../locales/zh-TW/settings.json"
+
+const delayLabel = (bundle: { autoApprove: { write: { delayLabel: string } } }): string =>
+ bundle.autoApprove.write.delayLabel
+
+describe("autoApprove.write.delayLabel locales", () => {
+ it("describes a delay AFTER the write, not an inspection window before it", () => {
+ // The setting delays the write-completion path so diagnostics can run after the file is
+ // on disk. A label that promises a window before approval describes behaviour the code
+ // does not have, which is what the zh-CN bundle used to say.
+ expect(delayLabel(en)).toContain("after writes")
+ expect(delayLabel(zhTw)).toContain("寫入後")
+ expect(delayLabel(zhCn)).toContain("写入后")
+ })
+
+ it("keeps the stale pre-write phrasing out of the zh-CN bundle", () => {
+ expect(delayLabel(zhCn)).not.toContain("再自动批准写入")
+ })
+})
diff --git a/webview-ui/src/i18n/locales/zh-CN/settings.json b/webview-ui/src/i18n/locales/zh-CN/settings.json
index e80c2bf79d..a1e255eda4 100644
--- a/webview-ui/src/i18n/locales/zh-CN/settings.json
+++ b/webview-ui/src/i18n/locales/zh-CN/settings.json
@@ -298,7 +298,7 @@
"write": {
"label": "写入",
"description": "自动创建和编辑文件,无需二次确认",
- "delayLabel": "延迟一段时间再自动批准写入,可以在期间检查模型输出是否有问题",
+ "delayLabel": "写入后延迟一小段时间,以便诊断工具检测潜在问题",
"outsideWorkspace": {
"label": "包含工作区外的文件",
"description": "允许 Zoo 创建和编辑当前工作区外的文件,无需批准。"
From fe5498bb678e49989475d70dee6dca48f10cd2a2 Mon Sep 17 00:00:00 2001
From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Wed, 4 Nov 2026 05:20:23 +0800
Subject: [PATCH 29/31] fix(skills): fail closed when the target's mode cannot
be read
The atomic rewrite treated every fs.stat failure as a missing target, so a coded failure (EACCES,
EIO, EPERM) skipped the chmod and renamed the temp file anyway - replacing a user-restricted
0600 SKILL.md with a file carrying the process default mode. Only ENOENT (and an error carrying
no filesystem code at all) now falls through to the default mode; everything else fails closed
and the existing cleanup removes the temp file.
New test `refuses the atomic rewrite when the target's mode cannot be read` (EACCES from stat must
reject and never reach chmod). Control: restoring `.catch(() => undefined)` fails it.
SkillsManager.spec 131 passed, tsc --noEmit 0 errors.
---
src/services/skills/SkillsManager.ts | 16 ++++++++-
.../skills/__tests__/SkillsManager.spec.ts | 35 +++++++++++++++++++
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts
index c43fd7d618..d1c46a1235 100644
--- a/src/services/skills/SkillsManager.ts
+++ b/src/services/skills/SkillsManager.ts
@@ -783,7 +783,21 @@ export class SkillsManager {
// permission bits: a user-restricted SKILL.md (e.g. 0600) would otherwise come back
// with the process default mode and become readable by other local users. A target
// that does not exist yet (createSkill) has no mode to preserve and keeps the default.
- const existingTarget = await fs.stat(filePath).catch(() => undefined)
+ // Only a missing target is allowed to fall through to the default mode: any other stat
+ // failure means the mode could not be read, and renaming anyway would replace a
+ // restricted SKILL.md with a file carrying the process default mode.
+ const existingTarget = await fs.stat(filePath).catch((error: unknown) => {
+ const code =
+ typeof error === "object" && error !== null ? (error as { code?: unknown }).code : undefined
+ // A missing target (or an error that carries no filesystem code at all) has no mode to
+ // preserve. Any coded failure - EACCES, EIO, EPERM - means the mode could not be read,
+ // and renaming anyway would replace a restricted SKILL.md with a file that carries the
+ // process default mode, so it fails closed.
+ if (code === "ENOENT" || typeof code !== "string") {
+ return undefined
+ }
+ throw error
+ })
if (existingTarget && typeof existingTarget.mode === "number") {
await fs.chmod(temporaryPath, existingTarget.mode & 0o777)
}
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index 28b7a26b07..07f2a8a56b 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -2577,6 +2577,41 @@ Body here.`
expect(parsed2.data.description).toBe(description)
})
+ it("refuses the atomic rewrite when the target's mode cannot be read", async () => {
+ const skillDir = p(globalSkillsDir, "mode-unreadable-skill")
+ const skillMd = p(skillDir, "SKILL.md")
+ const originalContent = `---
+ name: mode-unreadable-skill
+ description: 'Mode unreadable'
+ ---
+
+ Body.`
+ mockDirectoryExists.mockImplementation(async (dir: string) => dir === globalSkillsDir)
+ mockRealpath.mockImplementation(async (pathArg: string) => pathArg)
+ mockReaddir.mockImplementation(async (dir: string) =>
+ dir === globalSkillsDir ? ["mode-unreadable-skill"] : [],
+ )
+ mockReadFile.mockResolvedValue(originalContent)
+ mockStat.mockImplementation(async (pathArg: string) => {
+ if (pathArg === skillDir) {
+ return { isDirectory: () => true }
+ }
+ if (pathArg === skillMd) {
+ // A coded failure other than ENOENT: the mode cannot be read, so the rewrite must not
+ // replace a possibly restricted file with one carrying the default mode.
+ const error = new Error("EACCES: permission denied") as Error & { code: string }
+ error.code = "EACCES"
+ throw error
+ }
+ const missing = new Error("ENOENT: no such file") as Error & { code: string }
+ missing.code = "ENOENT"
+ throw missing
+ })
+
+ await expect(skillsManager.updateSkillModes("mode-unreadable-skill", "global", ["code"])).rejects.toThrow()
+ expect(mockChmod).not.toHaveBeenCalled()
+ })
+
it("preserves the existing SKILL.md permission bits through the atomic rewrite", async () => {
// rename replaces the target's inode: without an explicit chmod the new file gets
// the process default mode, so a user-restricted SKILL.md (0600) would come back
From f166724bb6c6e864e2f6a2459e0ccf29c4997d4f Mon Sep 17 00:00:00 2001
From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Wed, 4 Nov 2026 05:21:59 +0800
Subject: [PATCH 30/31] test(skills): remove the mode-guard test that never
reached the guard
fe5498bb6 claimed its new test covered the fail-closed stat guard. Tightening the assertion showed
the fixture never reaches it: the rewrite rejects with 'Skill ... not found' before the stat call,
both with and without the guard. The test therefore proved nothing and is removed; the guard stays
and is honestly unproven.
SkillsManager.spec 130 passed, tsc --noEmit 0 errors, eslint --max-warnings=0 exit 0.
---
.../skills/__tests__/SkillsManager.spec.ts | 35 -------------------
1 file changed, 35 deletions(-)
diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts
index 07f2a8a56b..28b7a26b07 100644
--- a/src/services/skills/__tests__/SkillsManager.spec.ts
+++ b/src/services/skills/__tests__/SkillsManager.spec.ts
@@ -2577,41 +2577,6 @@ Body here.`
expect(parsed2.data.description).toBe(description)
})
- it("refuses the atomic rewrite when the target's mode cannot be read", async () => {
- const skillDir = p(globalSkillsDir, "mode-unreadable-skill")
- const skillMd = p(skillDir, "SKILL.md")
- const originalContent = `---
- name: mode-unreadable-skill
- description: 'Mode unreadable'
- ---
-
- Body.`
- mockDirectoryExists.mockImplementation(async (dir: string) => dir === globalSkillsDir)
- mockRealpath.mockImplementation(async (pathArg: string) => pathArg)
- mockReaddir.mockImplementation(async (dir: string) =>
- dir === globalSkillsDir ? ["mode-unreadable-skill"] : [],
- )
- mockReadFile.mockResolvedValue(originalContent)
- mockStat.mockImplementation(async (pathArg: string) => {
- if (pathArg === skillDir) {
- return { isDirectory: () => true }
- }
- if (pathArg === skillMd) {
- // A coded failure other than ENOENT: the mode cannot be read, so the rewrite must not
- // replace a possibly restricted file with one carrying the default mode.
- const error = new Error("EACCES: permission denied") as Error & { code: string }
- error.code = "EACCES"
- throw error
- }
- const missing = new Error("ENOENT: no such file") as Error & { code: string }
- missing.code = "ENOENT"
- throw missing
- })
-
- await expect(skillsManager.updateSkillModes("mode-unreadable-skill", "global", ["code"])).rejects.toThrow()
- expect(mockChmod).not.toHaveBeenCalled()
- })
-
it("preserves the existing SKILL.md permission bits through the atomic rewrite", async () => {
// rename replaces the target's inode: without an explicit chmod the new file gets
// the process default mode, so a user-restricted SKILL.md (0600) would come back
From 2a25a2b9f9d65a6fb0f738d0299e91aead871812 Mon Sep 17 00:00:00 2001
From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com>
Date: Fri, 6 Nov 2026 00:36:25 +0800
Subject: [PATCH 31/31] fix(i18n): correct the Dutch prompt-caching control and
the Turkish Flex latency note
The Dutch control said 'enable prompt caching' while its English source is 'Disable prompt caching' and the
description now says that checking it turns caching off, and the Turkish Flex tooltip promised lower
latency where the English note says Flex trades higher latency for the lower price.
---
webview-ui/src/i18n/locales/nl/settings.json | 4 ++--
webview-ui/src/i18n/locales/tr/settings.json | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/webview-ui/src/i18n/locales/nl/settings.json b/webview-ui/src/i18n/locales/nl/settings.json
index 8c918152c8..bc05f5748d 100644
--- a/webview-ui/src/i18n/locales/nl/settings.json
+++ b/webview-ui/src/i18n/locales/nl/settings.json
@@ -1272,8 +1272,8 @@
}
},
"promptCaching": {
- "label": "Prompt caching inschakelen",
- "description": "Indien ingeschakeld, gebruikt Zoo dit model met prompt caching om kosten te verlagen."
+ "label": "Prompt caching uitschakelen",
+ "description": "Indien aangevinkt, gebruikt Zoo geen prompt caching voor dit model."
},
"limitMaxTokensDescription": "Beperk het maximale aantal tokens in het antwoord",
"maxOutputTokensLabel": "Maximale output tokens",
diff --git a/webview-ui/src/i18n/locales/tr/settings.json b/webview-ui/src/i18n/locales/tr/settings.json
index a3801e32c2..ab09e3f1fc 100644
--- a/webview-ui/src/i18n/locales/tr/settings.json
+++ b/webview-ui/src/i18n/locales/tr/settings.json
@@ -1149,7 +1149,7 @@
"cacheReads": "Önbellek okumaları"
},
"label": "Hizmet seviyesi",
- "tooltip": "Daha hızlı API isteği işleme için öncelikli işleme hizmeti seviyesini deneyin. Daha düşük gecikme süresiyle daha düşük fiyatlar için esnek işleme seviyesini deneyin."
+ "tooltip": "Daha hızlı API isteği işleme için öncelikli işleme hizmeti seviyesini deneyin. Daha yüksek gecikme süresiyle daha düşük fiyatlar için esnek işleme seviyesini deneyin."
},
"ui": {
"collapseThinking": {