Skip to content

fix(review): scope private review context by owner #1582

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JSONbored wants to merge 1 commit into from
Closed

fix(review): scope private review context by owner #1582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/selfhost/private-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,16 @@ export function makeLocalManifestReader(dir: string | undefined): RepoFocusManif
};
}

/** Per-repo review-context candidate FOLDERS (relative to GITTENSORY_REPO_CONFIG_DIR): `{owner}__{repo}/review` then
* `{repo}/review`. Same owner/repo validation as localConfigCandidates; an invalid full name yields none. (#review-skills) */
/** Per-repo review-context candidate FOLDERS (relative to GITTENSORY_REPO_CONFIG_DIR): only
* `{owner}__{repo}/review`. Review prompt material is private and must stay owner-scoped; an invalid full name
* yields none. (#review-skills) */
function reviewContextFolders(repoFullName: string): string[] {
const slash = repoFullName.indexOf("/");
if (slash <= 0 || slash === repoFullName.length - 1 || slash !== repoFullName.lastIndexOf("/")) return [];
const owner = repoFullName.slice(0, slash).toLowerCase();
const repo = repoFullName.slice(slash + 1).toLowerCase();
if (!GITHUB_OWNER_SEGMENT.test(owner) || !isSafeRepoSegment(repo)) return [];
return [join(`${owner}__${repo}`, "review"), join(repo, "review")];
return [join(`${owner}__${repo}`, "review")];
}

/** Parse a skill markdown file into {name, when, body}. YAML frontmatter (`---\nname:\nwhen:\n---`) is optional; name
Expand All @@ -108,7 +109,7 @@ export function parseReviewSkill(filename: string, text: string): RepoReviewSkil
}

/** Build the container-local review-context reader over GITTENSORY_REPO_CONFIG_DIR, or null when the dir is unset. Per
* repo (first existing folder wins) reads `review/CLAUDE.md` (the guide) + every `review/skills/*.md` (rubric modules,
* repo reads owner-qualified `review/CLAUDE.md` (the guide) + every `review/skills/*.md` (rubric modules,
* sorted). Missing files/dir degrade to nulls/empty; a per-file read error skips that file. (#review-skills) */
export function makeLocalReviewContextReader(dir: string | undefined): RepoReviewContextReader | null {
const trimmed = (dir ?? "").trim();
Expand Down
9 changes: 7 additions & 2 deletions test/unit/private-config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,17 @@ describe("makeLocalReviewContextReader (#review-skills)", () => {
expect(ctx.skills.map((s) => s.name)).toEqual(["a-first", "second"]); // sorted by filename; .txt ignored
});

it("falls back to the bare repo-name folder; returns empty for a missing or invalid repo", async () => {
it("ignores bare repo-name review folders to keep private prompt material owner-scoped", async () => {
const dir = mkdtempSync(join(tmpdir(), "gt-review-"));
mkdirSync(join(dir, "metagraphed", "review"), { recursive: true });
writeFileSync(join(dir, "metagraphed", "review", "CLAUDE.md"), "Bare-folder guide.\n");
const reader = makeLocalReviewContextReader(dir)!;
expect((await reader("JSONbored/metagraphed")).guide).toContain("Bare-folder guide.");
expect(await reader("JSONbored/metagraphed")).toEqual({ guide: null, skills: [] });
});

it("returns empty for a missing or invalid repo", async () => {
const dir = mkdtempSync(join(tmpdir(), "gt-review-"));
const reader = makeLocalReviewContextReader(dir)!;
expect(await reader("JSONbored/unknown-repo")).toEqual({ guide: null, skills: [] }); // no folder
expect(await reader("owner/..")).toEqual({ guide: null, skills: [] }); // invalid repo segment → no candidates
expect(await reader("noslash")).toEqual({ guide: null, skills: [] }); // invalid full name (no slash)
Expand Down
Loading