diff --git a/src/services/decision-pack.ts b/src/services/decision-pack.ts index 3166293c71..19f048bd4c 100644 --- a/src/services/decision-pack.ts +++ b/src/services/decision-pack.ts @@ -688,14 +688,16 @@ function buildRepoDecision(args: { mergedPullRequests: args.totals?.mergedPullRequestsTotal ?? args.syncState?.recentMergedPullRequestsCount ?? 0, closedUnmergedPullRequests: args.totals?.closedUnmergedPullRequestsTotal ?? 0, }; - const baseEmissionShare = config?.emissionShare ?? 0; // Lane shares are a split of the OSS *mining* pool (emissionShare * OSS_EMISSION_SHARE), matching - // preview.ts laneMath (directPrSlice/issueDiscoverySlice) and reward-risk.ts. The raw emissionShare - // field stays raw (it mirrors laneMath.repoEmissionShare). + // preview.ts laneMath (directPrSlice/issueDiscoverySlice) and reward-risk.ts. Both shares are clamped + // to [0, 1] exactly as preview.ts does, since registry config is untrusted and unclamped at ingestion; + // without this an out-of-range issueDiscoveryShare > 1 produces a negative directPrShare. + const baseEmissionShare = clamp(config?.emissionShare ?? 0, 0, 1); + const issueDiscoveryShare = clamp(config?.issueDiscoveryShare ?? 0, 0, 1); const rewardUpside = { emissionShare: round(baseEmissionShare), - directPrShare: round(baseEmissionShare * ossEmissionShare * (1 - (config?.issueDiscoveryShare ?? 0))), - issueDiscoveryShare: round(baseEmissionShare * ossEmissionShare * (config?.issueDiscoveryShare ?? 0)), + directPrShare: round(baseEmissionShare * ossEmissionShare * (1 - issueDiscoveryShare)), + issueDiscoveryShare: round(baseEmissionShare * ossEmissionShare * issueDiscoveryShare), maintainerCut: round(config?.maintainerCut ?? 0), }; const blockers = scoreBlockersFor(args.repo.fullName, lane.lane, args.roleContext, args.outcome); diff --git a/test/unit/decision-pack.test.ts b/test/unit/decision-pack.test.ts index 7e5f01cd7e..f92e7b3896 100644 --- a/test/unit/decision-pack.test.ts +++ b/test/unit/decision-pack.test.ts @@ -136,6 +136,29 @@ describe("decision-pack service", () => { expect(overridden.directPrShare).toBeCloseTo(0.04 * 0.8, 10); // 0.032 }); + it("clamps out-of-range registry shares so lane shares never go negative (matches preview.ts)", () => { + const outsideRole = { maintainerLane: false } as any; + // Registry config is untrusted and unclamped at ingestion. An out-of-range issueDiscoveryShare > 1 + // must clamp to 1 (as preview.ts laneMath does) rather than produce a negative directPrShare. + const outOfRange = __decisionPackInternals.buildRepoDecision({ + repo: repo("owner/out-of-range", 0.01, 1.5), + roleContext: outsideRole, + outcome: undefined, + }).rewardUpside; + expect(outOfRange.directPrShare).toBe(0); // 0.01 * 0.9 * (1 - clamp(1.5,0,1)) = 0, never negative + expect(outOfRange.directPrShare).toBeGreaterThanOrEqual(0); + expect(outOfRange.issueDiscoveryShare).toBeCloseTo(0.01 * 0.9 * 1, 10); // 0.009 + + // A negative issueDiscoveryShare clamps to 0, keeping the full slice on the direct-PR lane. + const negative = __decisionPackInternals.buildRepoDecision({ + repo: repo("owner/negative", 0.01, -0.5), + roleContext: outsideRole, + outcome: undefined, + }).rewardUpside; + expect(negative.issueDiscoveryShare).toBe(0); + expect(negative.directPrShare).toBeCloseTo(0.01 * 0.9 * 1, 10); // 0.009 + }); + it("feeds repo outcome patterns into repo decisions without inflating maintainer-lane evidence", () => { const outsideRole = { maintainerLane: false } as any; const maintainerRole = { maintainerLane: true } as any;