mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 06:16:06 -10:00
fix(evals): treat a job-less TestOps plan row as unassigned during registration (#19601)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Runner E2E cases run from Allure TestOps plans through
`runner-full-stack-e2e.yml`
> - #19549 added a sibling "Reliability — staging soak" plan and a
one-shot registration workflow
> - The first live `register` dispatch created the seven cases and the
plan, then stopped with `plan_job_conflict`
> - Live TestOps lists a new plan's uncovered cases as one row without a
job, and the code read that row as another job
> - This pull request treats a job-less row as "not assigned"
> - The benefit is that a second `register` dispatch can assign the plan
to the standing GitHub job and finish registration
## Linked Issues or Issue Description
Refs #19549
**What happened?**
The `runner-e2e-testops-registration.yml` `register` run on master
failed with `reliability_testops_registration_plan_job_conflict`. It had
already created the seven cases and the plan. `GET
/api/testplan/<id>/job` for that plan returned `[{"testCasesCount":7}]`.
The row has no `job` field.
**Expected behavior**
Registration treats a plan with only a job-less row as unassigned. It
assigns the plan to the standing Product regression GitHub job and reads
the assignment back.
**Steps to reproduce**
1. Dispatch `runner-e2e-testops-registration.yml` on master with
`operation=register` when no Reliability plan exists.
2. The new plan reports one job-less row.
3. Registration fails with `plan_job_conflict` before the assignment
step.
**Paperclip version or commit**
master at 744056ecc8 and later.
## What Changed
- `tests/runner-e2e/reliability-testops-registration.mjs`: add
`jobAssignments()`. It keeps the array check and ignores rows without a
`job`. Inspect, the pre-assignment conflict check and the
post-assignment read-back use it.
- `tests/runner-e2e/reliability-testops.test.mjs`: the fake TestOps now
returns the live job-less row for a new plan. A new test reproduces the
half-registered plan and checks that `register` assigns the standing job
and that the receipt becomes complete.
## Verification
- `node --test tests/runner-e2e/reliability-testops.test.mjs
tests/runner-e2e/product-regression-testops.test.mjs`: 28/28 pass.
- Red before the fix: with the old registration module, 4 tests fail,
including the new one. Green after: 13/13 in the reliability file.
- Live check after merge: dispatch `register` again. The cases and plan
already exist, so it only assigns the plan to the standing job.
## Risks
- Low risk. The change touches only the one-shot registration helper and
its tests. It does not change provider cells, launch selection or
reporting.
- A plan assigned to another job still fails with `plan_job_conflict`.
The existing test still covers that case.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code with tool use
(shell, browser read of the TestOps API response), standard context.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
8c7e86e73b
commit
02214b1a3c
2 files changed
+29
-6
No files matched your search
@@ -94,8 +94,7 @@ export async function inspectReliability(api) {
|
||||
if (candidates.length !== 1 || !positive(candidates[0].job.id)) fail('standing_job');
|
||||
const job = await api(`/api/job/${candidates[0].job.id}`);
|
||||
if (job?.id !== candidates[0].job.id || job.type !== 'github' || !workflowUrls.has(job.url)) fail('standing_job');
|
||||
const assignments = plan ? await api(`/api/testplan/${plan.id}/job`) : [];
|
||||
if (!Array.isArray(assignments)) fail('plan_job');
|
||||
const assignments = plan ? jobAssignments(await api(`/api/testplan/${plan.id}/job`)) : [];
|
||||
return { caseIds: Object.fromEntries(found), plan, jobId: job.id,
|
||||
standingParameters: Array.isArray(candidates[0].job.parameters) ? candidates[0].job.parameters : [], assignments };
|
||||
}
|
||||
@@ -134,8 +133,8 @@ export async function registerReliability(api) {
|
||||
plan = await api(`/api/testplan/${plan.id}`);
|
||||
}
|
||||
assertPlan(plan, ids);
|
||||
let assignments = await api(`/api/testplan/${plan.id}/job`);
|
||||
if (!Array.isArray(assignments) || assignments.length > 1 || assignments.some(row => row?.job?.id !== before.jobId)) fail('plan_job_conflict');
|
||||
let assignments = jobAssignments(await api(`/api/testplan/${plan.id}/job`));
|
||||
if (assignments.length > 1 || assignments.some(row => row?.job?.id !== before.jobId)) fail('plan_job_conflict');
|
||||
if (!assignments.length) {
|
||||
await api(`/api/testplan/${plan.id}/assign`, 'POST', { jobId: before.jobId, selection: { inverted: false, manual: false, leafsInclude: ids } });
|
||||
created.assignment = true;
|
||||
@@ -145,13 +144,21 @@ export async function registerReliability(api) {
|
||||
await api(`/api/testplan/${plan.id}/jobparameter`, 'POST', { jobId: before.jobId, parameters: before.standingParameters });
|
||||
created.parameters = true;
|
||||
}
|
||||
assignments = await api(`/api/testplan/${plan.id}/job`);
|
||||
if (!Array.isArray(assignments) || assignments.length !== 1 || assignments[0]?.job?.id !== before.jobId
|
||||
assignments = jobAssignments(await api(`/api/testplan/${plan.id}/job`));
|
||||
if (assignments.length !== 1 || assignments[0]?.job?.id !== before.jobId
|
||||
|| assignments[0].testCasesCount !== ids.length
|
||||
|| (before.standingParameters.length && !same(assignments[0].job.parameters ?? [], before.standingParameters))) fail('plan_job');
|
||||
return { caseIds, plan, jobId: before.jobId, created };
|
||||
}
|
||||
|
||||
// TestOps lists a plan's cases that no job covers as a job-less row, e.g.
|
||||
// `[{ testCasesCount: 7 }]` for a freshly created plan. That row is not a job
|
||||
// assignment; only rows naming a job are.
|
||||
function jobAssignments(rows) {
|
||||
if (!Array.isArray(rows)) fail('plan_job');
|
||||
return rows.filter(row => row?.job != null);
|
||||
}
|
||||
|
||||
export function registrationReceipt(operation, state, env) {
|
||||
const caseIds = Object.fromEntries(reliabilityDefinitions.filter(row => state.caseIds[row.name]).map(row => [row.name, state.caseIds[row.name]]));
|
||||
const complete = Object.keys(caseIds).length === reliabilityDefinitions.length && positive(state.plan?.id)
|
||||
|
||||
@@ -196,6 +196,8 @@ function fakeTestOps({ existing = [], plan = null, assignments = [], standing =
|
||||
if (method === 'GET' && url.pathname === '/api/testplan') return { content: state.plan ? [{ id: state.plan.id, name: state.plan.name }] : [], totalPages: 1 };
|
||||
if (method === 'POST' && url.pathname === '/api/testplan') {
|
||||
state.plan = { id: 77, projectId: 1, name: body.name, treeSelection: body.treeSelection, testCasesCount: body.treeSelection.leafsInclude.length };
|
||||
// Live TestOps lists a new plan's uncovered cases as one job-less row.
|
||||
state.assignments = [{ testCasesCount: body.treeSelection.leafsInclude.length }];
|
||||
return { id: 77 };
|
||||
}
|
||||
if (method === 'GET' && url.pathname === '/api/testplan/77') return state.plan;
|
||||
@@ -293,6 +295,20 @@ test('registration refuses a standing plan without exactly one trusted job or a
|
||||
await assert.rejects(registerReliability(api), /plan_job_conflict/);
|
||||
});
|
||||
|
||||
test('a plan TestOps reports only as a job-less row is unassigned, so registration assigns the standing job', async () => {
|
||||
const { api, state } = fakeTestOps();
|
||||
await registerReliability(api);
|
||||
// Reproduces staging plan 135: created, then the run stopped before assignment.
|
||||
state.assignments = [{ testCasesCount: 7 }];
|
||||
const inspected = await inspectReliability(api);
|
||||
assert.deepEqual(inspected.assignments, []);
|
||||
assert.equal(registrationReceipt('inspect', inspected, env).complete, false);
|
||||
const repaired = await registerReliability(api);
|
||||
assert.equal(repaired.created.assignment, true);
|
||||
assert.deepEqual(state.assignments.map(row => row.job?.id), [50]);
|
||||
assert.equal(registrationReceipt('register', repaired, { GITHUB_RUN_ID: '1', GITHUB_SHA: workflowSha }).complete, true);
|
||||
});
|
||||
|
||||
test('the workflows expose the reliability plan without new provider or reporting authority', async () => {
|
||||
const full = parse(await readFile(new URL('../../.github/workflows/runner-full-stack-e2e.yml', import.meta.url), 'utf8'));
|
||||
assert.deepEqual(full.on.workflow_dispatch.inputs.testops_plan.options, ['product-regression', 'reliability']);
|
||||
|
||||
Reference in new issue
Block a user