Skip to content

chore(codeowners): add @thinkall as Windows co-owner - #5498

Open
thinkall wants to merge 2 commits into
sonichi:mainfrom
thinkall:chore/windows-codeowners
Open

thinkall wants to merge 2 commits into
sonichi:mainfrom
thinkall:chore/windows-codeowners

Conversation

@thinkall

@thinkall thinkall commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Priority

Level: low
Reason: Contributor-requested Windows ownership; no runtime change or release blocker.

Closes

Summary

Add @thinkall as a Windows co-owner in .github/CODEOWNERS, following the Windows implementation in #1789. Cover PowerShell entry points and installation, the TypeScript/Python platform abstractions, Windows-specific tests, and the Windows runtime workflow; preserve every existing maintainer and test owner.

Maintainer action required: @thinkall currently has read-only access to sonichi/sutando. Please grant repository write access separately if accepting this ownership proposal: GitHub requires it for code-owner eligibility and for merging approved PRs. This PR does not grant permissions, change branch protection, enable bypass, or permit self-approval; the current main ruleset still requires one approving review including a code owner, plus required checks. Keeping the existing owners allows them to approve @thinkall-authored PRs.

Review .github/CODEOWNERS' new final Windows section first. Shared files outside the two platform abstraction modules retain their existing ownership; this is not ownership of all code with a Windows branch.

Documentation consistency: N/A for runtime documentation: no user-visible feature or behavior changes. .github/CODEOWNERS is the ownership declaration being updated.

Scope and risk: Configuration-only, unstacked PR. No migrations, deployment, or live-service restart required. New owner routing is evaluated from the base branch after merge and depends on write access being granted. Rollback is removal of the added Windows section.

Review requested from maintainers: @sonichi and @qingyun-wu, please review this Windows co-owner proposal and the separate write-access prerequisite above.

Reviewer notification status: Explicit review requests returned thinkall does not have the correct permissions to execute RequestReviews, but automatic CODEOWNERS routing nevertheless created active review requests for @sonichi and @qingyun-wu (verified with gh pr view 5498 --json reviewRequests). The local collaboration-intelligence reviewer roster is absent (no roster at any host), so stand-in/channel notifications cannot be resolved without that roster.

