From e3ade22a66fd4fd7aafe1dc66a12dbfa851a57ab Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Mon, 12 Oct 2026 11:52:17 +0100 Subject: [PATCH 01/13] HYPERFLEET-1762 - docs: repository ownership document, OWNERS generator and review rules Adds hyperfleet/docs/owners.md with the area table, the merge flow and the reviewer and approver criteria, generated from hyperfleet/docs/owners.yaml by hack/owners/gen_owners.py. The generator also renders OWNERS_ALIASES and OWNERS files for every repository; hack/owners/labels.sh creates the area labels and hack/owners/open_prs.sh opens the per-repository OWNERS PRs. Updates the working agreement with the reviewer and approver criteria, the area-approver merge rule, review turnaround, new commits instead of force-push during review, and squash on merge. README, CONTRIBUTING and the commit standard point at the ownership document. Co-Authored-By: Claude Fable 5.1 --- .gitignore | 2 + CONTRIBUTING.md | 10 +- Makefile | 8 + README.md | 18 +- .../__pycache__/gen_owners.cpython-314.pyc | Bin 0 -> 12759 bytes hack/owners/gen_owners.py | 197 ++++++++++++ hack/owners/labels.sh | 50 +++ hack/owners/open_prs.sh | 81 +++++ hyperfleet/docs/owners.md | 167 ++++++++++ hyperfleet/docs/owners.yaml | 304 ++++++++++++++++++ hyperfleet/docs/working-agreement.md | 60 +++- hyperfleet/standards/commit-standard.md | 4 +- 12 files changed, 870 insertions(+), 31 deletions(-) create mode 100644 .gitignore create mode 100644 hack/owners/__pycache__/gen_owners.cpython-314.pyc create mode 100755 hack/owners/gen_owners.py create mode 100755 hack/owners/labels.sh create mode 100755 hack/owners/open_prs.sh create mode 100644 hyperfleet/docs/owners.md create mode 100644 hyperfleet/docs/owners.yaml diff --git a/.gitignore b/.gitignore new file mode 100644 index 00000000..652fb3b9 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ + +hack/owners/out/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 259c40a7..f3c3ba23 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,7 +1,7 @@ --- Status: Active Owner: HyperFleet Architecture Team -Last Updated: 2026-03-25 +Last Updated: 2026-10-12 --- # Contributing to HyperFleet Architecture @@ -111,15 +111,13 @@ Use the scripts in `hack/` to run linting locally before pushing: 5. **Open a PR** with a description that includes: - **What changed**: Which documents were added or updated - **Why**: The architectural context or decision being documented - - **Reviewers to loop in**: Tag any component owners affected by the change + - **Reviewers to loop in**: Prow requests reviewers from the area's `OWNERS` pool automatically; tag anyone else who should see it 6. **Post the PR link** in [#hcm-hyperfleet-team](https://redhat.enterprise.slack.com/archives/hcm-hyperfleet-team) for team visibility -7. **Wait 24 hours** for peer review (accounts for time zone differences between regions) - - For urgent changes, use judgement but ensure the rationale is well-documented in the PR - - For major architectural changes, strongly consider waiting for at least one Technical Leader review +7. **Review**: reviewers aim for a substantive review within one business day. Push fixes as new commits once review has started -8. **Merge** once approved with no objections +8. **Merge**: an area approver's `/lgtm` or approving review merges the PR once CI is green. See [Repository Ownership](hyperfleet/docs/owners.md) for who approves which area. Want to become a reviewer or approver in an area? The criteria are in that document and apply to everyone --- diff --git a/Makefile b/Makefile index 492365f6..13345258 100644 --- a/Makefile +++ b/Makefile @@ -5,3 +5,11 @@ help: ## Show available targets .PHONY: install-hooks install-hooks: ## Install pre-commit hooks pre-commit install + +.PHONY: owners +owners: ## Render OWNERS files and the ownership table from hyperfleet/docs/owners.yaml + python3 hack/owners/gen_owners.py render + +.PHONY: verify-owners +verify-owners: ## Check hyperfleet/docs/owners.md is in sync with owners.yaml + python3 hack/owners/gen_owners.py check diff --git a/README.md b/README.md index 07159408..085a0159 100644 --- a/README.md +++ b/README.md @@ -90,7 +90,7 @@ architecture/ ├── README.md # This file - repository guide ├── CONTRIBUTING.md # How to contribute to this repository ├── CLAUDE.md # Claude Code guidelines for AI-assisted workflows -├── OWNERS # Repository ownership +├── OWNERS # Repository ownership (generated from hyperfleet/docs/owners.yaml) ├── hack/ # Utility scripts └── hyperfleet/ # HyperFleet-specific architecture ├── README.md # HyperFleet system overview and architecture summary @@ -305,25 +305,15 @@ Example of paying down technical debt: ### Review and Merge Process -**MVP Process (Current)**: - -All HyperFleet team developers have approve and merge access. **Goal**: Move fast while keeping the team informed. +A merge needs one approval from an approver for every path the PR touches, as declared in the `OWNERS` files and enforced by Prow. Reviewer and approver status in an area is earned against the criteria in [Repository Ownership](hyperfleet/docs/owners.md), which also holds the current area table and the path to becoming a reviewer or an approver. **How to submit and merge changes:** 1. **Create a PR** with your documentation updates 2. **Add a clear description** of what changed and why 3. **Post the PR link** in Slack: #hcm-hyperfleet-team for team visibility -4. **Wait 24 hours** for peer review and objections (accounts for time zone differences between regions) - - For urgent changes, use judgment but ensure the change is well-documented with clear rationale - - For major architectural changes, strongly consider waiting for at least one or two Technical Leader reviews -5. **Merge when ready**: Once you have approval from peers and no objections after the 24-hour window, you may merge - -**Post-MVP Process** (to be established): - -- Formal approval requirements based on change type -- Required reviewers for different document categories -- Defined merge timelines and escalation paths +4. Prow requests reviewers from the area's pool. Reviewers aim for a substantive review within one business day +5. An area approver's `/lgtm` or approving review merges the PR once CI is green --- diff --git a/hack/owners/__pycache__/gen_owners.cpython-314.pyc b/hack/owners/__pycache__/gen_owners.cpython-314.pyc new file mode 100644 index 0000000000000000000000000000000000000000..6b49cea2faf5f7c431fc41462c947a1cf6638ed4 GIT binary patch literal 12759 zcmbt4ZEO=~nltv;9((+mPbV?SgrpP$`2a$J04WJcAOwOt1c$aMUK39e7soa;4uO>2 zy7$$j(8l!Gjl0*X?rrz#J+->%w71G?_awBZ(@|-Em^vTcAyO-Cb-Ml6LZ$BZ$LXH; z9gpoKrc3wUD6ePc{d(T#`+44Xr`cp6pfvpIw=XtU6U6V)i&AvihlhlUAfg0EM2HIn zNA{5s691|qD*UUCsPQiqp}XHAiupHcFl&!hGkk+T2*t)Z3wC5w^RJdx2%yDZwA%vnzW#PxW=4?Kz9r&H@ALz`$@w^hZL% zHJI};%61<=>YWTk*m3X3r7UbMu%;7FEbyZOFOK8Q;VnJhr2gQec0J+SwAGPue&o_d zNJqW?0P7#kFU-Q6S@cZ!Sii`M5iejkB%m&j2i3Ekr%$1g{@_R`DEh?+AB=2*Mkd?_ zaKeH=7{Rc31>P$Y*I?i#G3~?QfS>oV-iSi9h<}1-p(6|Ao}7S1zkUgvo_%!v;%sC2;0=u zFnnR?AoK#J!Jvm7V6T?|uLJ%&k(m(%OPq+zX^rU)H#eu%%2t|k0 z5e)=O>?EGd4j6+K9fV(1M{FVL@E->~4ym^ge)4DJfRmD_P?!%&+OucQamRWj3L#t4 zi{4S*69{>IPEsOAMYI7{I^HrKn&4YTgb?szi%ST-2v`tX@)T()#7#?~q^y%#i9$0A zNQ2kGzxV`HQDRMRx;Z-Mxao-7)-0uSKX~m2ac|nfeyt`96?2X`#|_6fq8@t2D-LRY z$#fd@BRRCfSA;T*^1s59Y?2SVe}Y`a1i|rLAXg58oi;o z)e55P2)D|m+0YEHg+2OP2trTPLeB<5prO&Jmee92k;NJ$~B$&|$t&jIjf|{y?mqz#(_d^9JvX=h6CML`@+&7iXkQv7K#*vKC zI(K6BMC_TH{TZWW?&R#r*yT4)f2kwPrC-p5(GorR5QY*XImUw*v=5A{C_F@goj76_ zm~^FtRn34QHwn0E09;XnYF8nqv)zWe4#JZUDB>W78VkptR=KHWGLN}_pfAG9a1O(H ztThn8U^;KW?-hB`sgkt(RZurRym6-nB-|VEN}8Z|f)@n@VisRP5$$1H0mAsJVX!U& z;wK<5A+|#mB{Jrc*umFB(c>9|F?vkK8qLfZW(^73`;OTDAGI%ZF6>>Q_kXDdgniD~ zVhuMArKk$Q4#U^+7osP?Pd)rKp4d$k!wE+=6Kz>cTZwZZ*v-VGsuvuP%eK7e7jPbk zfSVjp^_R*R3ai58aE{A{6}Ji~RCG0%U0F@Rwp+Ce;8Tk7wE>{0ga0t;2d11p%JEnH z{1j*{HXN7~vBC~}BjZnkg;w<0E_T8n%(jhs1A&X6OxY}*nk9X%pQIO*$$+)hW5Dj# z-p*5}x;l^bE4CL?6NDa235WzGhWQbR^a}L=rigd~$SAYDDL0pAG$aVyp=}@P^bAx{ z;xh;P?uoZfB%WW`y5!iGww}G`*cZFJ)WzL9$j#egHh7KprL1Sy7~Ac}SY-a{t*Z$p z$)zh>(1ipc-`p|P`y`X?<^>!?jZ6N2pF|KOEdh%osG z8st>5qHui`NFoY&eS>rt%$!z#BbHy`i=NLW2bqTY{yLZ!@_8?_k< zRiTV3{Q(LUo|gnkRAZ6|TVWkZ8}MG_10sBDfL8%Tn85Ua52(MS2HT5!uBV_uHn$$s zia7BQ{EIwPAd5%@GdkwgtFK&*F{?VqCpyPkOqwQg1GTZZDTRb}grZ>khq8(`7e%>qi~;0T$>c?9)Tt9tR3vGycmtE5S}}Psp^?l9 zuqw2PApu-6r$%s~R~m_}XTlqns7aCcVJ;AAp=F~gHlk)vR#galpvQ64YXmB=V2t_3 zzG!#GYM-~dCWIIerr7bQnD*u(UP{c{`|-1Z~pp@t|qPJ-UHZT$)iT}iC*#^DrIxmj~-VPVVyh@{|*K`j?o zA){o!A?FG^ROEg%1`0x4FIXl$LWk!Pe@&E1vax_?G!UBdh>>dn9*~QOly#hZlM`XE zYwFO&7qRQKO;&Fd`Ys%RelGYI%b`LTvCMV6))8+=mfzX8#Pp@8zRi0;1jT#krqCF$ z&t#sJH`;H7F@UupAS-07Vm?j*ql4j`0-OkKZVFfv)IeA>#1&X8*-rv}w6Sn1x7tW> zYPZHn>?vf-MwuaVo)N)Oj}K1|*$WF{h{oKyKLh)JXM4_ar2zZ00K37hFD9sNLtap! z=TYd%3@xn3Q^B4=6o&q_;2orAkSyj24r&1d+Clmdspw!?gds52K@06RK&^K(fFYei zArRfJEQX$!e~ZJ+t%vz3fY~^7hcgu7vH(Va`7eTzaT@_f;~s4Re88>Pr=>X)3jDKb z>6f6EieeWMAz5Eh!GaI?7JTeakf&1&Z*GJ)^PZfd0-S)+MQiqu{V*1jOy#n2S z3^*`hS`T7cn;wJKV2r*it&79VS=}biRtV7s3KwGVh`fh>cXlr(uB31z$mpZ!7Xv)eEi2W#XU#L3 z`!+i^YHz%pu~y8RZkZC=#AISD)pTUhyLdKL+qq)xLguXstXs_<2;*w%;#2Q5tyVYP ztA@G-=AbgwTi>b$H=K zYUh!~^LO|=Lw5&KTc2Mk8~zezYyCn?Sj%F<8`Fu#54OLzJ$Yz#>;9#!`xO+FCA6t+ z-KpB16>INTr9=%{y{7ex3Ig1Np?OoxG(+7lZJIfjDX*G8f9rgFB(Z0ud|Qmnl$Ir` z-Z`^Ux^sd2g++m=C)t?1^3kqj?ZV~7(nZap=k7CikEOP{R?41%tyMe;lb1E40NEPT zy?z9IP_!YYIb$rDJ3V_k-jirdT>0s)L~ZiwBDFa5@1}*9?*voT=T?mFFHMB4_6sw3 z9MRL8{Y?s(0UK(>eqLY+u4w4yCiapXSxE$F6lS+UUsM#;iCm6p)h>2jNP^7nUig2A ze22s2R!E&hf%kfAK&g>uAq%ulsjM^mpa@7e3AJhub(M%}Kox9}RRO1NAoi*KB#0CO zcv{77mq_Qp&rs?@)!ZtP0W4HmA3rLyzy2^}b0FRpIwU)e%$pAfLL=UQ*wLJ8xeT+3 z$d~-%9{|i$S=TS0TJXlH`1yCtiOb7&XR2~Xa%=LL?BTt1wyy#p-_q;R8qN;OxOAv{t&TE6sGJsIJYf zlo5Jk3Lysy(FcNd2#i4uuM3dC#M}VPLLgQ&^v)%Qa%K*cg#+9hw~A92QUc7qDy%;3 zC=3YmvBZ#y(-ij3h4W~)P$2F=u;St_2o+(R_FKnMV7PS3$owy-1Ni8|k-!yt__odj z;cv*H)4M6CnLWAF65yQIvz%je<)WMX{O(-Z2YA>tz5P1di4wurSr=$p5q>+B`~p1Z zvk=#(ZP%Ncn%4abPs6kl?GKLgkj@vU^~w{Qk5Z>q*ID5x^qW@0e3Ay~a}k*Mx%3c&8g!@1hF0kFIlW{>QV<)w@pOu+{uTvQhS|S(J|Um0 zQdAO(C5j0`K2@FTsdmF*-vRJJ$FNAa;m8la0Xsq-V-I@vfz^f_Nsd8>X}~Hjz;0yT z7aS+us^MJlyBVroxj+|-Du}RKMM4}>7-YN2!ngHpLuwBC7q$c2Qhz{@OjovN52q%) z!X=a{oA8UGKR7lZfE!FWwGuVz5Bel#u(Rt_kLT=QCpReR<(Hm;Zpm==8XTNV^j!5r z{02;9NG$D8h-!2ru^&#$eSb{EL(L z0p)%BX8UemN*lLEdol*|oO9NByDx31MM)Op%;mqi7CRQNzkNI|zWvgg+5Yyi_`Vdo zKUH-gZ9bTy4`wL+&9k>_(xxrT)E1b4vBb2o@|nZ&#$~29QMJtMNDeMDyBE3_%N9;9 zMp$wx84K5tO1?gmZQq_Xe$zT>Q&kQ5S7ZuiC0oqkuu@}jCAk~Jup zf;2GQlGD>lI|@_LU;Z4)O%)UPJnecv%2MvfZha1GEXG=PuRyiO#MRKV3Vu$LgPJf? zfbHVcWHwOq|3fE+qm|XaolcO1_*Oa%ql^wNZi9l?jlV_kthzw1S7iPFB2x+i;5X%` z;r#v}u`3s4=3*QLkX9-E^1ll0!p4oX9tJN`F6WKI$h*QNpO9AY6E+R49hd?IzHftc zUXFz7ZZdntyU27I8FJ;0#-#J8Vr_K>GukWV+iz2B){KUS~TAE(zK9QeuooO)px zP$xj9ffxi&mgcX*p@%1QNixfK50oPjiJZcL_kg688=Mv#&q}J0C>%#qAV)=_C%l(< zpC7{6J?I3(=c;4q9XTvYNUVdiZx75OX+}bm!H9s; zYS{?*BAmO)2WyDK1SH4u^4s7^B7MRL`lYeC0>dTZka%58EW1fs`67mJ1-j3|zxW|` zFEU0G*cJ$ZH-@ik7Tes^?9|-#+3WG+Y0C~s1Ji~%<~1hPooHL8x387d#v|#H<{90m zObKL&Z(p1ryEQf+xD`mWrJM)T743`1R&7U@Y)9{OW~{b(-7VeQRNVjZRN6W8@zmY= z4+j?-7q0zV%boh=J>BW1o=jE~426!o zOFy^N#Jk^rB1!#ZXWG&nJ(1B_mUNYCCfn_Xm^gRg=7soW!0z1f+2gSjOU5lQ#_afv zu|`kL?2DfI(}Nmf$9YoTB;YA!JNnJn27m$p{pM>eOg~rk&DS&xdE){e^o`gBRQrvi z;zXP7H+#08Xes-TG9x_yXKQ00t^Y5Lt*=IN*H+Tkthw8wf$w+Qm_7~jG0pVZO&`}V zeLM9ZH#2=tuRoL5!IaoN_(#kPg&Au_;31EVdf>9zmn@>%P(X&}-B5C>O}+6tME(%w zygG06H>)Nl#>pfk#Ux3cr z>%otuhS8}%F5u5X0JcXpj6z&oT-C}`{2Ht?hC7#q-tlKIgq(UatGuXz>6UhM zBPV-xCwF~l7b{E#i{8BYMu*dF?d;DWY)6;8K&+8;P3Y$RRjzcBs z1U`VmHi_cSoEd}=4qYLFYZGwHq7@@>gG|UqYXV|W0c$C&lSqD3sD^Jy39mrE_uyZI z_#K?mH4_{!-M9dGIk;c)C|T2$#=75rB0>Gb&a`e@vT;e*oT1HgrdiY5CGS?gRrzk+ zTXi3ne&|bA?M>O+Qf(ut3f~gV-#3-ckZZIlri+s?ac1KEh7WeUw_}xVg6xvH>@!RG z&Gr;k{(Zi-@eg*Za-c*E%E z7}<{vg@Xg&JOe=!R}&9DzKd2d%6Ka}cha zFDyfW>b%J3ec{|U%B^EjPM!|@pyI2Pwaz1bwI_5&|!iJ(Ky0ESQGNT(EhM9() zN@q94nG5q#3f`fYWf(9k7gu47u6%1zTJyg^ry`IHw<L4SPAi=8GCp1$ z5&YqZKNOTKa0O+;8-XmpXC&YiMM*s};Twcr6!eNnw7_4U^b2qjj(nbJ4ZeX2E}t%Y zIE?iWcEbjQtI$KZhPr8|gb>_Koi10-K(ZHO;ZB_>ve<8&w&gFU`r*=SB*Z$aB-$&C zg}s8vOXf~t3}2UYp;tlDdC+YOk4LKEyi?uz4V(bQ+N*3qGQ%A-k2rY|&jBYP2J|k! zNJjJw^W#}1rn{%N^W3RH&zWkkU^@cl#h7dt>fVQs%^YW&31`18u&ZeKkaymvCVWD3DCjKR8UsJmyVOPpI6Od0CZ zhIYu4z*R8wtT}cyO|#L?45fK>;FW>c{`dQ%14~qEhSJIJOH|FeXU4Y5IPNiyG*cVx z{+zMgcnY$B*?TSfmvq%&5pMFa?i-hu=<46m=Jid?9Q_)7n~!(TUwZS>EnDL9679^H zs84mK`?gBta@u0Y5fIJ4y9R$W8)Ttlla&pnqX^*!I}_r~vi zZ+z*+VA>T*oe8H5mscnO=XWI91D8DfL667D3dn*A$eowThlq`@Ib=<@_o9eT&~T`t zg8%~`*ueydsXZP{WWj(2c~KQZ)df^}P}PVk6i?&pVff}AzF{QCGdSKkfLbw2;fz1# zxH0q%iJZ)Ei2ev>v@|R^fEtjuB=ZP;cp~J3>&G3!JMayeFi`{b7iy9uzoZzFdT1v| z>wQ9hpD_HMU_R67U!8ho>eZKDc{wJ`Pu-fDzkcg_qU(d>?;Zc(%zI}RPNW-;EJp4O zrYldTb^TE)qi5zSXDjC%vyPZAek!?bp((9D8r5dB^s8g9jJ+CoB@k^5S@PfjNgjhoJz0aAo_* literal 0 HcmV?d00001 diff --git a/hack/owners/gen_owners.py b/hack/owners/gen_owners.py new file mode 100755 index 00000000..62797d05 --- /dev/null +++ b/hack/owners/gen_owners.py @@ -0,0 +1,197 @@ +#!/usr/bin/env python3 +"""Render OWNERS files and the ownership table from hyperfleet/docs/owners.yaml. + +Usage: + gen_owners.py render [--out DIR] write OWNERS_ALIASES and OWNERS per repository + (default hack/owners/out) and refresh the table + in hyperfleet/docs/owners.md + gen_owners.py check fail if the table in owners.md is stale or the + YAML is inconsistent + gen_owners.py table print the area table + +Rules applied at render time are documented at the top of owners.yaml. +""" +import argparse +import os +import sys + +import yaml + +ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +SOURCE = os.path.join(ROOT, "hyperfleet", "docs", "owners.yaml") +DOC = os.path.join(ROOT, "hyperfleet", "docs", "owners.md") +DEFAULT_OUT = os.path.join(ROOT, "hack", "owners", "out") +TABLE_START = "" +TABLE_END = "" + +# Fallback approver sets that are never auto-requested as reviewers. +FALLBACK_ALIASES = {"tech-leads", "architecture-approvers"} + + +def load(): + with open(SOURCE) as f: + return yaml.safe_load(f) + + +def dedupe(items): + seen, out = set(), [] + for x in items: + if x not in seen: + seen.add(x) + out.append(x) + return out + + +def expand(data, names): + aliases = data["aliases"] + out = [] + for n in names: + out.extend(aliases[n] if n in aliases else [n]) + return dedupe(out) + + +def reviewers_for(spec): + """Reviewers plus the path's approvers, minus the fallback aliases.""" + approvers = [a for a in spec.get("approvers", []) if a not in FALLBACK_ALIASES] + return dedupe(list(spec.get("reviewers", [])) + approvers) + + +def owners_doc(spec): + doc = {} + if spec.get("approvers"): + doc["approvers"] = list(spec["approvers"]) + reviewers = reviewers_for(spec) + if reviewers: + doc["reviewers"] = reviewers + if spec.get("labels"): + doc["labels"] = list(spec["labels"]) + return doc + + +def used_aliases(data, spec_map): + used = set() + for spec in spec_map.values(): + for n in list(spec.get("approvers", [])) + reviewers_for(spec): + if n in data["aliases"]: + used.add(n) + return sorted(used) + + +def dump(obj): + return yaml.safe_dump(obj, sort_keys=False, default_flow_style=False) + + +def validate(data): + errors = [] + people = set(data["people"]) + for alias, members in data["aliases"].items(): + for m in members: + if m not in people: + errors.append(f"alias {alias}: unknown login {m}") + for repo, spec_map in data["repos"].items(): + for path, spec in spec_map.items(): + for key in ("approvers", "reviewers"): + for n in spec.get(key, []): + if n not in data["aliases"] and n not in people: + errors.append(f"{repo}/{path or '(root)'} {key}: unknown name {n}") + for label in spec.get("labels", []): + if label not in data["labels"]: + errors.append(f"{repo}/{path or '(root)'}: label {label} not in labels") + if path == "" and not spec.get("approvers"): + errors.append(f"{repo}: root OWNERS has no approvers") + for area in data["areas"]: + for key in ("approvers", "reviewers"): + for n in area.get(key, []): + if n not in data["aliases"] and n not in people: + errors.append(f"area {area['name']} {key}: unknown name {n}") + if area["label"] not in data["labels"]: + errors.append(f"area {area['name']}: label {area['label']} not in labels") + return errors + + +def names(data, logins): + return ", ".join(f"{data['people'][l]} ({l})" for l in logins) + + +def table(data): + rows = ["| Area | Paths | Approvers | Reviewers |", "|---|---|---|---|"] + for area in data["areas"]: + if area.get("inherits"): + approvers = f"inherits {area['inherits']}" + else: + approvers = names(data, expand(data, area["approvers"])) + reviewers = names(data, expand(data, area["reviewers"])) + rows.append(f"| {area['name']} | {area['paths']} | {approvers} | {reviewers} |") + return "\n".join(rows) + + +def splice_table(text, new_table): + start = text.find(TABLE_START) + end = text.find(TABLE_END) + if start < 0 or end < 0 or end < start: + raise SystemExit(f"{DOC}: table markers missing") + start += len(TABLE_START) + return text[:start] + "\n" + new_table + "\n" + text[end:] + + +def render(data, out_dir): + count = 0 + for repo, spec_map in data["repos"].items(): + base = os.path.join(out_dir, repo) + os.makedirs(base, exist_ok=True) + aliases = used_aliases(data, spec_map) + if aliases: + with open(os.path.join(base, "OWNERS_ALIASES"), "w") as f: + f.write(dump({"aliases": {a: list(data["aliases"][a]) for a in aliases}})) + for path, spec in spec_map.items(): + d = os.path.join(base, path) if path else base + os.makedirs(d, exist_ok=True) + with open(os.path.join(d, "OWNERS"), "w") as f: + f.write(dump(owners_doc(spec))) + count += 1 + with open(DOC) as f: + text = f.read() + new_text = splice_table(text, table(data)) + if new_text != text: + with open(DOC, "w") as f: + f.write(new_text) + return count + + +def check(data): + errors = validate(data) + with open(DOC) as f: + text = f.read() + if splice_table(text, table(data)) != text: + errors.append(f"{os.path.relpath(DOC, ROOT)}: area table is stale, run gen_owners.py render") + for e in errors: + print(f"error: {e}", file=sys.stderr) + return not errors + + +def main(): + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + sub = parser.add_subparsers(dest="cmd", required=True) + r = sub.add_parser("render") + r.add_argument("--out", default=DEFAULT_OUT) + sub.add_parser("check") + sub.add_parser("table") + args = parser.parse_args() + + data = load() + if args.cmd == "table": + print(table(data)) + return + if args.cmd == "check": + sys.exit(0 if check(data) else 1) + errors = validate(data) + if errors: + for e in errors: + print(f"error: {e}", file=sys.stderr) + sys.exit(1) + n = render(data, args.out) + print(f"wrote {n} OWNERS files across {len(data['repos'])} repositories to {os.path.relpath(args.out, ROOT)}") + + +if __name__ == "__main__": + main() diff --git a/hack/owners/labels.sh b/hack/owners/labels.sh new file mode 100755 index 00000000..d5dc97ad --- /dev/null +++ b/hack/owners/labels.sh @@ -0,0 +1,50 @@ +#!/bin/bash +# Create the area, kind and tide labels from hyperfleet/docs/owners.yaml in every +# repository listed there. Idempotent: existing labels are updated in place. +# +# Usage: hack/owners/labels.sh [--dry-run] [repo ...] +# Needs gh authenticated with write access to the repositories. +set -euo pipefail + +ORG="openshift-hyperfleet" +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +SOURCE="${ROOT}/hyperfleet/docs/owners.yaml" +DRY_RUN=0 +if [[ "${1:-}" == "--dry-run" ]]; then + DRY_RUN=1 + shift +fi + +color_for() { + case "$1" in + area/*) echo "0052cc" ;; + kind/*) echo "5319e7" ;; + tide/*) echo "ededed" ;; + *) echo "bfdadc" ;; + esac +} + +mapfile -t REPOS < <(python3 -c "import yaml; print('\n'.join(yaml.safe_load(open('${SOURCE}'))['repos']))") +if [[ $# -gt 0 ]]; then + REPOS=("$@") +fi +mapfile -t LABELS < <(python3 -c " +import yaml +for name, desc in yaml.safe_load(open('${SOURCE}'))['labels'].items(): + print(f'{name}\t{desc}') +") + +for repo in "${REPOS[@]}"; do + echo "== ${ORG}/${repo}" + for entry in "${LABELS[@]}"; do + name="${entry%%$'\t'*}" + desc="${entry#*$'\t'}" + color="$(color_for "${name}")" + if [[ "${DRY_RUN}" -eq 1 ]]; then + echo " would create ${name} (${color}) ${desc}" + continue + fi + gh label create "${name}" --repo "${ORG}/${repo}" --color "${color}" --description "${desc}" --force >/dev/null + echo " ${name}" + done +done diff --git a/hack/owners/open_prs.sh b/hack/owners/open_prs.sh new file mode 100755 index 00000000..88b9e5cc --- /dev/null +++ b/hack/owners/open_prs.sh @@ -0,0 +1,81 @@ +#!/bin/bash +# Open one PR per repository that replaces its OWNERS files with the rendered set +# from hack/owners/out. Run gen_owners.py render first. +# +# Usage: hack/owners/open_prs.sh [--dry-run] [repo ...] +# Pushes to a branch on the caller's fork (created if missing) and opens the PR +# with gh. Existing OWNERS files that the generator does not produce are removed, +# so stale per-directory files go in the same change. +set -euo pipefail + +ORG="openshift-hyperfleet" +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +OUT="${ROOT}/hack/owners/out" +BRANCH="${BRANCH:-HYPERFLEET-1200-owners-areas}" +TITLE="${TITLE:-HYPERFLEET-1200 - chore: narrow OWNERS to area owners and aliases}" +DOC_URL="https://github.com/${ORG}/architecture/blob/main/hyperfleet/docs/owners.md" +DRY_RUN=0 +if [[ "${1:-}" == "--dry-run" ]]; then + DRY_RUN=1 + shift +fi + +FORK_OWNER="$(gh api user --jq .login)" +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +mapfile -t REPOS < <(ls "${OUT}") +if [[ $# -gt 0 ]]; then + REPOS=("$@") +fi + +read -r -d '' BODY </dev/null 2>&1; then + gh repo fork "${ORG}/${repo}" --clone=false >/dev/null + fi + git remote add fork "git@github.com:${FORK_OWNER}/${repo}.git" + git push -q --force-with-lease fork "${BRANCH}" + gh pr create --repo "${ORG}/${repo}" --head "${FORK_OWNER}:${BRANCH}" --base main \ + --title "${TITLE}" --body "${BODY}" + ) +done diff --git a/hyperfleet/docs/owners.md b/hyperfleet/docs/owners.md new file mode 100644 index 00000000..89c13ce0 --- /dev/null +++ b/hyperfleet/docs/owners.md @@ -0,0 +1,167 @@ +--- +Status: Active +Owner: HyperFleet Team +Last Updated: 2026-10-12 +--- + +# Repository Ownership + +> Who reviews and who approves changes in each area of the HyperFleet repositories, how a merge works, and the criteria for becoming a reviewer or an approver. The area table below is generated from [`owners.yaml`](owners.yaml), which is the source of truth for every `OWNERS` file in the organisation. + +## Summary + +Every area of code has named owners: two or three **approvers** and a wider pool of **reviewers**, declared in `OWNERS` files that Prow reads. Review requests go to the people who know the area. A merge needs one approval, from an approver for every path the change touches. Reviewer and approver status is earned against the criteria in this document and nothing else. + +## The two roles + +These are Prow's own roles. Ownership is by path: a repository has one set of names at the root and different names for the directories that need them. A PR touching two areas needs an approver from each. + +| Role | What it means | Where it lives | +| ---- | ------------- | -------------- | +| **Reviewer** | You know the area well enough to give a substantive first review. Prow requests you on PRs in the area automatically. Your approving review sets `lgtm`, and an approver still has to sign off. | `reviewers` in the area's `OWNERS` file | +| **Approver** | You can be the last check on the area. Your `/lgtm` or approving review merges the PR on its own. You are accountable for what merges into the area. | `approvers` in the area's `OWNERS` file | + +Approvers are also reviewers. The reviewer pool is where people grow into an area, and the approver set is the gate. + +## How a merge works + +Two things happen automatically when a PR is opened, and neither gates the merge: + +- **Labels.** The `owners-label` plugin reads the `labels` field of every `OWNERS` file covering a changed file and adds those labels to the PR, for example `area/tenancy` and `area/api` for a change under `pkg/tenant`. The daily open-PRs digest groups by these labels. +- **Reviewer requests.** The `blunderbuss` plugin requests two reviewers, picked at random from the `reviewers` of the closest `OWNERS` file for each touched path. + +Then the merge itself: + +1. Anyone in the area's reviewer or approver pool reads the PR and either requests changes or approves. +2. If the person who approves is an **area approver** for every touched path, their `/lgtm` or GitHub approving review sets both `lgtm` and `approved`, and Tide merges when CI is green. One person, but the right person. +3. If the person who approves is a **reviewer**, their approval sets `lgtm` only. They `/assign` an area approver, or the approver picks it up from the digest, and the approver's `/approve` completes it. + +Rules that go with it: + +- **No self-approval.** An approver's own PR needs another approver. +- **One approval, not two.** A second required signature costs more than it returns: every rebase wipes the first approval and the PR queues twice. The gate is who the one person is, not how many there are. An approver who wants a second review on a risky change (migrations, tenant isolation, release plumbing, RBAC) asks for it on the PR with `/hold` until it lands. +- **Small PRs are the norm.** Over about 400 non-generated lines, add a short review plan to the description. Over 1,000, explain why it cannot be split. +- **Emergencies.** If no area approver is available to fix a broken `main` or a live incident, a tech lead approves and says so in the PR. +- **Bot PRs are unchanged.** Trusted non-major dependency updates arrive labelled for merge and Tide merges them after CI. Updates labelled `manual-review-required` wait for a human `/lgtm`, which now has to come from an approver for the files they touch. Konflux nudge PRs on the operator merge on their own label. +- **Squash on merge.** Tide squashes the PR into one commit titled from the PR title, so the PR title carries the JIRA key and follows the commit standard. Commits inside the PR do not reach `main`. + +## Areas and owners + +The tech leads sit in every root approver set as the fallback, not the routine merger, and are not auto-requested as reviewers. Where an area is left with one named approver, the tech-leads alias covers it until someone meets the criteria below. + + +| Area | Paths | Approvers | Reviewers | +|---|---|---|---| +| API | hyperfleet-api root | Dmitrii (kuudori), Phuongnhat (pnguyen44) | Tirth (tirthct), Michaela (Mischulee), Mallory (ma-hill), Martin (mliptak0), Leonardo (ldornele), Rafael (rafabene) | +| Tenancy and auth | `pkg/tenant`, `pkg/auth`, `pkg/middleware`; e2e `e2e/auth` | Phuongnhat (pnguyen44), Martin (mliptak0) | Dmitrii (kuudori), Michaela (Mischulee) | +| Generic model | `pkg/dao`, `pkg/db`, `pkg/registry`, `pkg/services`, `pkg/handlers` | Dmitrii (kuudori), Phuongnhat (pnguyen44) | Tirth (tirthct), Michaela (Mischulee), Mallory (ma-hill), Martin (mliptak0) | +| Search | `pkg/validation`, `pkg/validators`, `pkg/api` | inherits Generic model | Michaela (Mischulee), Mallory (ma-hill), Martin (mliptak0) | +| Contracts | hyperfleet-api-spec, hyperfleet-api-spec-template, `openapi/` | Angel (rh-amarin), Martin (mliptak0), Dmitrii (kuudori), Ciaran (ciaranRoche) | Phuongnhat (pnguyen44), Michaela (Mischulee), Tirth (tirthct) | +| Adapter framework | hyperfleet-adapter root | Angel (rh-amarin), Rafael (rafabene), Dmitrii (kuudori) | Michal (Ruclo), Michaela (Mischulee), Martin (mliptak0), Tirth (tirthct) | +| Desire delivery | adapter `internal/desireclient`, `internal/transportclient`, `internal/transportregistry`; hyperfleet-applier | Dmitrii (kuudori), Angel (rh-amarin), Phuongnhat (pnguyen44) | Michal (Ruclo), Mallory (ma-hill), Tirth (tirthct) | +| Sentinel | hyperfleet-sentinel | Phuongnhat (pnguyen44), Dmitrii (kuudori), Angel (rh-amarin) | Tirth (tirthct), Michal (Ruclo), Leonardo (ldornele), Martin (mliptak0) | +| Broker | hyperfleet-broker | Angel (rh-amarin), Dmitrii (kuudori) | Tirth (tirthct), Phuongnhat (pnguyen44), Michaela (Mischulee) | +| Gateway | infra `helm/hyperfleet-gateway`, `helm/network-policies`, `helm/mock-oidc` | Martin (mliptak0), Dmitrii (kuudori) | Leonardo (ldornele), Phuongnhat (pnguyen44) | +| Observability | `pkg/telemetry`, `pkg/metrics`, `charts/dashboards`; operator `internal/metrics`, `internal/servicemonitor`, `config/prometheus` | Phuongnhat (pnguyen44), Dmitrii (kuudori) | Leonardo (ldornele), Tirth (tirthct), Michal (Ruclo) | +| Release and packaging | `.tekton` everywhere; operator `catalog`, `config/manifests`, `validators`; hyperfleet-release | Mallory (ma-hill), Dmitrii (kuudori) | Tirth (tirthct), Michaela (Mischulee) | +| E2E | hyperfleet-e2e; infra `helmfile` | Dmitrii (kuudori), Mallory (ma-hill) | Phuongnhat (pnguyen44), Tirth (tirthct), Michaela (Mischulee), Martin (mliptak0) | +| Performance | e2e `perf` | Phuongnhat (pnguyen44), Dmitrii (kuudori) | Tirth (tirthct), Mallory (ma-hill) | +| Operator | hyperfleet-operator root | Dmitrii (kuudori), Ciaran (ciaranRoche), Rafael (rafabene), Angel (rh-amarin) | Tirth (tirthct), Mallory (ma-hill), Rafael (rafabene), Leonardo (ldornele), Michal (Ruclo) | +| Infra | hyperfleet-infra root, `terraform/envs`, `functions/lifecycle-enforcer` | Mallory (ma-hill), Dmitrii (kuudori), Rafael (rafabene) | Martin (mliptak0), Phuongnhat (pnguyen44), Tirth (tirthct) | +| OCI | infra `terraform/oci`, `functions/oci-ci-sweep` | Rafael (rafabene), Ciaran (ciaranRoche) | Phuongnhat (pnguyen44), Leonardo (ldornele), Michaela (Mischulee) | +| Team tooling | hyperfleet-claude-plugins, hyperfleet-hooks, coderabbit, .github | Rafael (rafabene), Phuongnhat (pnguyen44) | Dmitrii (kuudori), Tirth (tirthct) | +| Architecture | architecture repository | Ciaran (ciaranRoche), Michael (mbrudnoy), Rafael (rafabene), Angel (rh-amarin) | Dmitrii (kuudori), Phuongnhat (pnguyen44), Tirth (tirthct), Leonardo (ldornele), Mallory (ma-hill), Martin (mliptak0), Michal (Ruclo), Michaela (Mischulee) | +| Platform libraries | hyperfleet-logger, hyperfleet-credential-provider, renovate-config | Dmitrii (kuudori), Mallory (ma-hill) | Michal (Ruclo), Martin (mliptak0) | + + +## The criteria + +**Reviewer and approver status in an area follows the criteria below and nothing else. Title, level, tenure and team do not count, for anyone, including the tech leads.** + +The bar is deliberately concrete. It is sized for a team of ten, and the shape is the Kubernetes community ladder: contribute, review, get sponsored, and keep doing it. + +### Reviewer in an area + +You are ready when all of these are true: + +1. You have authored **five PRs** that merged in the area. +2. You have given **five reviews** in the area with comments you wrote yourself. Running the review plugin is fine and encouraged, but the findings you post are yours, and you have read the diff. +3. One current approver of the area **sponsors** you. + +### Approver in an area + +You are ready when all of these are true: + +1. You have been a reviewer in the area for **at least two sprints**. +2. You have been the **primary reviewer on ten PRs** in the area. At least three were substantial (a design change, or over 300 lines), and your review changed at least some of them. +3. You have **shipped one story end to end** in the area with a reviewer other than the area's most active approver, or you have written a design-level change for it (an ADR, a design doc, or the story decomposition of an epic). +4. You know the **tests that cover the area**: which suites exercise it, what they do and do not check, and where a change would need a new one. +5. An existing approver **nominates** you in a PR to `owners.yaml`, and no other approver objects within three working days. + +### Staying an approver + +- An approver with **no reviews in the area for two sprints** moves back to the reviewer alias at the next sweep. Nothing is lost: review in the area for a sprint and ask, and you are re-added without re-qualifying if it is within six months. +- On loan or extended leave, tell the other approvers in the area and either `/uncc` yourself or move to the reviewer alias for the duration. +- Leaving the team removes you from every alias in the same PR that removes your access. + +### How the table was set + +The same criteria were applied to everyone at launch, using the last three sprints of authored PRs and reviews. Nobody is an approver on title or on history alone. The same criteria, applied the same way, are used at every quarterly sweep. + +### How to apply + +Open a PR to `owners.yaml` that adds you to the alias, and cite the PRs and reviews that meet each criterion with links. The sponsor (for reviewer) or nominator (for approver) approves the PR. The evidence is the PR list, nothing else is asked for. Then regenerate the `OWNERS` files (see [Changing the table](#changing-the-table)). + +### Questions + +- **I'm a senior, why am I only a reviewer in this area?** Because approval follows contribution in the area, not level. The path is above and takes about two sprints of doing the work. +- **What about partner engineers?** Same criteria. An engineer from another team who meets the reviewer bar in an area belongs in that alias. +- **What if both approvers are out?** The tech leads are in every root approver set as the fallback. That is what they are for. +- **Can I still review outside my areas?** Yes, please do. Your review counts and is how you grow into the area; an area approver completes the merge. + +## What a review and an approval mean + +The [working agreement](working-agreement.md#code-review) says review for correctness, consistency, clarity and learning. This adds what the roles commit to. + +### Reviewers + +- **Acknowledge within four working hours, substantive review within one business day.** If you cannot, say so on the PR and `/unassign` so Blunderbuss picks someone else. +- **Read the whole diff.** A review that only covers the first file is not a review. +- **Say what kind of comment it is.** Every comment is one of: fix before merge, follow-up (with a ticket or a named place it is tracked), or FYI. Pre-production, only reproducible failures block. +- **Aim for about four reviews a week.** That is roughly three hours. It is not forty. + +### Approvers + +Approving means: I read the whole diff, I checked the behavioural claims against the code, and I would be comfortable debugging this in production. + +- **Do not approve what you have not verified.** If the PR says the API hard-deletes kinds without required adapters, you have looked at the function or the test that proves it. +- **Say when you are out of depth.** "This touches the broker, get one of its approvers" is a good approver comment. +- **Approve the change, not the author.** A senior's PR gets the same read as an associate's. + +### Authors + +- **You own every claim in your PR.** If a tool wrote the description, the design note or the ADR, it is still yours. Behavioural claims cite a file and line or a test. If you cannot point at the code for a claim, take the claim out. +- **Read your own diff before you ask a person to.** CodeRabbit will get there first, and that is fine. What it cannot do is decide whether the change is right, and a green bot is not the same as ready for review. +- **Keep it small.** A 200-line PR gets reviewed today. A 2,000-line PR waits a week, and the review it gets is worse. +- **Answer every comment**, and when you mark something fixed, check the fix across the whole change, not only the line the reviewer pointed at. +- **No force-push or amend once review has started.** Push the fix as a new commit. A rewritten branch throws away the reviewer's "changes since your last review" view. Squash on merge keeps `main` clean, so the branch history does not need to be tidy, only the PR title does. Rebase only when you have to, for example to resolve a conflict. + +## Mechanics + +- `OWNERS_ALIASES` at each repository root holds the names. `OWNERS` files reference alias names only, so a change of owner is a regenerated file per repository. +- Sub-directory `OWNERS` files exist only where ownership genuinely differs from the root. Everything else inherits the root. +- Every `OWNERS` file carries a `labels` entry so `owners-label` tags PRs by area. The labels are created in every repository by `hack/owners/labels.sh`. +- Prow's `approve` plugin runs with `lgtm_acts_as_approve` and `require_self_approval`, and `lgtm` with `review_acts_as_lgtm`, in the shared openshift/release configuration. No plugin setting changes for this model; the `OWNERS` files are the whole change. +- Blunderbuss is configured once for all of openshift/release (two reviewers per PR, drafts ignored). It reads `reviewers` only, which is why approvers are listed there too. + +### Changing the table + +1. Open a PR to [`owners.yaml`](owners.yaml) in this repository. For a role change, follow [How to apply](#how-to-apply). +2. Run `make owners` to regenerate the table in this document and the `OWNERS` files under `hack/owners/out/`. `make verify-owners` is the check that the table is current. +3. Once merged, run `hack/owners/open_prs.sh` to open the `OWNERS` PRs in the affected repositories. + +## References + +- Prow approve plugin and OWNERS semantics: +- Kubernetes community membership ladder, which the criteria are adapted from: +- Google's code review guide on speed: diff --git a/hyperfleet/docs/owners.yaml b/hyperfleet/docs/owners.yaml new file mode 100644 index 00000000..f6773511 --- /dev/null +++ b/hyperfleet/docs/owners.yaml @@ -0,0 +1,304 @@ +# Source of truth for repository ownership across the openshift-hyperfleet organisation. +# +# hack/owners/gen_owners.py reads this file, renders the area table into +# hyperfleet/docs/owners.md and writes OWNERS_ALIASES and OWNERS files for every +# repository. Change ownership here, regenerate, and open the per-repository PRs. +# +# Rules applied at render time: +# - every reviewers list also carries the area's approvers, so Blunderbuss can +# request them. The tech-leads and architecture-approvers aliases are the +# exception: they are fallback approvers and are never auto-requested. +# - a path without approvers inherits the approvers of the closest parent OWNERS. + +people: + ciaranRoche: Ciaran + rafabene: Rafael + rh-amarin: Angel + mbrudnoy: Michael + kuudori: Dmitrii + pnguyen44: Phuongnhat + tirthct: Tirth + Mischulee: Michaela + ma-hill: Mallory + mliptak0: Martin + ldornele: Leonardo + Ruclo: Michal + +aliases: + tech-leads: [ciaranRoche, rafabene, rh-amarin] + architecture-approvers: [ciaranRoche, mbrudnoy, rafabene, rh-amarin] + api-owners: [kuudori, pnguyen44] + api-reviewers: [tirthct, Mischulee, ma-hill, mliptak0, ldornele, rafabene] + tenancy-owners: [pnguyen44, mliptak0] + tenancy-reviewers: [kuudori, Mischulee] + generic-model-owners: [kuudori, pnguyen44] + generic-model-reviewers: [tirthct, Mischulee, ma-hill, mliptak0] + search-reviewers: [Mischulee, ma-hill, mliptak0] + contracts-owners: [rh-amarin, mliptak0, kuudori, ciaranRoche] + contracts-reviewers: [pnguyen44, Mischulee, tirthct] + adapter-owners: [rh-amarin, rafabene, kuudori] + adapter-reviewers: [Ruclo, Mischulee, mliptak0, tirthct] + desire-owners: [kuudori, rh-amarin, pnguyen44] + desire-reviewers: [Ruclo, ma-hill, tirthct] + sentinel-owners: [pnguyen44, kuudori, rh-amarin] + sentinel-reviewers: [tirthct, Ruclo, ldornele, mliptak0] + broker-owners: [rh-amarin, kuudori] + broker-reviewers: [tirthct, pnguyen44, Mischulee] + gateway-owners: [mliptak0, kuudori] + gateway-reviewers: [ldornele, pnguyen44] + observability-owners: [pnguyen44, kuudori] + observability-reviewers: [ldornele, tirthct, Ruclo] + release-owners: [ma-hill, kuudori] + release-reviewers: [tirthct, Mischulee] + e2e-owners: [kuudori, ma-hill] + e2e-reviewers: [pnguyen44, tirthct, Mischulee, mliptak0] + perf-owners: [pnguyen44, kuudori] + perf-reviewers: [tirthct, ma-hill] + operator-owners: [kuudori] + operator-reviewers: [tirthct, ma-hill, rafabene, ldornele, Ruclo] + infra-owners: [ma-hill, kuudori, rafabene] + infra-reviewers: [mliptak0, pnguyen44, tirthct] + oci-owners: [rafabene, ciaranRoche] + oci-reviewers: [pnguyen44, ldornele, Mischulee] + tooling-owners: [rafabene, pnguyen44] + tooling-reviewers: [kuudori, tirthct] + platform-owners: [kuudori, ma-hill] + platform-reviewers: [Ruclo, mliptak0] + +# The area table rendered into owners.md. Approvers and reviewers are alias names +# or logins. Paths is prose for the table. +areas: + - name: API + paths: hyperfleet-api root + approvers: [api-owners] + reviewers: [api-reviewers] + label: area/api + - name: Tenancy and auth + paths: "`pkg/tenant`, `pkg/auth`, `pkg/middleware`; e2e `e2e/auth`" + approvers: [tenancy-owners] + reviewers: [tenancy-reviewers] + label: area/tenancy + - name: Generic model + paths: "`pkg/dao`, `pkg/db`, `pkg/registry`, `pkg/services`, `pkg/handlers`" + approvers: [generic-model-owners] + reviewers: [generic-model-reviewers] + label: area/generic-model + - name: Search + paths: "`pkg/validation`, `pkg/validators`, `pkg/api`" + approvers: [] + inherits: Generic model + reviewers: [search-reviewers] + label: area/search + - name: Contracts + paths: "hyperfleet-api-spec, hyperfleet-api-spec-template, `openapi/`" + approvers: [contracts-owners] + reviewers: [contracts-reviewers] + label: area/contracts + - name: Adapter framework + paths: hyperfleet-adapter root + approvers: [adapter-owners] + reviewers: [adapter-reviewers] + label: area/adapter + - name: Desire delivery + paths: "adapter `internal/desireclient`, `internal/transportclient`, `internal/transportregistry`; hyperfleet-applier" + approvers: [desire-owners] + reviewers: [desire-reviewers] + label: area/desire-delivery + - name: Sentinel + paths: hyperfleet-sentinel + approvers: [sentinel-owners] + reviewers: [sentinel-reviewers] + label: area/sentinel + - name: Broker + paths: hyperfleet-broker + approvers: [broker-owners] + reviewers: [broker-reviewers] + label: area/broker + - name: Gateway + paths: "infra `helm/hyperfleet-gateway`, `helm/network-policies`, `helm/mock-oidc`" + approvers: [gateway-owners] + reviewers: [gateway-reviewers] + label: area/gateway + - name: Observability + paths: "`pkg/telemetry`, `pkg/metrics`, `charts/dashboards`; operator `internal/metrics`, `internal/servicemonitor`, `config/prometheus`" + approvers: [observability-owners] + reviewers: [observability-reviewers] + label: area/observability + - name: Release and packaging + paths: "`.tekton` everywhere; operator `catalog`, `config/manifests`, `validators`; hyperfleet-release" + approvers: [release-owners] + reviewers: [release-reviewers] + label: area/release + - name: E2E + paths: "hyperfleet-e2e; infra `helmfile`" + approvers: [e2e-owners] + reviewers: [e2e-reviewers] + label: area/e2e + - name: Performance + paths: "e2e `perf`" + approvers: [perf-owners] + reviewers: [perf-reviewers] + label: area/performance + - name: Operator + paths: hyperfleet-operator root + approvers: [operator-owners, tech-leads] + reviewers: [operator-reviewers] + label: area/operator + - name: Infra + paths: "hyperfleet-infra root, `terraform/envs`, `functions/lifecycle-enforcer`" + approvers: [infra-owners] + reviewers: [infra-reviewers] + label: area/infra + - name: OCI + paths: "infra `terraform/oci`, `functions/oci-ci-sweep`" + approvers: [oci-owners] + reviewers: [oci-reviewers] + label: area/oci + - name: Team tooling + paths: "hyperfleet-claude-plugins, hyperfleet-hooks, coderabbit, .github" + approvers: [tooling-owners] + reviewers: [tooling-reviewers] + label: area/tooling + - name: Architecture + paths: architecture repository + approvers: [architecture-approvers] + reviewers: [kuudori, pnguyen44, tirthct, ldornele, ma-hill, mliptak0, Ruclo, Mischulee] + label: area/architecture + - name: Platform libraries + paths: hyperfleet-logger, hyperfleet-credential-provider, renovate-config + approvers: [platform-owners] + reviewers: [platform-reviewers] + label: area/platform + +# GitHub labels that owners-label applies. Created in every repository by +# hack/owners/labels.sh. owners-label only adds labels that already exist. +labels: + area/api: API service + area/tenancy: Tenancy and auth + area/auth: Tenancy and auth + area/generic-model: Generic resource model + area/search: Search and validation + area/contracts: API contracts and specs + area/adapter: Adapter framework and DSL + area/desire-delivery: Desire delivery, transports and applier + area/sentinel: Sentinel + area/broker: Broker + area/gateway: Gateway, network policies and mock OIDC + area/observability: Telemetry, metrics and dashboards + area/release: Release and packaging + area/e2e: End-to-end testing and deployment + area/performance: Performance testing + area/operator: Operator + area/infra: Infrastructure and environments + area/oci: Oracle Cloud track + area/tooling: Team tooling, plugins and hooks + area/architecture: Architecture repository + area/platform: Platform libraries + kind/adr: Architecture decision record + kind/standard: Engineering standard + tide/merge-method-merge: Keep the branch commits on merge instead of squashing + +# repo -> path -> OWNERS file. "" is the repository root. +# approvers and reviewers are alias names or logins. Reviewers get the path's +# approvers appended at render time (see the rule at the top). +repos: + hyperfleet-api: + "": {approvers: [api-owners, tech-leads], reviewers: [api-reviewers], labels: [area/api]} + pkg/tenant: {approvers: [tenancy-owners], reviewers: [tenancy-reviewers], labels: [area/tenancy]} + pkg/auth: {approvers: [tenancy-owners], reviewers: [tenancy-reviewers], labels: [area/auth]} + pkg/middleware: {approvers: [tenancy-owners, generic-model-owners], reviewers: [tenancy-reviewers], labels: [area/auth]} + pkg/dao: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} + pkg/db: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model, area/search]} + pkg/registry: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} + pkg/services: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} + pkg/handlers: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} + pkg/validation: {approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} + pkg/validators: {approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} + pkg/api: {approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} + pkg/telemetry: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + pkg/metrics: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + charts/dashboards: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + openapi: {approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} + .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + hyperfleet-api-spec: + "": {approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} + hyperfleet-api-spec-template: + "": {approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} + hyperfleet-adapter: + "": {approvers: [adapter-owners, tech-leads], reviewers: [adapter-reviewers], labels: [area/adapter]} + internal/desireclient: {approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} + internal/transportclient: {approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} + internal/transportregistry: {approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} + charts/dashboards: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + pkg/telemetry: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + hyperfleet-applier: + "": {approvers: [desire-owners, tech-leads], reviewers: [desire-reviewers], labels: [area/desire-delivery]} + .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + hyperfleet-sentinel: + "": {approvers: [sentinel-owners, tech-leads], reviewers: [sentinel-reviewers], labels: [area/sentinel]} + pkg/telemetry: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + charts/dashboards: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + hyperfleet-broker: + "": {approvers: [broker-owners, tech-leads], reviewers: [broker-reviewers], labels: [area/broker]} + hyperfleet-logger: + "": {approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} + hyperfleet-credential-provider: + "": {approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} + renovate-config: + "": {approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} + hyperfleet-e2e: + "": {approvers: [e2e-owners, tech-leads], reviewers: [e2e-reviewers], labels: [area/e2e]} + e2e/auth: {approvers: [e2e-owners, tenancy-owners], reviewers: [gateway-reviewers], labels: [area/auth]} + perf: {approvers: [perf-owners], reviewers: [perf-reviewers], labels: [area/performance]} + hyperfleet-operator: + "": {approvers: [operator-owners, tech-leads], reviewers: [operator-reviewers], labels: [area/operator]} + catalog: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + config/manifests: {approvers: [release-owners, operator-owners], reviewers: [release-reviewers], labels: [area/release]} + validators: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + internal/metrics: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + internal/servicemonitor: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + config/prometheus: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + hyperfleet-infra: + "": {approvers: [infra-owners, tech-leads], reviewers: [infra-reviewers], labels: [area/infra]} + helm/hyperfleet-gateway: {approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} + helm/network-policies: {approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} + helm/mock-oidc: {approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} + helmfile: {approvers: [e2e-owners], reviewers: [mliptak0, tirthct], labels: [area/e2e]} + terraform/oci: {approvers: [oci-owners], reviewers: [oci-reviewers], labels: [area/oci]} + functions/oci-ci-sweep: {approvers: [oci-owners], reviewers: [oci-reviewers], labels: [area/oci]} + functions/lifecycle-enforcer: {approvers: [rafabene, ma-hill], reviewers: [kuudori], labels: [area/infra]} + terraform/envs: {approvers: [rafabene, ma-hill], reviewers: [kuudori, mliptak0], labels: [area/infra]} + hyperfleet-release: + "": {approvers: [release-owners, tech-leads], reviewers: [release-reviewers], labels: [area/release]} + architecture: + "": {approvers: [architecture-approvers], reviewers: [kuudori, pnguyen44, tirthct, ldornele, ma-hill, mliptak0, Ruclo, Mischulee], labels: [area/architecture]} + hyperfleet/adrs: {reviewers: [pnguyen44, kuudori, tirthct, ldornele], labels: [kind/adr]} + hyperfleet/standards: {reviewers: [rafabene, kuudori, ldornele, pnguyen44], labels: [kind/standard]} + hyperfleet/components/api-service: {reviewers: [api-owners, api-reviewers], labels: [area/api]} + hyperfleet/components/adapter: {reviewers: [adapter-owners, adapter-reviewers], labels: [area/adapter]} + hyperfleet/components/sentinel: {reviewers: [sentinel-owners, sentinel-reviewers], labels: [area/sentinel]} + hyperfleet/components/broker: {reviewers: [broker-owners, broker-reviewers], labels: [area/broker]} + hyperfleet/components/claude-code-plugin: {reviewers: [tooling-owners, tooling-reviewers], labels: [area/tooling]} + hyperfleet/docs/e2e-testing: {reviewers: [e2e-owners, e2e-reviewers], labels: [area/e2e]} + hyperfleet/docs/release: {reviewers: [release-owners, release-reviewers], labels: [area/release]} + hyperfleet-claude-plugins: + "": {approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers, ma-hill], labels: [area/tooling]} + hyperfleet-adapter-authoring: {approvers: [adapter-owners], reviewers: [adapter-reviewers], labels: [area/adapter]} + hyperfleet-architecture: {approvers: [ciaranRoche, rafabene], reviewers: [pnguyen44], labels: [area/architecture]} + hyperfleet-code-review: {approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-devtools: {approvers: [rafabene, kuudori], reviewers: [tirthct], labels: [area/tooling]} + hyperfleet-hooks: {approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-jira: {approvers: [rafabene, ciaranRoche], reviewers: [pnguyen44], labels: [area/tooling]} + hyperfleet-operational-readiness: {approvers: [tirthct, ciaranRoche], reviewers: [pnguyen44, ma-hill], labels: [area/tooling]} + hyperfleet-release: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + hyperfleet-standards: {approvers: [rafabene, kuudori], reviewers: [ldornele], labels: [kind/standard]} + hyperfleet-work-triage: {approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-hooks: + "": {approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers], labels: [area/tooling]} + coderabbit: + "": {approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers], labels: [area/tooling]} + .github: + "": {approvers: [tech-leads], reviewers: [tooling-owners, tooling-reviewers], labels: [area/tooling]} diff --git a/hyperfleet/docs/working-agreement.md b/hyperfleet/docs/working-agreement.md index c552b2fb..c6e134cf 100644 --- a/hyperfleet/docs/working-agreement.md +++ b/hyperfleet/docs/working-agreement.md @@ -1,7 +1,7 @@ --- Status: Active Owner: HyperFleet Team -Last Updated: 2026-08-18 +Last Updated: 2026-10-12 --- # HyperFleet Working Agreement @@ -54,6 +54,35 @@ The two intersect during delivery: the **Epic Owner drives the Epic to done**, b This section defines the two roles the team asks about most: the **Feature Owner** and the **Epic Owner**. +### Repository roles: reviewer and approver + +Code review and merge rights are a third axis, declared per directory in `OWNERS` files and enforced by Prow. The full model, the current area table and how to apply are in [Repository Ownership](owners.md). The criteria are repeated here because they are rules of this agreement, not guidance. + +**Reviewer and approver status in an area follows the criteria below and nothing else. Title, level, tenure and team do not count, for anyone, including the tech leads.** + +| Role | What it means | +| ---- | ------------- | +| **Reviewer** | You know the area well enough to give a substantive first review. Prow requests you on PRs in the area. Your approving review sets `lgtm`, and an approver still has to sign off. | +| **Approver** | You can be the last check on the area. Your approval merges the PR on its own. You are accountable for what merges into the area. | + +**Reviewer in an area**, all of: + +1. Five PRs you authored have merged in the area. +2. Five reviews in the area with comments you wrote yourself, having read the diff. +3. A current approver of the area sponsors you. + +**Approver in an area**, all of: + +1. Reviewer in the area for at least two sprints. +2. Primary reviewer on ten PRs in the area, at least three substantial (a design change, or over 300 lines), and your review changed at least some of them. +3. One story shipped end to end in the area with a reviewer other than the area's most active approver, or a design-level change for it (ADR, design doc, or an epic's story decomposition). +4. You know the tests that cover the area. +5. Nominated by an existing approver in a PR to `owners.yaml`, with no objection from another approver within three working days. + +**Staying an approver:** no reviews in the area for two sprints moves you back to reviewer at the next sweep. One sprint of reviewing and a request restores you within six months without re-qualifying. On loan or leave, step back for the duration. Leaving the team removes you from every alias. + +The same criteria were applied to everyone at launch, using the last three sprints, and are applied the same way at every quarterly sweep. The tech leads are the fallback approver in every repository root, not the routine merger. + ### Feature Owner (Level 4) A Feature delivers customer-facing value within a Quarter/Release and spans several Epics, often across teams. Feature ownership is **shared between the Architect and the Tech Lead**, working as a pair — the Architect owns the direction, the Tech Lead owns getting it delivered across Epics. @@ -131,7 +160,7 @@ Example of a facilitation comment on an Epic: - **Epic Owner (engineer)** — owns *delivering one Epic*: breakdown, readiness, tracking, and demo, within the release. Altitude: execution of a single Epic. - Beyond co-owning Features, the **Tech Lead** also owns *sprint execution across Epics*: planning, commitment, capacity, and unblocking, and holds Epic Owners accountable for delivery. -> **See also:** the [HyperFleet Team Operating Model](https://docs.google.com/document/d/1Pqq9EdWGBuMR00wL7sguZHGFiP5_f4aQ2VPoDcW5HCA/edit?tab=t.bthlqz8tix2a) for the full role ladder, Domain Ownership model, decision-making framework, and escalation paths (the canonical source for team roles); the [Sprint Planning — Architect & Tech Lead Roles](https://docs.google.com/document/d/1TV5hNX02fgsHVD_2SHjg9SpmMbPHfiDq1RGLYOz8s3I/edit) document for how these roles interact during sprint planning; and the [Work Assignment Process](https://docs.google.com/document/d/1Pqq9EdWGBuMR00wL7sguZHGFiP5_f4aQ2VPoDcW5HCA/edit?tab=t.dssq8ihx7z3) (a tab of the Operating Model) for ticket assignment, the Epic Owner model, and reactive-work capacity. +> **See also:** the [HyperFleet Team Operating Model](https://docs.google.com/document/d/1Pqq9EdWGBuMR00wL7sguZHGFiP5_f4aQ2VPoDcW5HCA/edit?tab=t.bthlqz8tix2a) for the full role ladder, Domain Ownership model, decision-making framework, and escalation paths (the canonical source for team roles; repository reviewer and approver roles are governed by this agreement and [Repository Ownership](owners.md)); the [Sprint Planning — Architect & Tech Lead Roles](https://docs.google.com/document/d/1TV5hNX02fgsHVD_2SHjg9SpmMbPHfiDq1RGLYOz8s3I/edit) document for how these roles interact during sprint planning; and the [Work Assignment Process](https://docs.google.com/document/d/1Pqq9EdWGBuMR00wL7sguZHGFiP5_f4aQ2VPoDcW5HCA/edit?tab=t.dssq8ihx7z3) (a tab of the Operating Model) for ticket assignment, the Epic Owner model, and reactive-work capacity. --- @@ -161,13 +190,15 @@ Example of a facilitation comment on an Epic: ### 4. Review and Merge -- Multiple commits per PR are fine, but each commit message should be meaningful -- Allow **24 hours** for peer review (accounts for time zone differences) -- If a PR has no review activity after 24 hours, bump the Slack thread with a reminder -- For urgent changes, use judgement but document rationale clearly -- For major architectural changes, wait for at least one Technical Leader review +- Prow labels the PR with its `area/*` labels and requests two reviewers from the area's `OWNERS` pool. Anyone else is welcome to review as well +- A merge needs **one approval from an area approver** for every path the PR touches (see [Repository Ownership](owners.md#how-a-merge-works)). A reviewer's approval sets `lgtm`; the reviewer then `/assign`s an area approver to complete it +- Approvers do not approve their own PRs +- Reviewers **acknowledge within four working hours and give a substantive review within one business day**, or say so on the PR and `/unassign` so Prow picks someone else. If a PR has no review activity after one business day, bump the Slack thread +- Multiple commits per PR are fine. **Once review has started, push fixes as new commits**; do not amend or force-push, so the reviewer can see what changed since their last pass. Rebase only to resolve a conflict +- Tide **squashes on merge** with the PR title as the commit message, so the PR title must follow the [commit standard](../../hyperfleet/standards/commit-standard.md). Commits inside the PR do not reach `main` +- In an emergency (broken `main`, live incident) with no area approver available, a tech lead approves and says so in the PR +- An approver who wants a second review on a risky change (migrations, tenant isolation, release plumbing, RBAC) asks for it on the PR with `/hold` until it lands - Use the Jira Development Panel to track PR status directly from the ticket (see [GitHub-Jira Integration](github-jira-integration.md)) -- Merge once approved with no objections ### 5. Close the Ticket @@ -191,13 +222,22 @@ Example of a facilitation comment on an Epic: - Everyone is welcome to add comments to a review — every question is valid - Be constructive and specific -- Distinguish between blocking issues and suggestions (prefix with `nit:` for non-blocking) +- Say what kind of comment it is: **fix before merge**, **follow-up** (with a ticket or a named place it is tracked), or **FYI** (prefix with `nit:` for non-blocking). Pre-production, only reproducible failures block +- Read the whole diff. A review that only covers the first file is not a review - If you approve with suggestions, trust the author to address them - Don't block PRs on style preferences already covered by linting +### Approver Guidelines + +- Approving means: I read the whole diff, I checked the behavioural claims against the code, and I would be comfortable debugging this in production +- Do not approve what you have not verified. Say when a change is outside your depth and name who should look +- Approve the change, not the author + ### Author Guidelines -- Keep PRs focused and reviewable (smaller is better) +- You own every claim in your PR, whoever or whatever wrote the description. Behavioural claims cite a file and line or a test +- Read your own diff before you ask a person to. A green bot is not the same as ready for review +- Keep PRs focused and reviewable (smaller is better). Over about 400 non-generated lines, add a short review plan to the description - Respond to all review comments, even if just acknowledging - If you disagree with feedback, discuss it — don't ignore it diff --git a/hyperfleet/standards/commit-standard.md b/hyperfleet/standards/commit-standard.md index f357366d..dc902046 100644 --- a/hyperfleet/standards/commit-standard.md +++ b/hyperfleet/standards/commit-standard.md @@ -80,6 +80,8 @@ When there is no associated JIRA ticket (e.g., intermediate commits during devel The **header** is mandatory. > **Note:** PR titles **always** require the JIRA prefix (`HYPERFLEET-XXX - : `). Since HyperFleet uses squash merges, the PR title becomes the final commit on `main` and must be traceable to a JIRA ticket. +> +> Commits inside a PR need only the `: ` shape. Once review has started, push fixes as new commits rather than amending, so the reviewer can see what changed. Squash on merge keeps them off `main`. ### Character Limits @@ -198,7 +200,7 @@ This standard is enforced through automated tooling at two levels: ### CI Validation -- **Prow** runs [`hyperfleet-hooks commitlint --pr`](https://github.com/openshift-hyperfleet/hyperfleet-hooks) on every pull request to validate all commit messages and the PR title +- **Prow** runs [`hyperfleet-hooks commitlint --pr`](https://github.com/openshift-hyperfleet/hyperfleet-hooks) on every pull request to validate the PR title, which becomes the squashed commit on `main` - PR titles **MUST** include a JIRA prefix (`HYPERFLEET-XXX - : `) because squash merges use the PR title as the final commit message - See the [commitlint documentation](https://github.com/openshift-hyperfleet/hyperfleet-hooks/blob/main/docs/commitlint.md) for Prow configuration details From 8018b093e366c846aacc90c81e74ce752af930c5 Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Mon, 12 Oct 2026 12:14:29 +0100 Subject: [PATCH 02/13] HYPERFLEET-1762 - chore: draft and hold mode for the OWNERS PR script, merge order in the body Co-Authored-By: Claude Fable 5.1 --- hack/owners/open_prs.sh | 54 +++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/hack/owners/open_prs.sh b/hack/owners/open_prs.sh index 88b9e5cc..a5a7b301 100755 --- a/hack/owners/open_prs.sh +++ b/hack/owners/open_prs.sh @@ -2,10 +2,12 @@ # Open one PR per repository that replaces its OWNERS files with the rendered set # from hack/owners/out. Run gen_owners.py render first. # -# Usage: hack/owners/open_prs.sh [--dry-run] [repo ...] +# Usage: hack/owners/open_prs.sh [--dry-run] [--draft] [repo ...] # Pushes to a branch on the caller's fork (created if missing) and opens the PR # with gh. Existing OWNERS files that the generator does not produce are removed, -# so stale per-directory files go in the same change. +# so stale per-directory files go in the same change. With --draft the PR opens +# as a draft and holds itself, so it cannot merge until it is marked ready and +# the hold is lifted. set -euo pipefail ORG="openshift-hyperfleet" @@ -14,11 +16,17 @@ OUT="${ROOT}/hack/owners/out" BRANCH="${BRANCH:-HYPERFLEET-1200-owners-areas}" TITLE="${TITLE:-HYPERFLEET-1200 - chore: narrow OWNERS to area owners and aliases}" DOC_URL="https://github.com/${ORG}/architecture/blob/main/hyperfleet/docs/owners.md" +DOC_PR="${DOC_PR:-openshift-hyperfleet/architecture#232}" DRY_RUN=0 -if [[ "${1:-}" == "--dry-run" ]]; then - DRY_RUN=1 +DRAFT=0 +while [[ "${1:-}" == --* ]]; do + case "$1" in + --dry-run) DRY_RUN=1 ;; + --draft) DRAFT=1 ;; + *) echo "unknown flag: $1" >&2; exit 2 ;; + esac shift -fi +done FORK_OWNER="$(gh api user --jq .login)" WORK="$(mktemp -d)" @@ -30,19 +38,43 @@ if [[ $# -gt 0 ]]; then fi read -r -d '' BODY < Date: Mon, 12 Oct 2026 12:15:00 +0100 Subject: [PATCH 03/13] HYPERFLEET-1762 - chore: distinct branch name for the OWNERS PRs The default collided with this branch on the same fork, so the push to the architecture repository was refused. Co-Authored-By: Claude Fable 5.1 --- hack/owners/open_prs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/owners/open_prs.sh b/hack/owners/open_prs.sh index a5a7b301..f3479284 100755 --- a/hack/owners/open_prs.sh +++ b/hack/owners/open_prs.sh @@ -13,7 +13,7 @@ set -euo pipefail ORG="openshift-hyperfleet" ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" OUT="${ROOT}/hack/owners/out" -BRANCH="${BRANCH:-HYPERFLEET-1200-owners-areas}" +BRANCH="${BRANCH:-HYPERFLEET-1200-owners-files}" TITLE="${TITLE:-HYPERFLEET-1200 - chore: narrow OWNERS to area owners and aliases}" DOC_URL="https://github.com/${ORG}/architecture/blob/main/hyperfleet/docs/owners.md" DOC_PR="${DOC_PR:-openshift-hyperfleet/architecture#232}" From 8a2e3b8460af630934b00a1b0d860fd28ea8ffd5 Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Mon, 12 Oct 2026 12:16:38 +0100 Subject: [PATCH 04/13] HYPERFLEET-1762 - chore: make the OWNERS PR script safe to rerun Overwrite the single-purpose fork branch and reuse an open PR for it. Co-Authored-By: Claude Fable 5.1 --- hack/owners/open_prs.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/hack/owners/open_prs.sh b/hack/owners/open_prs.sh index f3479284..e52ffa95 100755 --- a/hack/owners/open_prs.sh +++ b/hack/owners/open_prs.sh @@ -106,7 +106,13 @@ for repo in "${REPOS[@]}"; do gh repo fork "${ORG}/${repo}" --clone=false >/dev/null fi git remote add fork "git@github.com:${FORK_OWNER}/${repo}.git" - git push -q --force-with-lease fork "${BRANCH}" + # the branch is ours and single purpose, so overwrite whatever a previous run left + git push -q --force fork "${BRANCH}" + existing="$(gh pr list --repo "${ORG}/${repo}" --state open --head "${BRANCH}" --json url --jq '.[0].url // empty')" + if [[ -n "${existing}" ]]; then + echo " PR already open: ${existing}" + exit 0 + fi body="${BODY}" draft_flag=() if [[ "${DRAFT}" -eq 1 ]]; then From ae87121ea23fffcca7140ff600eaf6bfe0c6e91f Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Mon, 12 Oct 2026 12:19:02 +0100 Subject: [PATCH 05/13] HYPERFLEET-1762 - chore: handle repositories without OWNERS and non-fork copies in the PR script Co-Authored-By: Claude Fable 5.1 --- hack/owners/open_prs.sh | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/hack/owners/open_prs.sh b/hack/owners/open_prs.sh index e52ffa95..90263d95 100755 --- a/hack/owners/open_prs.sh +++ b/hack/owners/open_prs.sh @@ -81,12 +81,13 @@ for repo in "${REPOS[@]}"; do echo "== ${ORG}/${repo}" src="${OUT}/${repo}" dir="${WORK}/${repo}" - git clone --quiet "https://github.com/${ORG}/${repo}.git" "${dir}" + git clone --quiet "git@github.com:${ORG}/${repo}.git" "${dir}" ( cd "${dir}" git checkout -q -b "${BRANCH}" # remove every tracked OWNERS and OWNERS_ALIASES, then copy the rendered set - git ls-files -z | grep -z -E '(^|/)OWNERS(_ALIASES)?$' | xargs -0 -r git rm -q + # a repository with no OWNERS yet is fine, grep then matches nothing + { git ls-files -z | grep -z -E '(^|/)OWNERS(_ALIASES)?$' || true; } | xargs -0 -r git rm -q (cd "${src}" && find . -type f -print0) | while IFS= read -r -d '' f; do mkdir -p "$(dirname "${f}")" cp "${src}/${f}" "${f}" @@ -102,12 +103,20 @@ for repo in "${REPOS[@]}"; do exit 0 fi git commit -q -m "${TITLE}" - if ! gh repo view "${FORK_OWNER}/${repo}" >/dev/null 2>&1; then - gh repo fork "${ORG}/${repo}" --clone=false >/dev/null + # Push to the caller's fork, or to a branch on the org repository when the + # caller's copy is not registered as a fork (GitHub refuses a cross-repo PR then). + head_ref="${FORK_OWNER}:${BRANCH}" + if [[ "${PUSH_TO_ORIGIN:-0}" -eq 1 ]]; then + git push -q --force origin "${BRANCH}" + head_ref="${BRANCH}" + else + if ! gh repo view "${FORK_OWNER}/${repo}" >/dev/null 2>&1; then + gh repo fork "${ORG}/${repo}" --clone=false >/dev/null + fi + git remote add fork "git@github.com:${FORK_OWNER}/${repo}.git" + # the branch is ours and single purpose, so overwrite whatever a previous run left + git push -q --force fork "${BRANCH}" fi - git remote add fork "git@github.com:${FORK_OWNER}/${repo}.git" - # the branch is ours and single purpose, so overwrite whatever a previous run left - git push -q --force fork "${BRANCH}" existing="$(gh pr list --repo "${ORG}/${repo}" --state open --head "${BRANCH}" --json url --jq '.[0].url // empty')" if [[ -n "${existing}" ]]; then echo " PR already open: ${existing}" @@ -119,7 +128,7 @@ for repo in "${REPOS[@]}"; do draft_flag=(--draft) body="${BODY}"$'\n\n/hold' fi - gh pr create --repo "${ORG}/${repo}" --head "${FORK_OWNER}:${BRANCH}" --base main \ + gh pr create --repo "${ORG}/${repo}" --head "${head_ref}" --base main \ "${draft_flag[@]}" --title "${TITLE}" --body "${body}" ) done From 648dc1afa70ebd8d37090a3e03b72a5bce04a84f Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Mon, 12 Oct 2026 12:42:05 +0100 Subject: [PATCH 06/13] HYPERFLEET-1762 - chore: OWNERS PR script defaults carry the rollout ticket key Co-Authored-By: Claude Fable 5.1 --- hack/owners/open_prs.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/owners/open_prs.sh b/hack/owners/open_prs.sh index 90263d95..0a411483 100755 --- a/hack/owners/open_prs.sh +++ b/hack/owners/open_prs.sh @@ -13,8 +13,8 @@ set -euo pipefail ORG="openshift-hyperfleet" ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" OUT="${ROOT}/hack/owners/out" -BRANCH="${BRANCH:-HYPERFLEET-1200-owners-files}" -TITLE="${TITLE:-HYPERFLEET-1200 - chore: narrow OWNERS to area owners and aliases}" +BRANCH="${BRANCH:-HYPERFLEET-1762-owners-files}" +TITLE="${TITLE:-HYPERFLEET-1762 - chore: narrow OWNERS to area owners and aliases}" DOC_URL="https://github.com/${ORG}/architecture/blob/main/hyperfleet/docs/owners.md" DOC_PR="${DOC_PR:-openshift-hyperfleet/architecture#232}" DRY_RUN=0 From 8c9d154588e2c8c8e43b9b96acaea377367a00a1 Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Mon, 12 Oct 2026 12:48:03 +0100 Subject: [PATCH 07/13] HYPERFLEET-1762 - fix: clear the OWNERS output directory before rendering A repository or path removed from owners.yaml left its rendered file behind, and the PR script would have copied it. The output directory is now cleared on every render, refusing the repository root and the current directory. Co-Authored-By: Claude Fable 5.1 --- hack/owners/gen_owners.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/hack/owners/gen_owners.py b/hack/owners/gen_owners.py index 62797d05..c6506085 100755 --- a/hack/owners/gen_owners.py +++ b/hack/owners/gen_owners.py @@ -13,6 +13,7 @@ """ import argparse import os +import shutil import sys import yaml @@ -134,7 +135,18 @@ def splice_table(text, new_table): return text[:start] + "\n" + new_table + "\n" + text[end:] +def clear_out_dir(out_dir): + """Remove a previous render so the output reflects only the current repository set.""" + out_dir = os.path.abspath(out_dir) + protected = {os.path.abspath(os.sep), ROOT, os.getcwd()} + if out_dir in protected or ROOT.startswith(out_dir + os.sep): + raise SystemExit(f"refusing to clear {out_dir}: pass a dedicated output directory") + shutil.rmtree(out_dir, ignore_errors=True) + os.makedirs(out_dir, exist_ok=True) + + def render(data, out_dir): + clear_out_dir(out_dir) count = 0 for repo, spec_map in data["repos"].items(): base = os.path.join(out_dir, repo) From 56cf956f4bec1594915073a235c0b7949f8e883c Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Mon, 12 Oct 2026 12:54:18 +0100 Subject: [PATCH 08/13] HYPERFLEET-1762 - fix: harden the OWNERS scripts and correct the reviewer hand-off command Review feedback on the generator and the two scripts: - gen_owners.py resolves symlinks before clearing the output directory and refuses the filesystem root, the repository, the current directory, any parent of the repository, and any path inside the repository other than hack/owners/. - labels.sh passes the YAML path to Python as an argument instead of interpolating it into source. - open_prs.sh checks the rendered directory exists before listing repositories, requires a rendered root OWNERS per repository before cloning so a bad name can never stage a root OWNERS deletion, keeps git ls-files failures fatal while tolerating an empty grep match (through files, so NUL separators survive), and only force-updates a remote branch when an open PR with the expected head owner and this title exists, leased on the observed OID. - owners.md and the working agreement tell a reviewer who cannot meet the timeline to /uncc and /auto-cc, since Blunderbuss manages review requests and only re-runs on request. Co-Authored-By: Claude Fable 5.1 --- hack/owners/gen_owners.py | 29 ++++++++++++----- hack/owners/labels.sh | 19 ++++++++--- hack/owners/open_prs.sh | 48 ++++++++++++++++++++++------ hyperfleet/docs/owners.md | 2 +- hyperfleet/docs/working-agreement.md | 2 +- 5 files changed, 76 insertions(+), 24 deletions(-) diff --git a/hack/owners/gen_owners.py b/hack/owners/gen_owners.py index c6506085..1328d999 100755 --- a/hack/owners/gen_owners.py +++ b/hack/owners/gen_owners.py @@ -136,17 +136,30 @@ def splice_table(text, new_table): def clear_out_dir(out_dir): - """Remove a previous render so the output reflects only the current repository set.""" - out_dir = os.path.abspath(out_dir) - protected = {os.path.abspath(os.sep), ROOT, os.getcwd()} - if out_dir in protected or ROOT.startswith(out_dir + os.sep): - raise SystemExit(f"refusing to clear {out_dir}: pass a dedicated output directory") - shutil.rmtree(out_dir, ignore_errors=True) - os.makedirs(out_dir, exist_ok=True) + """Remove a previous render so the output reflects only the current repository set. + + Symlinks are resolved first so the check and the deletion see the same real path. + The target may not be the filesystem root, the repository, the current directory, + or a parent of the repository. Inside the repository only hack/owners/ is allowed, + so a stray --out can never delete documentation. + """ + target = os.path.realpath(out_dir) + root = os.path.realpath(ROOT) + cwd = os.path.realpath(os.getcwd()) + allowed_in_root = os.path.join(root, "hack", "owners") + if target in {os.path.realpath(os.sep), root, cwd}: + raise SystemExit(f"refusing to clear {target}: pass a dedicated output directory") + if os.path.commonpath([target, root]) == target: + raise SystemExit(f"refusing to clear {target}: it contains the repository") + if os.path.commonpath([target, root]) == root and os.path.commonpath([target, allowed_in_root]) != allowed_in_root: + raise SystemExit(f"refusing to clear {target}: inside the repository only {allowed_in_root} may be used") + shutil.rmtree(target, ignore_errors=True) + os.makedirs(target, exist_ok=True) + return target def render(data, out_dir): - clear_out_dir(out_dir) + out_dir = clear_out_dir(out_dir) count = 0 for repo, spec_map in data["repos"].items(): base = os.path.join(out_dir, repo) diff --git a/hack/owners/labels.sh b/hack/owners/labels.sh index d5dc97ad..668e5c17 100755 --- a/hack/owners/labels.sh +++ b/hack/owners/labels.sh @@ -24,15 +24,24 @@ color_for() { esac } -mapfile -t REPOS < <(python3 -c "import yaml; print('\n'.join(yaml.safe_load(open('${SOURCE}'))['repos']))") +mapfile -t REPOS < <(python3 - "${SOURCE}" <<'PY' +import sys +import yaml +with open(sys.argv[1]) as f: + print("\n".join(yaml.safe_load(f)["repos"])) +PY +) if [[ $# -gt 0 ]]; then REPOS=("$@") fi -mapfile -t LABELS < <(python3 -c " +mapfile -t LABELS < <(python3 - "${SOURCE}" <<'PY' +import sys import yaml -for name, desc in yaml.safe_load(open('${SOURCE}'))['labels'].items(): - print(f'{name}\t{desc}') -") +with open(sys.argv[1]) as f: + for name, desc in yaml.safe_load(f)["labels"].items(): + print(f"{name}\t{desc}") +PY +) for repo in "${REPOS[@]}"; do echo "== ${ORG}/${repo}" diff --git a/hack/owners/open_prs.sh b/hack/owners/open_prs.sh index 0a411483..112fffa3 100755 --- a/hack/owners/open_prs.sh +++ b/hack/owners/open_prs.sh @@ -32,9 +32,14 @@ FORK_OWNER="$(gh api user --jq .login)" WORK="$(mktemp -d)" trap 'rm -rf "${WORK}"' EXIT -mapfile -t REPOS < <(ls "${OUT}") if [[ $# -gt 0 ]]; then REPOS=("$@") +else + if [[ ! -d "${OUT}" ]]; then + echo "rendered OWNERS directory not found: ${OUT} (run 'make owners' first)" >&2 + exit 1 + fi + mapfile -t REPOS < <(ls -A "${OUT}") fi read -r -d '' BODY <&2 + exit 1 + fi git clone --quiet "git@github.com:${ORG}/${repo}.git" "${dir}" ( cd "${dir}" git checkout -q -b "${BRANCH}" - # remove every tracked OWNERS and OWNERS_ALIASES, then copy the rendered set - # a repository with no OWNERS yet is fine, grep then matches nothing - { git ls-files -z | grep -z -E '(^|/)OWNERS(_ALIASES)?$' || true; } | xargs -0 -r git rm -q + # remove every tracked OWNERS and OWNERS_ALIASES, then copy the rendered set. + # git ls-files failures stay fatal; only grep's "no match" (exit 1) is tolerated, + # because a repository with no OWNERS yet is fine. + # NUL-separated lists go through files: command substitution would drop the NULs. + git ls-files -z > "${WORK}/${repo}.tracked" + grep -z -E '(^|/)OWNERS(_ALIASES)?$' "${WORK}/${repo}.tracked" > "${WORK}/${repo}.owners" || [[ $? -eq 1 ]] + xargs -0 -r git rm -q < "${WORK}/${repo}.owners" (cd "${src}" && find . -type f -print0) | while IFS= read -r -d '' f; do mkdir -p "$(dirname "${f}")" cp "${src}/${f}" "${f}" @@ -105,19 +119,35 @@ for repo in "${REPOS[@]}"; do git commit -q -m "${TITLE}" # Push to the caller's fork, or to a branch on the org repository when the # caller's copy is not registered as a fork (GitHub refuses a cross-repo PR then). - head_ref="${FORK_OWNER}:${BRANCH}" if [[ "${PUSH_TO_ORIGIN:-0}" -eq 1 ]]; then - git push -q --force origin "${BRANCH}" + remote="origin" + head_owner="${ORG}" head_ref="${BRANCH}" else + remote="fork" + head_owner="${FORK_OWNER}" + head_ref="${FORK_OWNER}:${BRANCH}" if ! gh repo view "${FORK_OWNER}/${repo}" >/dev/null 2>&1; then gh repo fork "${ORG}/${repo}" --clone=false >/dev/null fi git remote add fork "git@github.com:${FORK_OWNER}/${repo}.git" - # the branch is ours and single purpose, so overwrite whatever a previous run left - git push -q --force fork "${BRANCH}" fi - existing="$(gh pr list --repo "${ORG}/${repo}" --state open --head "${BRANCH}" --json url --jq '.[0].url // empty')" + # An existing remote branch is only overwritten when it is this workflow's: an open + # PR in the org repository with that head, the expected owner and this title. The + # update is then leased on the OID observed here, never a blind force push. + remote_oid="$(git ls-remote "${remote}" "refs/heads/${BRANCH}" | cut -f1)" + existing="$(gh pr list --repo "${ORG}/${repo}" --state open --head "${BRANCH}" \ + --json url,title,headRefName,headRepositoryOwner | + jq -r --arg owner "${head_owner}" --arg branch "${BRANCH}" --arg title "${TITLE}" \ + '[.[] | select(.headRepositoryOwner.login == $owner and .headRefName == $branch and .title == $title)][0].url // empty')" + if [[ -z "${remote_oid}" ]]; then + git push -q "${remote}" "${BRANCH}" + elif [[ -n "${existing}" ]]; then + git push -q --force-with-lease="${BRANCH}:${remote_oid}" "${remote}" "${BRANCH}" + else + echo " branch ${BRANCH} already exists on ${remote} (${remote_oid:0:7}) with no open PR titled '${TITLE}' from ${head_owner}; refusing to overwrite. Set BRANCH to a different name or remove the branch." >&2 + exit 1 + fi if [[ -n "${existing}" ]]; then echo " PR already open: ${existing}" exit 0 diff --git a/hyperfleet/docs/owners.md b/hyperfleet/docs/owners.md index 89c13ce0..f6c0fb22 100644 --- a/hyperfleet/docs/owners.md +++ b/hyperfleet/docs/owners.md @@ -125,7 +125,7 @@ The [working agreement](working-agreement.md#code-review) says review for correc ### Reviewers -- **Acknowledge within four working hours, substantive review within one business day.** If you cannot, say so on the PR and `/unassign` so Blunderbuss picks someone else. +- **Acknowledge within four working hours, substantive review within one business day.** If you cannot, say so on the PR, `/uncc` yourself and `/auto-cc` so Blunderbuss requests someone else from the area's pool. - **Read the whole diff.** A review that only covers the first file is not a review. - **Say what kind of comment it is.** Every comment is one of: fix before merge, follow-up (with a ticket or a named place it is tracked), or FYI. Pre-production, only reproducible failures block. - **Aim for about four reviews a week.** That is roughly three hours. It is not forty. diff --git a/hyperfleet/docs/working-agreement.md b/hyperfleet/docs/working-agreement.md index c6e134cf..39490060 100644 --- a/hyperfleet/docs/working-agreement.md +++ b/hyperfleet/docs/working-agreement.md @@ -193,7 +193,7 @@ Example of a facilitation comment on an Epic: - Prow labels the PR with its `area/*` labels and requests two reviewers from the area's `OWNERS` pool. Anyone else is welcome to review as well - A merge needs **one approval from an area approver** for every path the PR touches (see [Repository Ownership](owners.md#how-a-merge-works)). A reviewer's approval sets `lgtm`; the reviewer then `/assign`s an area approver to complete it - Approvers do not approve their own PRs -- Reviewers **acknowledge within four working hours and give a substantive review within one business day**, or say so on the PR and `/unassign` so Prow picks someone else. If a PR has no review activity after one business day, bump the Slack thread +- Reviewers **acknowledge within four working hours and give a substantive review within one business day**, or say so on the PR, `/uncc` yourself and `/auto-cc` so Prow requests someone else from the area's pool. If a PR has no review activity after one business day, bump the Slack thread - Multiple commits per PR are fine. **Once review has started, push fixes as new commits**; do not amend or force-push, so the reviewer can see what changed since their last pass. Rebase only to resolve a conflict - Tide **squashes on merge** with the PR title as the commit message, so the PR title must follow the [commit standard](../../hyperfleet/standards/commit-standard.md). Commits inside the PR do not reach `main` - In an emergency (broken `main`, live incident) with no area approver available, a tech lead approves and says so in the PR From 659d32f6b0280f6d92b52c1ecab6c65d0af6a85a Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Mon, 12 Oct 2026 13:17:18 +0100 Subject: [PATCH 09/13] HYPERFLEET-1762 - fix: only a directory under hack/owners may be cleared by the generator The allowlist accepted hack/owners itself, which would have deleted the generator and the scripts. The target must now be a strict descendant. Co-Authored-By: Claude Fable 5.1 --- hack/owners/gen_owners.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/hack/owners/gen_owners.py b/hack/owners/gen_owners.py index 1328d999..94e385aa 100755 --- a/hack/owners/gen_owners.py +++ b/hack/owners/gen_owners.py @@ -140,8 +140,9 @@ def clear_out_dir(out_dir): Symlinks are resolved first so the check and the deletion see the same real path. The target may not be the filesystem root, the repository, the current directory, - or a parent of the repository. Inside the repository only hack/owners/ is allowed, - so a stray --out can never delete documentation. + or a parent of the repository. Inside the repository only a directory under + hack/owners/ is allowed (not hack/owners itself), so a stray --out can never + delete documentation or these scripts. """ target = os.path.realpath(out_dir) root = os.path.realpath(ROOT) @@ -151,8 +152,10 @@ def clear_out_dir(out_dir): raise SystemExit(f"refusing to clear {target}: pass a dedicated output directory") if os.path.commonpath([target, root]) == target: raise SystemExit(f"refusing to clear {target}: it contains the repository") - if os.path.commonpath([target, root]) == root and os.path.commonpath([target, allowed_in_root]) != allowed_in_root: - raise SystemExit(f"refusing to clear {target}: inside the repository only {allowed_in_root} may be used") + inside_root = os.path.commonpath([target, root]) == root + strict_descendant = target != allowed_in_root and os.path.commonpath([target, allowed_in_root]) == allowed_in_root + if inside_root and not strict_descendant: + raise SystemExit(f"refusing to clear {target}: inside the repository only a directory under {allowed_in_root} may be used") shutil.rmtree(target, ignore_errors=True) os.makedirs(target, exist_ok=True) return target From 8a210e91f5771893c227d83cbf1ea59b44646ebb Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Tue, 13 Oct 2026 10:40:25 +0100 Subject: [PATCH 10/13] HYPERFLEET-1762 - fix: verify OWNERS paths against the area table and run the scripts on bash 3.2 Every repository path in owners.yaml now names its area, and gen_owners.py check fails when the path's approvers or reviewers differ from that row of the table, so the table and the OWNERS files cannot drift. The paths that had drifted are aligned to the table: .github, e2e/auth, infra helmfile, pkg/middleware, operator config/manifests, the claude-plugins root and six per-plugin files. The two infra sub-paths that became identical to the root are removed. labels.sh and open_prs.sh no longer use mapfile or unguarded empty-array expansion, so they run on the bash 3.2 that macOS ships. The stray __pycache__ file is untracked and ignored. Co-Authored-By: Claude Fable 5.1 --- .gitignore | 1 + .../__pycache__/gen_owners.cpython-314.pyc | Bin 12759 -> 0 bytes hack/owners/gen_owners.py | 70 +++++++- hack/owners/labels.sh | 13 +- hack/owners/open_prs.sh | 13 +- hyperfleet/docs/owners.md | 4 +- hyperfleet/docs/owners.yaml | 168 +++++++++--------- 7 files changed, 177 insertions(+), 92 deletions(-) delete mode 100644 hack/owners/__pycache__/gen_owners.cpython-314.pyc diff --git a/.gitignore b/.gitignore index 652fb3b9..9550df6a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ hack/owners/out/ +__pycache__/ diff --git a/hack/owners/__pycache__/gen_owners.cpython-314.pyc b/hack/owners/__pycache__/gen_owners.cpython-314.pyc deleted file mode 100644 index 6b49cea2faf5f7c431fc41462c947a1cf6638ed4..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 12759 zcmbt4ZEO=~nltv;9((+mPbV?SgrpP$`2a$J04WJcAOwOt1c$aMUK39e7soa;4uO>2 zy7$$j(8l!Gjl0*X?rrz#J+->%w71G?_awBZ(@|-Em^vTcAyO-Cb-Ml6LZ$BZ$LXH; z9gpoKrc3wUD6ePc{d(T#`+44Xr`cp6pfvpIw=XtU6U6V)i&AvihlhlUAfg0EM2HIn zNA{5s691|qD*UUCsPQiqp}XHAiupHcFl&!hGkk+T2*t)Z3wC5w^RJdx2%yDZwA%vnzW#PxW=4?Kz9r&H@ALz`$@w^hZL% zHJI};%61<=>YWTk*m3X3r7UbMu%;7FEbyZOFOK8Q;VnJhr2gQec0J+SwAGPue&o_d zNJqW?0P7#kFU-Q6S@cZ!Sii`M5iejkB%m&j2i3Ekr%$1g{@_R`DEh?+AB=2*Mkd?_ zaKeH=7{Rc31>P$Y*I?i#G3~?QfS>oV-iSi9h<}1-p(6|Ao}7S1zkUgvo_%!v;%sC2;0=u zFnnR?AoK#J!Jvm7V6T?|uLJ%&k(m(%OPq+zX^rU)H#eu%%2t|k0 z5e)=O>?EGd4j6+K9fV(1M{FVL@E->~4ym^ge)4DJfRmD_P?!%&+OucQamRWj3L#t4 zi{4S*69{>IPEsOAMYI7{I^HrKn&4YTgb?szi%ST-2v`tX@)T()#7#?~q^y%#i9$0A zNQ2kGzxV`HQDRMRx;Z-Mxao-7)-0uSKX~m2ac|nfeyt`96?2X`#|_6fq8@t2D-LRY z$#fd@BRRCfSA;T*^1s59Y?2SVe}Y`a1i|rLAXg58oi;o z)e55P2)D|m+0YEHg+2OP2trTPLeB<5prO&Jmee92k;NJ$~B$&|$t&jIjf|{y?mqz#(_d^9JvX=h6CML`@+&7iXkQv7K#*vKC zI(K6BMC_TH{TZWW?&R#r*yT4)f2kwPrC-p5(GorR5QY*XImUw*v=5A{C_F@goj76_ zm~^FtRn34QHwn0E09;XnYF8nqv)zWe4#JZUDB>W78VkptR=KHWGLN}_pfAG9a1O(H ztThn8U^;KW?-hB`sgkt(RZurRym6-nB-|VEN}8Z|f)@n@VisRP5$$1H0mAsJVX!U& z;wK<5A+|#mB{Jrc*umFB(c>9|F?vkK8qLfZW(^73`;OTDAGI%ZF6>>Q_kXDdgniD~ zVhuMArKk$Q4#U^+7osP?Pd)rKp4d$k!wE+=6Kz>cTZwZZ*v-VGsuvuP%eK7e7jPbk zfSVjp^_R*R3ai58aE{A{6}Ji~RCG0%U0F@Rwp+Ce;8Tk7wE>{0ga0t;2d11p%JEnH z{1j*{HXN7~vBC~}BjZnkg;w<0E_T8n%(jhs1A&X6OxY}*nk9X%pQIO*$$+)hW5Dj# z-p*5}x;l^bE4CL?6NDa235WzGhWQbR^a}L=rigd~$SAYDDL0pAG$aVyp=}@P^bAx{ z;xh;P?uoZfB%WW`y5!iGww}G`*cZFJ)WzL9$j#egHh7KprL1Sy7~Ac}SY-a{t*Z$p z$)zh>(1ipc-`p|P`y`X?<^>!?jZ6N2pF|KOEdh%osG z8st>5qHui`NFoY&eS>rt%$!z#BbHy`i=NLW2bqTY{yLZ!@_8?_k< zRiTV3{Q(LUo|gnkRAZ6|TVWkZ8}MG_10sBDfL8%Tn85Ua52(MS2HT5!uBV_uHn$$s zia7BQ{EIwPAd5%@GdkwgtFK&*F{?VqCpyPkOqwQg1GTZZDTRb}grZ>khq8(`7e%>qi~;0T$>c?9)Tt9tR3vGycmtE5S}}Psp^?l9 zuqw2PApu-6r$%s~R~m_}XTlqns7aCcVJ;AAp=F~gHlk)vR#galpvQ64YXmB=V2t_3 zzG!#GYM-~dCWIIerr7bQnD*u(UP{c{`|-1Z~pp@t|qPJ-UHZT$)iT}iC*#^DrIxmj~-VPVVyh@{|*K`j?o zA){o!A?FG^ROEg%1`0x4FIXl$LWk!Pe@&E1vax_?G!UBdh>>dn9*~QOly#hZlM`XE zYwFO&7qRQKO;&Fd`Ys%RelGYI%b`LTvCMV6))8+=mfzX8#Pp@8zRi0;1jT#krqCF$ z&t#sJH`;H7F@UupAS-07Vm?j*ql4j`0-OkKZVFfv)IeA>#1&X8*-rv}w6Sn1x7tW> zYPZHn>?vf-MwuaVo)N)Oj}K1|*$WF{h{oKyKLh)JXM4_ar2zZ00K37hFD9sNLtap! z=TYd%3@xn3Q^B4=6o&q_;2orAkSyj24r&1d+Clmdspw!?gds52K@06RK&^K(fFYei zArRfJEQX$!e~ZJ+t%vz3fY~^7hcgu7vH(Va`7eTzaT@_f;~s4Re88>Pr=>X)3jDKb z>6f6EieeWMAz5Eh!GaI?7JTeakf&1&Z*GJ)^PZfd0-S)+MQiqu{V*1jOy#n2S z3^*`hS`T7cn;wJKV2r*it&79VS=}biRtV7s3KwGVh`fh>cXlr(uB31z$mpZ!7Xv)eEi2W#XU#L3 z`!+i^YHz%pu~y8RZkZC=#AISD)pTUhyLdKL+qq)xLguXstXs_<2;*w%;#2Q5tyVYP ztA@G-=AbgwTi>b$H=K zYUh!~^LO|=Lw5&KTc2Mk8~zezYyCn?Sj%F<8`Fu#54OLzJ$Yz#>;9#!`xO+FCA6t+ z-KpB16>INTr9=%{y{7ex3Ig1Np?OoxG(+7lZJIfjDX*G8f9rgFB(Z0ud|Qmnl$Ir` z-Z`^Ux^sd2g++m=C)t?1^3kqj?ZV~7(nZap=k7CikEOP{R?41%tyMe;lb1E40NEPT zy?z9IP_!YYIb$rDJ3V_k-jirdT>0s)L~ZiwBDFa5@1}*9?*voT=T?mFFHMB4_6sw3 z9MRL8{Y?s(0UK(>eqLY+u4w4yCiapXSxE$F6lS+UUsM#;iCm6p)h>2jNP^7nUig2A ze22s2R!E&hf%kfAK&g>uAq%ulsjM^mpa@7e3AJhub(M%}Kox9}RRO1NAoi*KB#0CO zcv{77mq_Qp&rs?@)!ZtP0W4HmA3rLyzy2^}b0FRpIwU)e%$pAfLL=UQ*wLJ8xeT+3 z$d~-%9{|i$S=TS0TJXlH`1yCtiOb7&XR2~Xa%=LL?BTt1wyy#p-_q;R8qN;OxOAv{t&TE6sGJsIJYf zlo5Jk3Lysy(FcNd2#i4uuM3dC#M}VPLLgQ&^v)%Qa%K*cg#+9hw~A92QUc7qDy%;3 zC=3YmvBZ#y(-ij3h4W~)P$2F=u;St_2o+(R_FKnMV7PS3$owy-1Ni8|k-!yt__odj z;cv*H)4M6CnLWAF65yQIvz%je<)WMX{O(-Z2YA>tz5P1di4wurSr=$p5q>+B`~p1Z zvk=#(ZP%Ncn%4abPs6kl?GKLgkj@vU^~w{Qk5Z>q*ID5x^qW@0e3Ay~a}k*Mx%3c&8g!@1hF0kFIlW{>QV<)w@pOu+{uTvQhS|S(J|Um0 zQdAO(C5j0`K2@FTsdmF*-vRJJ$FNAa;m8la0Xsq-V-I@vfz^f_Nsd8>X}~Hjz;0yT z7aS+us^MJlyBVroxj+|-Du}RKMM4}>7-YN2!ngHpLuwBC7q$c2Qhz{@OjovN52q%) z!X=a{oA8UGKR7lZfE!FWwGuVz5Bel#u(Rt_kLT=QCpReR<(Hm;Zpm==8XTNV^j!5r z{02;9NG$D8h-!2ru^&#$eSb{EL(L z0p)%BX8UemN*lLEdol*|oO9NByDx31MM)Op%;mqi7CRQNzkNI|zWvgg+5Yyi_`Vdo zKUH-gZ9bTy4`wL+&9k>_(xxrT)E1b4vBb2o@|nZ&#$~29QMJtMNDeMDyBE3_%N9;9 zMp$wx84K5tO1?gmZQq_Xe$zT>Q&kQ5S7ZuiC0oqkuu@}jCAk~Jup zf;2GQlGD>lI|@_LU;Z4)O%)UPJnecv%2MvfZha1GEXG=PuRyiO#MRKV3Vu$LgPJf? zfbHVcWHwOq|3fE+qm|XaolcO1_*Oa%ql^wNZi9l?jlV_kthzw1S7iPFB2x+i;5X%` z;r#v}u`3s4=3*QLkX9-E^1ll0!p4oX9tJN`F6WKI$h*QNpO9AY6E+R49hd?IzHftc zUXFz7ZZdntyU27I8FJ;0#-#J8Vr_K>GukWV+iz2B){KUS~TAE(zK9QeuooO)px zP$xj9ffxi&mgcX*p@%1QNixfK50oPjiJZcL_kg688=Mv#&q}J0C>%#qAV)=_C%l(< zpC7{6J?I3(=c;4q9XTvYNUVdiZx75OX+}bm!H9s; zYS{?*BAmO)2WyDK1SH4u^4s7^B7MRL`lYeC0>dTZka%58EW1fs`67mJ1-j3|zxW|` zFEU0G*cJ$ZH-@ik7Tes^?9|-#+3WG+Y0C~s1Ji~%<~1hPooHL8x387d#v|#H<{90m zObKL&Z(p1ryEQf+xD`mWrJM)T743`1R&7U@Y)9{OW~{b(-7VeQRNVjZRN6W8@zmY= z4+j?-7q0zV%boh=J>BW1o=jE~426!o zOFy^N#Jk^rB1!#ZXWG&nJ(1B_mUNYCCfn_Xm^gRg=7soW!0z1f+2gSjOU5lQ#_afv zu|`kL?2DfI(}Nmf$9YoTB;YA!JNnJn27m$p{pM>eOg~rk&DS&xdE){e^o`gBRQrvi z;zXP7H+#08Xes-TG9x_yXKQ00t^Y5Lt*=IN*H+Tkthw8wf$w+Qm_7~jG0pVZO&`}V zeLM9ZH#2=tuRoL5!IaoN_(#kPg&Au_;31EVdf>9zmn@>%P(X&}-B5C>O}+6tME(%w zygG06H>)Nl#>pfk#Ux3cr z>%otuhS8}%F5u5X0JcXpj6z&oT-C}`{2Ht?hC7#q-tlKIgq(UatGuXz>6UhM zBPV-xCwF~l7b{E#i{8BYMu*dF?d;DWY)6;8K&+8;P3Y$RRjzcBs z1U`VmHi_cSoEd}=4qYLFYZGwHq7@@>gG|UqYXV|W0c$C&lSqD3sD^Jy39mrE_uyZI z_#K?mH4_{!-M9dGIk;c)C|T2$#=75rB0>Gb&a`e@vT;e*oT1HgrdiY5CGS?gRrzk+ zTXi3ne&|bA?M>O+Qf(ut3f~gV-#3-ckZZIlri+s?ac1KEh7WeUw_}xVg6xvH>@!RG z&Gr;k{(Zi-@eg*Za-c*E%E z7}<{vg@Xg&JOe=!R}&9DzKd2d%6Ka}cha zFDyfW>b%J3ec{|U%B^EjPM!|@pyI2Pwaz1bwI_5&|!iJ(Ky0ESQGNT(EhM9() zN@q94nG5q#3f`fYWf(9k7gu47u6%1zTJyg^ry`IHw<L4SPAi=8GCp1$ z5&YqZKNOTKa0O+;8-XmpXC&YiMM*s};Twcr6!eNnw7_4U^b2qjj(nbJ4ZeX2E}t%Y zIE?iWcEbjQtI$KZhPr8|gb>_Koi10-K(ZHO;ZB_>ve<8&w&gFU`r*=SB*Z$aB-$&C zg}s8vOXf~t3}2UYp;tlDdC+YOk4LKEyi?uz4V(bQ+N*3qGQ%A-k2rY|&jBYP2J|k! zNJjJw^W#}1rn{%N^W3RH&zWkkU^@cl#h7dt>fVQs%^YW&31`18u&ZeKkaymvCVWD3DCjKR8UsJmyVOPpI6Od0CZ zhIYu4z*R8wtT}cyO|#L?45fK>;FW>c{`dQ%14~qEhSJIJOH|FeXU4Y5IPNiyG*cVx z{+zMgcnY$B*?TSfmvq%&5pMFa?i-hu=<46m=Jid?9Q_)7n~!(TUwZS>EnDL9679^H zs84mK`?gBta@u0Y5fIJ4y9R$W8)Ttlla&pnqX^*!I}_r~vi zZ+z*+VA>T*oe8H5mscnO=XWI91D8DfL667D3dn*A$eowThlq`@Ib=<@_o9eT&~T`t zg8%~`*ueydsXZP{WWj(2c~KQZ)df^}P}PVk6i?&pVff}AzF{QCGdSKkfLbw2;fz1# zxH0q%iJZ)Ei2ev>v@|R^fEtjuB=ZP;cp~J3>&G3!JMayeFi`{b7iy9uzoZzFdT1v| z>wQ9hpD_HMU_R67U!8ho>eZKDc{wJ`Pu-fDzkcg_qU(d>?;Zc(%zI}RPNW-;EJp4O zrYldTb^TE)qi5zSXDjC%vyPZAek!?bp((9D8r5dB^s8g9jJ+CoB@k^5S@PfjNgjhoJz0aAo_* diff --git a/hack/owners/gen_owners.py b/hack/owners/gen_owners.py index 94e385aa..acc8ea7f 100755 --- a/hack/owners/gen_owners.py +++ b/hack/owners/gen_owners.py @@ -5,8 +5,9 @@ gen_owners.py render [--out DIR] write OWNERS_ALIASES and OWNERS per repository (default hack/owners/out) and refresh the table in hyperfleet/docs/owners.md - gen_owners.py check fail if the table in owners.md is stale or the - YAML is inconsistent + gen_owners.py check fail if the table in owners.md is stale, the + YAML is inconsistent, or a path's approvers or + reviewers differ from its area row in the table gen_owners.py table print the area table Rules applied at render time are documented at the top of owners.yaml. @@ -100,6 +101,7 @@ def validate(data): errors.append(f"{repo}/{path or '(root)'}: label {label} not in labels") if path == "" and not spec.get("approvers"): errors.append(f"{repo}: root OWNERS has no approvers") + areas = {a["name"]: a for a in data["areas"]} for area in data["areas"]: for key in ("approvers", "reviewers"): for n in area.get(key, []): @@ -107,6 +109,70 @@ def validate(data): errors.append(f"area {area['name']} {key}: unknown name {n}") if area["label"] not in data["labels"]: errors.append(f"area {area['name']}: label {area['label']} not in labels") + if area.get("inherits") and area["inherits"] not in areas: + errors.append(f"area {area['name']}: inherits unknown area {area['inherits']}") + if not errors: + errors.extend(validate_areas(data, areas)) + return errors + + +def area_approvers(areas, area): + """The area's approvers, following `inherits` to the parent row.""" + while area.get("inherits"): + area = areas[area["inherits"]] + return list(area.get("approvers", [])) + + +def logins(data, names): + """Expanded login set, with the fallback aliases left out.""" + return set(expand(data, [n for n in names if n not in FALLBACK_ALIASES])) + + +def describe(have, want): + parts = [] + if have - want: + parts.append("extra " + ", ".join(sorted(have - want))) + if want - have: + parts.append("missing " + ", ".join(sorted(want - have))) + return "; ".join(parts) + + +def validate_areas(data, areas): + """Every path with approvers names an area, and matches that area's row. + + Approvers are compared without the fallback aliases, which the root OWNERS of + most repositories carry and the table shows only where they are the area's + second approver. Reviewers are compared as rendered, so the area's approvers + count on both sides. A path without approvers inherits them from its parent + OWNERS and only its reviewers are checked. + """ + errors = [] + named = set() + for repo, spec_map in data["repos"].items(): + for path, spec in spec_map.items(): + where = f"{repo}/{path or '(root)'}" + name = spec.get("area") + if name is None: + if spec.get("approvers"): + errors.append(f"{where}: has approvers but names no area") + continue + if name not in areas: + errors.append(f"{where}: unknown area {name}") + continue + named.add(name) + area = areas[name] + want_approvers = logins(data, area_approvers(areas, area)) + want_reviewers = logins(data, area["reviewers"]) | want_approvers + if spec.get("approvers"): + have = logins(data, spec["approvers"]) + if have != want_approvers: + errors.append(f"{where}: approvers differ from area {name}: {describe(have, want_approvers)}") + have = logins(data, reviewers_for(spec)) + if have != want_reviewers: + errors.append(f"{where}: reviewers differ from area {name}: {describe(have, want_reviewers)}") + for name in areas: + if name not in named: + errors.append(f"area {name}: no path in repos names it") return errors diff --git a/hack/owners/labels.sh b/hack/owners/labels.sh index 668e5c17..582c2622 100755 --- a/hack/owners/labels.sh +++ b/hack/owners/labels.sh @@ -3,7 +3,8 @@ # repository listed there. Idempotent: existing labels are updated in place. # # Usage: hack/owners/labels.sh [--dry-run] [repo ...] -# Needs gh authenticated with write access to the repositories. +# Needs gh authenticated with write access to the repositories. Runs on the bash +# 3.2 that macOS ships, so no mapfile, associative arrays or ${var,,}. set -euo pipefail ORG="openshift-hyperfleet" @@ -24,7 +25,8 @@ color_for() { esac } -mapfile -t REPOS < <(python3 - "${SOURCE}" <<'PY' +REPOS=() +while IFS= read -r line; do REPOS+=("${line}"); done < <(python3 - "${SOURCE}" <<'PY' import sys import yaml with open(sys.argv[1]) as f: @@ -34,7 +36,8 @@ PY if [[ $# -gt 0 ]]; then REPOS=("$@") fi -mapfile -t LABELS < <(python3 - "${SOURCE}" <<'PY' +LABELS=() +while IFS= read -r line; do LABELS+=("${line}"); done < <(python3 - "${SOURCE}" <<'PY' import sys import yaml with open(sys.argv[1]) as f: @@ -42,6 +45,10 @@ with open(sys.argv[1]) as f: print(f"{name}\t{desc}") PY ) +if [[ ${#REPOS[@]} -eq 0 || ${#LABELS[@]} -eq 0 ]]; then + echo "no repositories or labels found in ${SOURCE}" >&2 + exit 1 +fi for repo in "${REPOS[@]}"; do echo "== ${ORG}/${repo}" diff --git a/hack/owners/open_prs.sh b/hack/owners/open_prs.sh index 112fffa3..b2f6761c 100755 --- a/hack/owners/open_prs.sh +++ b/hack/owners/open_prs.sh @@ -7,7 +7,8 @@ # with gh. Existing OWNERS files that the generator does not produce are removed, # so stale per-directory files go in the same change. With --draft the PR opens # as a draft and holds itself, so it cannot merge until it is marked ready and -# the hold is lifted. +# the hold is lifted. Runs on the bash 3.2 that macOS ships, so no mapfile, +# associative arrays or unguarded empty-array expansion under set -u. set -euo pipefail ORG="openshift-hyperfleet" @@ -39,7 +40,12 @@ else echo "rendered OWNERS directory not found: ${OUT} (run 'make owners' first)" >&2 exit 1 fi - mapfile -t REPOS < <(ls -A "${OUT}") + REPOS=() + while IFS= read -r line; do REPOS+=("${line}"); done < <(ls -A "${OUT}") +fi +if [[ ${#REPOS[@]} -eq 0 ]]; then + echo "nothing rendered under ${OUT} (run 'make owners' first)" >&2 + exit 1 fi read -r -d '' BODY < path -> OWNERS file. "" is the repository root. -# approvers and reviewers are alias names or logins. Reviewers get the path's -# approvers appended at render time (see the rule at the top). +# area names the row of the table above that the path belongs to; approvers and +# reviewers are alias names or logins and must match that row. Reviewers get the +# path's approvers appended at render time (see the rule at the top). A path with +# no approvers inherits them and only routes reviews, so it may name an area +# without carrying its approvers. repos: hyperfleet-api: - "": {approvers: [api-owners, tech-leads], reviewers: [api-reviewers], labels: [area/api]} - pkg/tenant: {approvers: [tenancy-owners], reviewers: [tenancy-reviewers], labels: [area/tenancy]} - pkg/auth: {approvers: [tenancy-owners], reviewers: [tenancy-reviewers], labels: [area/auth]} - pkg/middleware: {approvers: [tenancy-owners, generic-model-owners], reviewers: [tenancy-reviewers], labels: [area/auth]} - pkg/dao: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} - pkg/db: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model, area/search]} - pkg/registry: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} - pkg/services: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} - pkg/handlers: {approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} - pkg/validation: {approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} - pkg/validators: {approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} - pkg/api: {approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} - pkg/telemetry: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - pkg/metrics: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - charts/dashboards: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - openapi: {approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} - .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + "": {area: API, approvers: [api-owners, tech-leads], reviewers: [api-reviewers], labels: [area/api]} + pkg/tenant: {area: Tenancy and auth, approvers: [tenancy-owners], reviewers: [tenancy-reviewers], labels: [area/tenancy]} + pkg/auth: {area: Tenancy and auth, approvers: [tenancy-owners], reviewers: [tenancy-reviewers], labels: [area/auth]} + pkg/middleware: {area: Tenancy and auth, approvers: [tenancy-owners], reviewers: [tenancy-reviewers], labels: [area/auth]} + pkg/dao: {area: Generic model, approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} + pkg/db: {area: Generic model, approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model, area/search]} + pkg/registry: {area: Generic model, approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} + pkg/services: {area: Generic model, approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} + pkg/handlers: {area: Generic model, approvers: [generic-model-owners], reviewers: [generic-model-reviewers], labels: [area/generic-model]} + pkg/validation: {area: Search, approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} + pkg/validators: {area: Search, approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} + pkg/api: {area: Search, approvers: [generic-model-owners], reviewers: [search-reviewers], labels: [area/search]} + pkg/telemetry: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + pkg/metrics: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + charts/dashboards: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + openapi: {area: Contracts, approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} + .tekton: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} hyperfleet-api-spec: - "": {approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} + "": {area: Contracts, approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} hyperfleet-api-spec-template: - "": {approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} + "": {area: Contracts, approvers: [contracts-owners], reviewers: [contracts-reviewers], labels: [area/contracts]} hyperfleet-adapter: - "": {approvers: [adapter-owners, tech-leads], reviewers: [adapter-reviewers], labels: [area/adapter]} - internal/desireclient: {approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} - internal/transportclient: {approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} - internal/transportregistry: {approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} - charts/dashboards: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - pkg/telemetry: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + "": {area: Adapter framework, approvers: [adapter-owners, tech-leads], reviewers: [adapter-reviewers], labels: [area/adapter]} + internal/desireclient: {area: Desire delivery, approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} + internal/transportclient: {area: Desire delivery, approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} + internal/transportregistry: {area: Desire delivery, approvers: [desire-owners], reviewers: [desire-reviewers], labels: [area/desire-delivery]} + charts/dashboards: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + pkg/telemetry: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + .tekton: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} hyperfleet-applier: - "": {approvers: [desire-owners, tech-leads], reviewers: [desire-reviewers], labels: [area/desire-delivery]} - .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + "": {area: Desire delivery, approvers: [desire-owners, tech-leads], reviewers: [desire-reviewers], labels: [area/desire-delivery]} + .tekton: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} hyperfleet-sentinel: - "": {approvers: [sentinel-owners, tech-leads], reviewers: [sentinel-reviewers], labels: [area/sentinel]} - pkg/telemetry: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - charts/dashboards: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + "": {area: Sentinel, approvers: [sentinel-owners, tech-leads], reviewers: [sentinel-reviewers], labels: [area/sentinel]} + pkg/telemetry: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + charts/dashboards: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + .tekton: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} hyperfleet-broker: - "": {approvers: [broker-owners, tech-leads], reviewers: [broker-reviewers], labels: [area/broker]} + "": {area: Broker, approvers: [broker-owners, tech-leads], reviewers: [broker-reviewers], labels: [area/broker]} hyperfleet-logger: - "": {approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} + "": {area: Platform libraries, approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} hyperfleet-credential-provider: - "": {approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} + "": {area: Platform libraries, approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} renovate-config: - "": {approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} + "": {area: Platform libraries, approvers: [platform-owners, tech-leads], reviewers: [platform-reviewers], labels: [area/platform]} hyperfleet-e2e: - "": {approvers: [e2e-owners, tech-leads], reviewers: [e2e-reviewers], labels: [area/e2e]} - e2e/auth: {approvers: [e2e-owners, tenancy-owners], reviewers: [gateway-reviewers], labels: [area/auth]} - perf: {approvers: [perf-owners], reviewers: [perf-reviewers], labels: [area/performance]} + "": {area: E2E, approvers: [e2e-owners, tech-leads], reviewers: [e2e-reviewers], labels: [area/e2e]} + e2e/auth: {area: Tenancy and auth, approvers: [tenancy-owners], reviewers: [tenancy-reviewers], labels: [area/auth]} + perf: {area: Performance, approvers: [perf-owners], reviewers: [perf-reviewers], labels: [area/performance]} hyperfleet-operator: - "": {approvers: [operator-owners, tech-leads], reviewers: [operator-reviewers], labels: [area/operator]} - catalog: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} - config/manifests: {approvers: [release-owners, operator-owners], reviewers: [release-reviewers], labels: [area/release]} - validators: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} - .tekton: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} - internal/metrics: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - internal/servicemonitor: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} - config/prometheus: {approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + "": {area: Operator, approvers: [operator-owners, tech-leads], reviewers: [operator-reviewers], labels: [area/operator]} + catalog: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + config/manifests: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + validators: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + .tekton: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + internal/metrics: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + internal/servicemonitor: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} + config/prometheus: {area: Observability, approvers: [observability-owners], reviewers: [observability-reviewers], labels: [area/observability]} hyperfleet-infra: - "": {approvers: [infra-owners, tech-leads], reviewers: [infra-reviewers], labels: [area/infra]} - helm/hyperfleet-gateway: {approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} - helm/network-policies: {approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} - helm/mock-oidc: {approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} - helmfile: {approvers: [e2e-owners], reviewers: [mliptak0, tirthct], labels: [area/e2e]} - terraform/oci: {approvers: [oci-owners], reviewers: [oci-reviewers], labels: [area/oci]} - functions/oci-ci-sweep: {approvers: [oci-owners], reviewers: [oci-reviewers], labels: [area/oci]} - functions/lifecycle-enforcer: {approvers: [rafabene, ma-hill], reviewers: [kuudori], labels: [area/infra]} - terraform/envs: {approvers: [rafabene, ma-hill], reviewers: [kuudori, mliptak0], labels: [area/infra]} + "": {area: Infra, approvers: [infra-owners, tech-leads], reviewers: [infra-reviewers], labels: [area/infra]} + helm/hyperfleet-gateway: {area: Gateway, approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} + helm/network-policies: {area: Gateway, approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} + helm/mock-oidc: {area: Gateway, approvers: [gateway-owners], reviewers: [gateway-reviewers], labels: [area/gateway]} + helmfile: {area: E2E, approvers: [e2e-owners], reviewers: [e2e-reviewers], labels: [area/e2e]} + terraform/oci: {area: OCI, approvers: [oci-owners], reviewers: [oci-reviewers], labels: [area/oci]} + functions/oci-ci-sweep: {area: OCI, approvers: [oci-owners], reviewers: [oci-reviewers], labels: [area/oci]} hyperfleet-release: - "": {approvers: [release-owners, tech-leads], reviewers: [release-reviewers], labels: [area/release]} + "": {area: Release and packaging, approvers: [release-owners, tech-leads], reviewers: [release-reviewers], labels: [area/release]} architecture: - "": {approvers: [architecture-approvers], reviewers: [kuudori, pnguyen44, tirthct, ldornele, ma-hill, mliptak0, Ruclo, Mischulee], labels: [area/architecture]} + "": {area: Architecture, approvers: [architecture-approvers], reviewers: [kuudori, pnguyen44, tirthct, ldornele, ma-hill, mliptak0, Ruclo, Mischulee], labels: [area/architecture]} hyperfleet/adrs: {reviewers: [pnguyen44, kuudori, tirthct, ldornele], labels: [kind/adr]} hyperfleet/standards: {reviewers: [rafabene, kuudori, ldornele, pnguyen44], labels: [kind/standard]} - hyperfleet/components/api-service: {reviewers: [api-owners, api-reviewers], labels: [area/api]} - hyperfleet/components/adapter: {reviewers: [adapter-owners, adapter-reviewers], labels: [area/adapter]} - hyperfleet/components/sentinel: {reviewers: [sentinel-owners, sentinel-reviewers], labels: [area/sentinel]} - hyperfleet/components/broker: {reviewers: [broker-owners, broker-reviewers], labels: [area/broker]} - hyperfleet/components/claude-code-plugin: {reviewers: [tooling-owners, tooling-reviewers], labels: [area/tooling]} - hyperfleet/docs/e2e-testing: {reviewers: [e2e-owners, e2e-reviewers], labels: [area/e2e]} - hyperfleet/docs/release: {reviewers: [release-owners, release-reviewers], labels: [area/release]} + hyperfleet/components/api-service: {area: API, reviewers: [api-owners, api-reviewers], labels: [area/api]} + hyperfleet/components/adapter: {area: Adapter framework, reviewers: [adapter-owners, adapter-reviewers], labels: [area/adapter]} + hyperfleet/components/sentinel: {area: Sentinel, reviewers: [sentinel-owners, sentinel-reviewers], labels: [area/sentinel]} + hyperfleet/components/broker: {area: Broker, reviewers: [broker-owners, broker-reviewers], labels: [area/broker]} + hyperfleet/components/claude-code-plugin: {area: Team tooling, reviewers: [tooling-owners, tooling-reviewers], labels: [area/tooling]} + hyperfleet/docs/e2e-testing: {area: E2E, reviewers: [e2e-owners, e2e-reviewers], labels: [area/e2e]} + hyperfleet/docs/release: {area: Release and packaging, reviewers: [release-owners, release-reviewers], labels: [area/release]} hyperfleet-claude-plugins: - "": {approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers, ma-hill], labels: [area/tooling]} - hyperfleet-adapter-authoring: {approvers: [adapter-owners], reviewers: [adapter-reviewers], labels: [area/adapter]} - hyperfleet-architecture: {approvers: [ciaranRoche, rafabene], reviewers: [pnguyen44], labels: [area/architecture]} - hyperfleet-code-review: {approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} - hyperfleet-devtools: {approvers: [rafabene, kuudori], reviewers: [tirthct], labels: [area/tooling]} - hyperfleet-hooks: {approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} - hyperfleet-jira: {approvers: [rafabene, ciaranRoche], reviewers: [pnguyen44], labels: [area/tooling]} - hyperfleet-operational-readiness: {approvers: [tirthct, ciaranRoche], reviewers: [pnguyen44, ma-hill], labels: [area/tooling]} - hyperfleet-release: {approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} - hyperfleet-standards: {approvers: [rafabene, kuudori], reviewers: [ldornele], labels: [kind/standard]} - hyperfleet-work-triage: {approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + "": {area: Team tooling, approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-adapter-authoring: {area: Adapter framework, approvers: [adapter-owners], reviewers: [adapter-reviewers], labels: [area/adapter]} + hyperfleet-architecture: {area: Team tooling, approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-code-review: {area: Team tooling, approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-devtools: {area: Team tooling, approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-hooks: {area: Team tooling, approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-jira: {area: Team tooling, approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-operational-readiness: {area: Team tooling, approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-release: {area: Release and packaging, approvers: [release-owners], reviewers: [release-reviewers], labels: [area/release]} + hyperfleet-standards: {area: Team tooling, approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} + hyperfleet-work-triage: {area: Team tooling, approvers: [tooling-owners], reviewers: [tooling-reviewers], labels: [area/tooling]} hyperfleet-hooks: - "": {approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers], labels: [area/tooling]} + "": {area: Team tooling, approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers], labels: [area/tooling]} coderabbit: - "": {approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers], labels: [area/tooling]} + "": {area: Team tooling, approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers], labels: [area/tooling]} .github: - "": {approvers: [tech-leads], reviewers: [tooling-owners, tooling-reviewers], labels: [area/tooling]} + "": {area: Team tooling, approvers: [tooling-owners, tech-leads], reviewers: [tooling-reviewers], labels: [area/tooling]} From d80ec8be18587c73f125f952e07374e5ee25d812 Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Tue, 13 Oct 2026 10:46:38 +0100 Subject: [PATCH 11/13] HYPERFLEET-1762 - docs: replace the README MVP access section with the ownership rules The README still said every developer has approve and merge access and that review requirements would come after the MVP, twenty lines above the section that describes the area approver rule. The access section now states the rule and points at the ownership document. Last Updated is refreshed on the README and on the commit standard, whose squash wording changed in this PR. Co-Authored-By: Claude Fable 5.1 --- README.md | 30 ++++++++----------------- hyperfleet/standards/commit-standard.md | 2 +- 2 files changed, 10 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 085a0159..4a6170a6 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ --- Status: Active Owner: Architecture Team -Last Updated: 2025-11-07 +Last Updated: 2026-10-13 --- # HyperFleet Architecture Repository @@ -9,7 +9,7 @@ Last Updated: 2025-11-07 ## Table of Contents - [Overview](#overview) -- [Repository Access & MVP Process](#repository-access--mvp-process) +- [Repository Access and Review](#repository-access-and-review) - [Repository Structure](#repository-structure) - [Document Types](#document-types) - [1. Architecture Overview (`hyperfleet/README.md`)](#1-architecture-overview-hyperfleetreadmemd) @@ -53,33 +53,21 @@ This repository serves as the **single source of truth** for all architectural d --- -## Repository Access & MVP Process - -**Current Phase**: MVP Development +## Repository Access and Review **Access & Permissions**: -- All developers in the HyperFleet team are granted **write access** to this repository -- To support quick iteration during MVP, developers have **approve and merge access** if there are no objections from peers -- This temporary process enables rapid documentation of decisions and scope changes +- All developers in the HyperFleet team have **write access** to this repository +- A merge needs an approval from an **area approver**, as declared in the `OWNERS` files and enforced by Prow. The process is in [Review and Merge Process](#review-and-merge-process) below +- Reviewer and approver status is earned against the criteria in [Repository Ownership](hyperfleet/docs/owners.md), and nothing else **Repository Usage**: - This repository primarily records: - Architectural decisions and their rationale - - Trade-offs we're accepting during MVP - - Changes to MVP scope and priorities - - Technical debt we're consciously incurring - -**Post-MVP**: - -- Once MVP is complete, we will establish official processes for: - - Repository access management - - PR review requirements - - Approval workflows - - Documentation governance - -**During MVP, prioritize speed over process while maintaining clear documentation of decisions.** + - Trade-offs we are accepting + - Changes to scope and priorities + - Technical debt we are consciously incurring --- diff --git a/hyperfleet/standards/commit-standard.md b/hyperfleet/standards/commit-standard.md index dc902046..3dac0996 100644 --- a/hyperfleet/standards/commit-standard.md +++ b/hyperfleet/standards/commit-standard.md @@ -1,7 +1,7 @@ --- Status: Active Owner: HyperFleet Platform Team -Last Updated: 2026-04-24 +Last Updated: 2026-10-13 --- # HyperFleet Commit Message Standard From 9fd08bf7ccd18e37e2f491d449f9ca53a5974bc3 Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Tue, 13 Oct 2026 14:05:27 +0100 Subject: [PATCH 12/13] HYPERFLEET-1762 - docs: copy the ownership criteria into the working agreement from one source The reviewer and approver criteria are written once in owners.md between markers. The generator copies the block into the working agreement, with headings one level deeper, and the check fails when the copy differs from the source, the same way the area table is handled. The agreement keeps the criteria as its own rules without a second hand-maintained copy. Co-Authored-By: Claude Fable 5.1 --- hack/owners/gen_owners.py | 55 +++++++++++++++++++++++----- hyperfleet/docs/owners.md | 5 ++- hyperfleet/docs/working-agreement.md | 45 ++++++++++++++--------- 3 files changed, 78 insertions(+), 27 deletions(-) diff --git a/hack/owners/gen_owners.py b/hack/owners/gen_owners.py index acc8ea7f..b93e268f 100755 --- a/hack/owners/gen_owners.py +++ b/hack/owners/gen_owners.py @@ -3,10 +3,12 @@ Usage: gen_owners.py render [--out DIR] write OWNERS_ALIASES and OWNERS per repository - (default hack/owners/out) and refresh the table - in hyperfleet/docs/owners.md - gen_owners.py check fail if the table in owners.md is stale, the - YAML is inconsistent, or a path's approvers or + (default hack/owners/out), refresh the table in + hyperfleet/docs/owners.md and copy its criteria + block into hyperfleet/docs/working-agreement.md + gen_owners.py check fail if the table in owners.md or the criteria + copy in the working agreement is stale, the YAML + is inconsistent, or a path's approvers or reviewers differ from its area row in the table gen_owners.py table print the area table @@ -22,9 +24,15 @@ ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) SOURCE = os.path.join(ROOT, "hyperfleet", "docs", "owners.yaml") DOC = os.path.join(ROOT, "hyperfleet", "docs", "owners.md") +AGREEMENT = os.path.join(ROOT, "hyperfleet", "docs", "working-agreement.md") DEFAULT_OUT = os.path.join(ROOT, "hack", "owners", "out") TABLE_START = "" TABLE_END = "" +# The criteria are authored once in owners.md between these markers and copied +# into the working agreement between the same markers, with headings one level +# deeper because the agreement's section sits under a level-three heading. +CRITERIA_START = "" +CRITERIA_END = "" # Fallback approver sets that are never auto-requested as reviewers. FALLBACK_ALIASES = {"tech-leads", "architecture-approvers"} @@ -192,13 +200,32 @@ def table(data): return "\n".join(rows) +def splice(text, start_marker, end_marker, block, path): + """Replace what sits between the two markers in text with block.""" + start = text.find(start_marker) + end = text.find(end_marker) + if start < 0 or end < 0 or end < start: + raise SystemExit(f"{os.path.relpath(path, ROOT)}: markers {start_marker} and {end_marker} missing or out of order") + start += len(start_marker) + return text[:start] + "\n" + block + "\n" + text[end:] + + def splice_table(text, new_table): - start = text.find(TABLE_START) - end = text.find(TABLE_END) + return splice(text, TABLE_START, TABLE_END, new_table, DOC) + + +def criteria_block(doc_text): + """The criteria as written in owners.md, headings demoted one level for the agreement.""" + start = doc_text.find(CRITERIA_START) + end = doc_text.find(CRITERIA_END) if start < 0 or end < 0 or end < start: - raise SystemExit(f"{DOC}: table markers missing") - start += len(TABLE_START) - return text[:start] + "\n" + new_table + "\n" + text[end:] + raise SystemExit(f"{os.path.relpath(DOC, ROOT)}: criteria markers missing or out of order") + block = doc_text[start + len(CRITERIA_START):end].strip("\n") + return "\n".join("#" + line if line.startswith("#") else line for line in block.splitlines()) + + +def splice_criteria(agreement_text, doc_text): + return splice(agreement_text, CRITERIA_START, CRITERIA_END, criteria_block(doc_text), AGREEMENT) def clear_out_dir(out_dir): @@ -249,6 +276,12 @@ def render(data, out_dir): if new_text != text: with open(DOC, "w") as f: f.write(new_text) + with open(AGREEMENT) as f: + agreement = f.read() + new_agreement = splice_criteria(agreement, new_text) + if new_agreement != agreement: + with open(AGREEMENT, "w") as f: + f.write(new_agreement) return count @@ -258,6 +291,10 @@ def check(data): text = f.read() if splice_table(text, table(data)) != text: errors.append(f"{os.path.relpath(DOC, ROOT)}: area table is stale, run gen_owners.py render") + with open(AGREEMENT) as f: + agreement = f.read() + if splice_criteria(agreement, text) != agreement: + errors.append(f"{os.path.relpath(AGREEMENT, ROOT)}: criteria copy differs from owners.md, run gen_owners.py render") for e in errors: print(f"error: {e}", file=sys.stderr) return not errors diff --git a/hyperfleet/docs/owners.md b/hyperfleet/docs/owners.md index 4a48f8dc..37844f17 100644 --- a/hyperfleet/docs/owners.md +++ b/hyperfleet/docs/owners.md @@ -76,6 +76,7 @@ The tech leads sit in every root approver set as the fallback, not the routine m ## The criteria + **Reviewer and approver status in an area follows the criteria below and nothing else. Title, level, tenure and team do not count, for anyone, including the tech leads.** The bar is deliberately concrete. It is sized for a team of ten, and the shape is the Kubernetes community ladder: contribute, review, get sponsored, and keep doing it. @@ -108,6 +109,8 @@ You are ready when all of these are true: The same criteria were applied to everyone at launch, using the last three sprints of authored PRs and reviews. Nobody is an approver on title or on history alone. The same criteria, applied the same way, are used at every quarterly sweep. + + ### How to apply Open a PR to `owners.yaml` that adds you to the alias, and cite the PRs and reviews that meet each criterion with links. The sponsor (for reviewer) or nominator (for approver) approves the PR. The evidence is the PR list, nothing else is asked for. Then regenerate the `OWNERS` files (see [Changing the table](#changing-the-table)). @@ -157,7 +160,7 @@ Approving means: I read the whole diff, I checked the behavioural claims against ### Changing the table 1. Open a PR to [`owners.yaml`](owners.yaml) in this repository. For a role change, follow [How to apply](#how-to-apply). -2. Run `make owners` to regenerate the table in this document and the `OWNERS` files under `hack/owners/out/`. `make verify-owners` fails when the table is stale or when any path's approvers or reviewers differ from its area's row, so the table and the `OWNERS` files cannot drift apart. The scripts under `hack/owners/` run on the bash 3.2 that macOS ships. +2. Run `make owners` to regenerate the table in this document, the copy of the criteria in the [working agreement](working-agreement.md#repository-roles-reviewer-and-approver), and the `OWNERS` files under `hack/owners/out/`. `make verify-owners` fails when the table or that copy is stale, or when any path's approvers or reviewers differ from its area's row, so the three cannot drift apart. The criteria are edited here only. The scripts under `hack/owners/` run on the bash 3.2 that macOS ships. 3. Once merged, run `hack/owners/open_prs.sh` to open the `OWNERS` PRs in the affected repositories. ## References diff --git a/hyperfleet/docs/working-agreement.md b/hyperfleet/docs/working-agreement.md index 39490060..316bd59e 100644 --- a/hyperfleet/docs/working-agreement.md +++ b/hyperfleet/docs/working-agreement.md @@ -56,32 +56,43 @@ This section defines the two roles the team asks about most: the **Feature Owner ### Repository roles: reviewer and approver -Code review and merge rights are a third axis, declared per directory in `OWNERS` files and enforced by Prow. The full model, the current area table and how to apply are in [Repository Ownership](owners.md). The criteria are repeated here because they are rules of this agreement, not guidance. +Code review and merge rights are a third axis, declared per directory in `OWNERS` files and enforced by Prow. The two roles, the current area table and how to apply are in [Repository Ownership](owners.md). The criteria below are rules of this agreement, not guidance. They are written once in that document and copied here by the generator, so the two cannot differ. + **Reviewer and approver status in an area follows the criteria below and nothing else. Title, level, tenure and team do not count, for anyone, including the tech leads.** -| Role | What it means | -| ---- | ------------- | -| **Reviewer** | You know the area well enough to give a substantive first review. Prow requests you on PRs in the area. Your approving review sets `lgtm`, and an approver still has to sign off. | -| **Approver** | You can be the last check on the area. Your approval merges the PR on its own. You are accountable for what merges into the area. | +The bar is deliberately concrete. It is sized for a team of ten, and the shape is the Kubernetes community ladder: contribute, review, get sponsored, and keep doing it. -**Reviewer in an area**, all of: +#### Reviewer in an area -1. Five PRs you authored have merged in the area. -2. Five reviews in the area with comments you wrote yourself, having read the diff. -3. A current approver of the area sponsors you. +You are ready when all of these are true: -**Approver in an area**, all of: +1. You have authored **five PRs** that merged in the area. +2. You have given **five reviews** in the area with comments you wrote yourself. Running the review plugin is fine and encouraged, but the findings you post are yours, and you have read the diff. +3. One current approver of the area **sponsors** you. -1. Reviewer in the area for at least two sprints. -2. Primary reviewer on ten PRs in the area, at least three substantial (a design change, or over 300 lines), and your review changed at least some of them. -3. One story shipped end to end in the area with a reviewer other than the area's most active approver, or a design-level change for it (ADR, design doc, or an epic's story decomposition). -4. You know the tests that cover the area. -5. Nominated by an existing approver in a PR to `owners.yaml`, with no objection from another approver within three working days. +#### Approver in an area -**Staying an approver:** no reviews in the area for two sprints moves you back to reviewer at the next sweep. One sprint of reviewing and a request restores you within six months without re-qualifying. On loan or leave, step back for the duration. Leaving the team removes you from every alias. +You are ready when all of these are true: -The same criteria were applied to everyone at launch, using the last three sprints, and are applied the same way at every quarterly sweep. The tech leads are the fallback approver in every repository root, not the routine merger. +1. You have been a reviewer in the area for **at least two sprints**. +2. You have been the **primary reviewer on ten PRs** in the area. At least three were substantial (a design change, or over 300 lines), and your review changed at least some of them. +3. You have **shipped one story end to end** in the area with a reviewer other than the area's most active approver, or you have written a design-level change for it (an ADR, a design doc, or the story decomposition of an epic). +4. You know the **tests that cover the area**: which suites exercise it, what they do and do not check, and where a change would need a new one. +5. An existing approver **nominates** you in a PR to `owners.yaml`, and no other approver objects within three working days. + +#### Staying an approver + +- An approver with **no reviews in the area for two sprints** moves back to the reviewer alias at the next sweep. Nothing is lost: review in the area for a sprint and ask, and you are re-added without re-qualifying if it is within six months. +- On loan or extended leave, tell the other approvers in the area and either `/uncc` yourself or move to the reviewer alias for the duration. +- Leaving the team removes you from every alias in the same PR that removes your access. + +#### How the table was set + +The same criteria were applied to everyone at launch, using the last three sprints of authored PRs and reviews. Nobody is an approver on title or on history alone. The same criteria, applied the same way, are used at every quarterly sweep. + + +The tech leads are the fallback approver in every repository root, not the routine merger. ### Feature Owner (Level 4) From de55c3b6167527f6c7373cca0dd3cf17da249b2d Mon Sep 17 00:00:00 2001 From: Ciaran Roche Date: Tue, 13 Oct 2026 15:02:44 +0100 Subject: [PATCH 13/13] HYPERFLEET-1762 - docs: say areas have two or more approvers The summary said two or three, but Contracts, Operator and Architecture list four in the generated table. Co-Authored-By: Claude Fable 5.1 --- hyperfleet/docs/owners.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hyperfleet/docs/owners.md b/hyperfleet/docs/owners.md index 37844f17..628284ba 100644 --- a/hyperfleet/docs/owners.md +++ b/hyperfleet/docs/owners.md @@ -10,7 +10,7 @@ Last Updated: 2026-10-12 ## Summary -Every area of code has named owners: two or three **approvers** and a wider pool of **reviewers**, declared in `OWNERS` files that Prow reads. Review requests go to the people who know the area. A merge needs one approval, from an approver for every path the change touches. Reviewer and approver status is earned against the criteria in this document and nothing else. +Every area of code has named owners: two or more **approvers** and a wider pool of **reviewers**, declared in `OWNERS` files that Prow reads. Review requests go to the people who know the area. A merge needs one approval, from an approver for every path the change touches. Reviewer and approver status is earned against the criteria in this document and nothing else. ## The two roles