Summary
updatePlanOperation validates that previous.kind === 'plan' before creating a derives_from relation (src/core/operations/plan.ts:117, added by Issue 6A in #228). The same class of bug exists in reviewOperation and reproduceOperation: they accept any CID as a target without verifying the kind matches the relation type.
Specifically:
reviewOperation accepts targetCid: string but doesn't check that the target is reviewable (typically kind: 'work'). You can review a discussion or a plan today.
reproduceOperation accepts targetCid: string but doesn't check that the target is reproducible (typically kind: 'work' with artifacts). You can reproduceOperation against a chat message.
Both produce a relation that points at a contribution of the wrong kind, polluting the DAG.
Recommended fix
Two options:
A. Mirror the pattern from updatePlanOperation — add explicit kind checks in each operation:
// In reviewOperation
if (target.kind !== 'work') {
return validationErr(``Cannot review a '${target.kind}' contribution`);
}
Effort: ~30 min for both operations + tests.
B. Generalize validateRelations to accept an expected-kind map:
// In contributeOperation
await validateRelations(store, relations, {
expectedKinds: {
derives_from: ['plan'],
reviews: ['work'],
reproduces: ['work'],
},
});
Effort: ~2h. Centralizes the rule but touches more files.
Recommended option
Start with A (per-operation checks) — it's less code, mirrors the existing updatePlanOperation pattern, and is sufficient unless we add more relation types where this would matter. Move to B if/when a third operation needs the same validation.
Context
Surfaced during the #228 review (Issue 6 generalization, deferred per the locked-in plan: "Plan only this PR, file follow-up for review/reproduce"). The plan-only fix landed in commit 5ccf818.
References
Summary
updatePlanOperationvalidates thatprevious.kind === 'plan'before creating a derives_from relation (src/core/operations/plan.ts:117, added by Issue 6A in #228). The same class of bug exists inreviewOperationandreproduceOperation: they accept any CID as a target without verifying the kind matches the relation type.Specifically:
reviewOperationacceptstargetCid: stringbut doesn't check that the target is reviewable (typicallykind: 'work'). You can review a discussion or a plan today.reproduceOperationacceptstargetCid: stringbut doesn't check that the target is reproducible (typicallykind: 'work'with artifacts). You canreproduceOperationagainst a chat message.Both produce a relation that points at a contribution of the wrong kind, polluting the DAG.
Recommended fix
Two options:
A. Mirror the pattern from
updatePlanOperation— add explicit kind checks in each operation:Effort: ~30 min for both operations + tests.
B. Generalize
validateRelationsto accept an expected-kind map:Effort: ~2h. Centralizes the rule but touches more files.
Recommended option
Start with A (per-operation checks) — it's less code, mirrors the existing
updatePlanOperationpattern, and is sufficient unless we add more relation types where this would matter. Move to B if/when a third operation needs the same validation.Context
Surfaced during the #228 review (Issue 6 generalization, deferred per the locked-in plan: "Plan only this PR, file follow-up for review/reproduce"). The plan-only fix landed in commit 5ccf818.
References
src/core/operations/plan.ts:117-122— the kind-check pattern inupdatePlanOperationsrc/core/operations/contribute.ts:546-580—reviewOperationsrc/core/operations/contribute.ts:586-624—reproduceOperation