Checklist

  • Confirmed no other open PR closes the same issue (gh pr list --repo sonichi/sutando --search "closes #N")
  • Git author email matches my CLA-signed email (git log -1 --format='%ae' shows a GH-mapped email, not *.local or noreply@anthropic.com)
  • Single concern per PR — no bundled refactors / drive-by feature additions
  • Confirmed bug exists on upstream/main (or feature isn't already covered)
  • Test added (or N/A explained below)
  • Before/after evidence pasted below — the actual command output for both states, not a description of it (this is the Add demo video to README #1 reason PRs bounce)
  • Every claim in this PR body is verifiable from the diff or the pasted output — no statement the reviewer can't check
  • Live path? If this touches a bridge, network path, delivery loop, or startup, I included a real post-restart round-trip — unit tests / harnesses alone are not accepted for these
  • Stacked PR? I named the parent + merge order and will rebase/update + rerun full checks after the parent lands (or N/A)
  • Scanned added lines for host-specific hardcoded paths and inline home/workspace fallbacks; fixtures are narrowly scoped
  • If this PR adds or edits a CI workflow, I confirmed it actually runs on this PR (see the Checks tab — a branches: filter can silently exclude a stacked PR)
  • Doesn't reinvent workspace / home-path resolution — uses the resolve_workspace / resolveWorkspace / claude_home_path helpers (see CLAUDE.md "Workspace contract"; enforced by the lint-workspace-resolution, lint-claude-home-path, and workspace-leak-check CI checks)

No linked issue; searched open PRs and all-state Windows/CODEOWNERS PRs/issues, and found no duplicate Windows ownership proposal. No permanent runtime test added: N/A for ownership configuration; the reproducible ad-hoc validator below checks all tracked paths. Live-path, stacked-PR, workflow-edit, and workspace-resolution checklist items are N/A: only .github/CODEOWNERS changes.

The commit author email maps to @thinkall in the upstream commits API. The prior Windows PR head has license/cla: success with Contributor License Agreement is signed.; this PR's own SHA-bound CLA check must still complete.

Before / after evidence

Parent: 97cb7566 (upstream/main). Head: f46dfd3e.

Before, using the validator included below:

python "$env:SESSION_ARTIFACTS\validate-windows-codeowners.py" --ref upstream/main
ref: upstream/main
Windows/platform files in scope: 36
Files declaring @thinkall: 0
Windows/platform files without @thinkall: 36
Files losing an existing owner: 0
Unrelated files gaining @thinkall: 0
src/startup.ps1: @sonichi @qingyun-wu
src/platform.ts: @sonichi @qingyun-wu
tests/windows-startup.test.ps1: @sonichi @qingyun-wu @liususan091219
.github/workflows/windows-runtime.yml: @sonichi @qingyun-wu
src/voice-agent.ts: @sonichi @liususan091219 @yixuan-ag2

After:

python "$env:SESSION_ARTIFACTS\validate-windows-codeowners.py" --ref HEAD
ref: HEAD
Windows/platform files in scope: 36
Files declaring @thinkall: 36
Windows/platform files without @thinkall: 0
Files losing an existing owner: 0
Unrelated files gaining @thinkall: 0
src/startup.ps1: @sonichi @qingyun-wu @thinkall
src/platform.ts: @sonichi @qingyun-wu @thinkall
tests/windows-startup.test.ps1: @sonichi @qingyun-wu @liususan091219 @thinkall
.github/workflows/windows-runtime.yml: @sonichi @qingyun-wu @thinkall
src/voice-agent.ts: @sonichi @liususan091219 @yixuan-ag2
PASS: all scoped files covered; existing and unrelated ownership preserved

These are declarations, not proof of GitHub eligibility before write access is granted. Permission snapshot from gh api repos/sonichi/sutando --jq '{permissions}':

{"permissions":{"admin":false,"maintain":false,"pull":true,"push":false,"triage":false}}

The pull-request rule returned by gh api repos/sonichi/sutando/rules/branches/main has required_approving_review_count: 1 and require_code_owner_review: true.

Test plan

  • Both validator commands above: passed (exit 0). Baseline reports absent Windows ownership; HEAD additionally asserts full Windows coverage. The validator checks last-match-wins for the simple patterns used here, preserves all existing owners across tracked files, rejects unrelated ownership changes, and checks that every added pattern matches an existing file.

  • git --no-pager diff --check upstream/main...HEAD: passed (exit 0; no output).

  • Repository review scanner: exit 0, PARTIAL, not a full prose-cap pass. On Windows, the exact command uses Git Bash rather than the unrelated WSL launcher:

    $gitRoot = Split-Path (Split-Path (Get-Command git).Source)
    $gitBash = Join-Path $gitRoot 'bin\bash.exe'
    git --no-pager diff upstream/main...HEAD | & $gitBash scripts\review-checks.sh
    prose-cap: no in-scope files (exts .py); nothing was scanned
    review-checks: PARTIAL (hardcoded-paths + root-artifacts clean; prose-cap had no in-scope files)
    
  • Runtime tests/typecheck and live round trips: not run, N/A for this CODEOWNERS-only change.

  • gh api repos/sonichi/sutando/commits/f46dfd3e/status: license/cla success for this PR head; Contributor License Agreement is signed.

  • GitHub owner assignment cannot be verified as active until this merges into the base branch and a maintainer grants @thinkall write access.

Reproducible ad-hoc ownership validator (standard library only)

Save this as validate-windows-codeowners.py outside the checkout and set $env:SESSION_ARTIFACTS to its containing directory. Run the commands above from this PR checkout with upstream/main pointing at the parent commit 97cb7566.

import argparse
import re
import subprocess
from pathlib import Path, PurePosixPath


def git(*args):
    return subprocess.check_output(["git", *args], text=True).strip()


def rules(text):
    result = []
    for number, line in enumerate(text.splitlines(), 1):
        fields = line.split("#", 1)[0].split()
        if not fields:
            continue
        pattern, *owners = fields
        assert owners and all(owner.startswith("@") for owner in owners), number
        assert pattern == "*" or pattern.startswith("/"), pattern
        expression = re.escape(pattern.lstrip("/")).replace(r"\*", "[^/]*")
        if pattern == "*":
            expression = ".*"
        elif pattern.endswith("/"):
            expression += ".*"
        result.append((pattern, re.compile("^" + expression + "$"), owners))
    return result


def owners_for(path, entries):
    owners = []
    for _, expression, candidates in entries:
        if expression.fullmatch(path):
            owners = candidates
    return owners


parser = argparse.ArgumentParser()
parser.add_argument("--ref", required=True)
args = parser.parse_args()
baseline = rules(git("show", "upstream/main:.github/CODEOWNERS"))
text = (
    Path(".github", "CODEOWNERS").read_text()
    if args.ref == "WORKTREE"
    else git("show", args.ref + ":.github/CODEOWNERS")
)
entries = rules(text)
paths = git("ls-tree", "-r", "--name-only", "upstream/main").splitlines()
explicit = {
    "src/platform.ts",
    "src/sutando_platform.py",
    ".github/workflows/windows-runtime.yml",
}
windows = {
    path for path in paths
    if path in explicit
    or path.endswith(".ps1")
    or (path.startswith("tests/") and (
        PurePosixPath(path).name.startswith("windows-")
        or "-windows" in PurePosixPath(path).name
        or PurePosixPath(path).name.startswith("notify-ps1-")
    ))
}
covered = {path for path in paths if "@thinkall" in owners_for(path, entries)}
removed = [
    path for path in paths
    if not set(owners_for(path, baseline)).issubset(owners_for(path, entries))
]
outside = covered - windows
assert not removed, removed
assert not outside, outside
for pattern, expression, owners in entries:
    if "@thinkall" in owners:
        assert any(expression.fullmatch(path) for path in paths), pattern
print("ref:", args.ref)
print("Windows/platform files in scope:", len(windows))
print("Files declaring @thinkall:", len(covered))
print("Windows/platform files without @thinkall:", len(windows - covered))
print("Files losing an existing owner:", len(removed))
print("Unrelated files gaining @thinkall:", len(outside))
for path in (
    "src/startup.ps1",
    "src/platform.ts",
    "tests/windows-startup.test.ps1",
    ".github/workflows/windows-runtime.yml",
    "src/voice-agent.ts",
):
    print(path + ": " + " ".join(owners_for(path, entries)))
if args.ref != "upstream/main":
    assert covered == windows, sorted(windows - covered)
    print("PASS: all scoped files covered; existing and unrelated ownership preserved")

Cover Windows PowerShell entry points, platform abstractions, Windows-specific tests, and the Windows runtime workflow while preserving all existing owners.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d09bd138-3f8d-4999-a525-6118747e1291
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment at f17c517612e384b65a7a4e89fb02467c77760232. The mechanics are correct. Whether to grant ownership is the maintainers' decision (this account is one of the requested owners), so this is a comment, not an approval.

Verified

  • I wrote an independent last-match-wins check over all 2762 tracked files (base origin/main:.github/CODEOWNERS vs head). Result: lost-owner 0, @thinkall on 36 files, all Windows/PowerShell/platform files, every added pattern matches at least one file, and no file's non-@thinkall owner set changed. This agrees with the PR body's validator.
  • Hardcoded-path scan: clean. CI is green.

Merge-order point (would block merging as-is)

  • gh api repos/sonichi/sutando/codeowners/errors?ref=f17c5176 returns 10 errors, all Unknown owner ("make sure @thinkall exists and has write access"), one per added line. The same call on main returns 0.
  • If this merges before write access is granted, main's CODEOWNERS starts carrying 10 errors. @thinkall would also not count as an owner until access is granted.
  • Suggested order: the maintainers decide on write access first; if granted, re-run that API call (expect 0), then merge. If access is not granted, this PR should not merge.

No code nits.

— sutando-qingyun-air

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants