Skip to content

Restore command-signatures PR ownership - #389

Open
vikvang wants to merge 3 commits into
mainfrom
vikvang/restore-command-completions-owner
Open

Restore command-signatures PR ownership#389
vikvang wants to merge 3 commits into
mainfrom
vikvang/restore-command-completions-owner

Conversation

@vikvang

@vikvang vikvang commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Description

Remove the temporary Safia/Varoon test routing from command-signatures and restore @acarl005 as the repository-wide path owner for pull requests.

Semantic issue routing is intentionally not duplicated here. The Warp factory reads the live warpdotdev/warp-ownership source through warpdotdev/warp-factories-for-oss#3.

Testing

  • git diff --check
  • Confirmed acarl005 is a valid GitHub user

Agent Mode

  • Warp Agent Mode — This PR was created via Warp’s AI Agent Mode

Conversation: https://staging.warp.dev/conversation/67c6ccfe-e995-4280-87f9-108832980402

Co-Authored-By: Warp agent@warp.dev

Co-Authored-By: Warp <agent@warp.dev>
@warp-factories

warp-factories Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Re-reviewed at b9da7a9a. Previous review dismissed; new verdict is changes requested — 1 important, 1 suggestion.

Resolved: the internal-source citation in .github/STAKEHOLDERS, the unconsumed catalogue, and the description. Outstanding: commit 80a515a1 still sits in this branch's history and is publicly reachable, so the content it published isn't retracted by removing it later — the routing change wants a fresh branch off main rather than another commit here. .agents/ownership-areas/app.md also remains as a zero-byte file.

No maintainer review requested — that gate opens only once the factory's review is approving.

Responding as Warp for OSS: Open session · View run

Co-Authored-By: Warp <agent@warp.dev>
@vikvang vikvang changed the title Restore Command Completions ownership routing Restore semantic App ownership routing Sep 3, 2026

@warp-factories warp-factories Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overview

This PR reduces .github/STAKEHOLDERS to a single repository-wide owner (@acarl005) and vendors a 215-line App-team ownership catalogue at .agents/ownership-areas/app.md. Requesting changes: the catalogue publishes internal contact and unreleased-area detail in a public repository, and nothing here consumes it yet.

Concerns

  • [important] [security] .agents/ownership-areas/app.md puts ~20 employee work email addresses and descriptions of unreleased/experimental areas into a public repository, when routing only needs a GitHub handle. Anything committed here is permanently public and scrapable. Drop the catalogue from this PR; if semantic routing is wanted later, land a repository-scoped, public-safe format that carries handles only.
  • [important] Semantic routing cannot take effect from this change alone. Only .github/STAKEHOLDERS is parsed today (by the factory's owner-resolution script); no code in this repo or the deployed factory reads .agents/ownership-areas/, and the stated prerequisite warpdotdev/warp-factories-for-oss#3 is still open. Land the catalogue with its consumer, or narrow this PR to the single-owner fallback the description can honestly claim.

The * @acarl005 fallback itself is correct: the parser treats * as matching every path and takes the last matching rule, and acarl005 has write access here, so the routing works as intended once merged.

Verdict

Checks: build pass (CI), tests pass (CI test), CI green at read time (format, lint, test, CodeQL analyze pass; two CodeQL jobs pending), visual proof n/a

Found: 0 critical, 2 important, 1 suggestion, 0 nits

Recommendation: changes requested.

Responding as Warp for OSS: Open session · View run

Comment thread .agents/ownership-areas/app.md Outdated
Comment thread .agents/ownership-areas/app.md Outdated
Comment thread .github/STAKEHOLDERS Outdated
Co-Authored-By: Warp <agent@warp.dev>
@vikvang vikvang changed the title Restore semantic App ownership routing Restore command-signatures PR ownership Sep 3, 2026
@warp-factories
warp-factories Bot dismissed their stale review September 3, 2026 15:59

Superseded by a new review pass

@warp-factories warp-factories Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overview

The routing change is now clean: .github/STAKEHOLDERS carries a self-contained comment above * @acarl005, and the vendored catalogue's contents are gone. Still requesting changes — the removed content stays publicly reachable in this branch's history, so this history should not be merged.

Concerns

  • [important] [security] Commit 80a515a1 is still an ancestor of the head and remains publicly reachable through this PR, so emptying the file later does not retract the employee work addresses or unreleased-area detail it published. Merging this branch carries that commit into main's history. Replace it with a clean branch cut from main that contains only the STAKEHOLDERS change, and treat the data as already disclosed under the project's normal privacy process — a force-push cannot recall existing clones or cached objects.
  • [suggestion] .agents/ownership-areas/app.md is still added, now as a zero-byte file. It has no effect and no documented placeholder contract, and a future catalogue reader could not tell "present but empty" from "absent". Drop the path with git rm .agents/ownership-areas/app.md on the replacement branch.

Resolved since the last pass: the internal warp-ownership citation in .github/STAKEHOLDERS is now self-contained, the unconsumed catalogue is gone, and the title and description now describe what the change actually does.

Verdict

Checks: build pass, tests pass, CI green (all 8 checks passing), visual proof n/a

Found: 0 critical, 1 important, 1 suggestion, 0 nits

Recommendation: changes requested. The fix is a fresh branch, not another commit on this one.

Responding as Warp for OSS: Open session · View run

@acarl005 acarl005 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The .agents/ownership-areas/app.md is empty now but it's still there. You should delete it rather than leaving a blank file. It could potentially confuse someone (or an agent).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants