diff --git a/src/services/contributor-issue-draft.ts b/src/services/contributor-issue-draft.ts index e72eef309e..b3d8bac82c 100644 --- a/src/services/contributor-issue-draft.ts +++ b/src/services/contributor-issue-draft.ts @@ -124,8 +124,15 @@ const GENERIC_TESTING_REQUIREMENTS = [ "Public GitHub output must stay advisory and must not imply guaranteed participation outcomes.", ]; +/** The manifest's testExpectations, filtered to public-safe entries and formatted for a contributor-facing + * draft. The single source shared by both testingRequirements and implementationRequirements so a public-unsafe + * expectation can never leak through one path while the other filters it, and neither re-diverges (#9704). */ +function publicSafeTestExpectations(manifest: FocusManifest): string[] { + return manifest.testExpectations.filter(isFocusManifestPublicSafe).map(formatContributorIssueDraftTestExpectation); +} + export function buildContributorIssueDraftTestingRequirements(manifest: FocusManifest): string[] { - const policyExpectations = manifest.testExpectations.filter(isFocusManifestPublicSafe).map(formatContributorIssueDraftTestExpectation); + const policyExpectations = publicSafeTestExpectations(manifest); if (policyExpectations.length === 0) return [...GENERIC_TESTING_REQUIREMENTS]; return [ ...policyExpectations, @@ -483,7 +490,9 @@ function wantedPathCandidate(repoFullName: string, wantedPath: string, openIssue implementationRequirements: [ `Stay within ${wantedPath} unless safety or release readiness requires adjacent files.`, "Avoid blocked manifest paths and keep PRs narrowly scoped.", - ...(manifest.testExpectations.length > 0 ? manifest.testExpectations.map((entry) => `Run ${entry} before requesting review.`) : []), + // Same public-safe-filtered, formatted expectations as testingRequirements -- an unfiltered raw + // `Run ${entry}` here would both double-format and leak a public-unsafe expectation the sibling drops (#9704). + ...publicSafeTestExpectations(manifest), ], publicPrivateBoundaries: [ "Public issues must not promise compensation, sort contributors, or expose private maintainer-only claims.", diff --git a/test/unit/contributor-issue-draft.test.ts b/test/unit/contributor-issue-draft.test.ts index 7a923e52f8..b9e0d37b72 100644 --- a/test/unit/contributor-issue-draft.test.ts +++ b/test/unit/contributor-issue-draft.test.ts @@ -457,6 +457,36 @@ describe("contributor issue drafts", () => { expect(candidates.some((entry) => entry.sections.implementationRequirements.some((line) => line.includes("npm run test:ci")))).toBe(true); }); + it("#9704: a wanted-path candidate's implementationRequirements carry the SAME public-safe-filtered expectations as testingRequirements", () => { + const manifest = parseFocusManifestContent( + // A safe expectation that survives the filter and an unsafe one that must be dropped from BOTH lists. + '{"wantedPaths":["src/"],"testExpectations":["npm run test:ci","wallet seed phrase"],"issueDiscoveryPolicy":"discouraged"}', + "repo_file", + ); + const testing = buildContributorIssueDraftTestingRequirements(manifest); + const candidates = buildContributorIssueDraftCandidates({ + repoFullName: "owner/repo", + repo: { fullName: "owner/repo", isRegistered: true } as never, + settings: { requireLinkedIssue: false } as never, + lane: buildLaneAdvice({ fullName: "owner/repo", isRegistered: true } as never, "owner/repo"), + configQuality: buildConfigQuality({ fullName: "owner/repo" } as never, [], [], "owner/repo"), + labelAudit: buildLabelAudit({ fullName: "owner/repo" } as never, [], [], [], "owner/repo"), + queueHealth: buildQueueHealth({ fullName: "owner/repo" } as never, [], [], buildCollisionReport("owner/repo", [], [])), + contributorIntakeHealth: buildContributorIntakeHealth({ fullName: "owner/repo" } as never, [], [], "owner/repo", buildCollisionReport("owner/repo", [], [])), + openIssues: [], + upstreamDriftWarnings: [], + focusManifest: manifest, + }); + const wantedPathCandidate = candidates.find((entry) => entry.topic?.startsWith("focus:wanted_path:")); + expect(wantedPathCandidate).toBeDefined(); + const implExpectations = wantedPathCandidate!.sections.implementationRequirements.filter((line) => line.includes("test:ci") || /wallet|seed phrase/i.test(line)); + const testingExpectations = testing.filter((line) => line.includes("test:ci") || /wallet|seed phrase/i.test(line)); + // Both derive from the shared publicSafeTestExpectations helper: identical list, unsafe entry gone from each. + expect(implExpectations).toEqual(testingExpectations); + expect(implExpectations).not.toEqual([]); + expect(JSON.stringify(implExpectations)).not.toMatch(/wallet|seed phrase/i); + }); + it("ignores closed issues and empty title keys when checking duplicates", () => { const fingerprint = "fp"; const title = "feat(issues): address validation policy readiness for repo"; @@ -490,7 +520,7 @@ describe("contributor issue drafts", () => { expect(new Set(candidates.map((entry) => entry.topic)).size).toBe(candidates.length); }); - it("skips unsafe drafts when wanted-path validation text fails public hygiene", async () => { + it("#9704: filters a public-unsafe testExpectation out of BOTH requirement lists (never leaks into the draft)", async () => { const env = createTestEnv(); await upsertRepoFocusManifest(env, "owner/unsafe-path", { wantedPaths: ["src/unsafe-path-only/"], @@ -500,7 +530,12 @@ describe("contributor issue drafts", () => { }); vi.spyOn(repositories, "listOpenIssues").mockResolvedValue([]); const result = await generateContributorIssueDrafts(env, "owner/unsafe-path", { dryRun: true, limit: 10 }); - expect(result.drafts.some((draft) => draft.status === "skipped_unsafe")).toBe(true); + // Before #9704, implementationRequirements emitted a raw `Run wallet seed phrase ...` that only got caught + // downstream as skipped_unsafe. Now BOTH lists filter through isFocusManifestPublicSafe, so the unsafe + // expectation never reaches the draft at all -- the wanted-path draft is produced and carries no such text. + expect(result.drafts.some((draft) => draft.status === "skipped_unsafe")).toBe(false); + expect(JSON.stringify(result.drafts)).not.toMatch(/wallet|seed phrase/i); + expect(result.drafts.some((draft) => draft.topic?.startsWith("focus:wanted_path:"))).toBe(true); }); it("returns null for invalid repo names when creating GitHub issues", async () => {