[codex] Preserve required constraints when creating links

[stopachka]

Summary

• preserve forward.required when diffing a newly created link so the first schema push emits the required step
• add a regression test covering creation of a required link and verifying the plan includes the follow-up required step

Testing

• ./node_modules/.bin/vitest run __tests__/src/migrations.test.ts

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 94e685ba-43a5-4d28-940b-708184c7eb10

📥 Commits

Reviewing files that changed from the base of the PR and between e9cadbe and 011a258.

📒 Files selected for processing (2)

• client/packages/platform/__tests__/src/migrations.test.ts
• client/packages/platform/src/migrationUtils.ts

---

📝 Walkthrough

Walkthrough

The changes modify the migration utility to reflect the required status of new attributes based on the link's forward.required property, and expose convertTxSteps for testing the required-plan step generation.

Changes

Cohort / File(s)	Summary
Migration Utility client/packages/platform/src/migrationUtils.ts	Modified linkDefToNewAttrTx to set the required field based on link.forward.required instead of always false, making the required status conditional on the link definition.
Migration Tests client/packages/platform/__tests__/src/migrations.test.ts	Added convertTxSteps to public exports and introduced a new test case "create required link" that verifies required-attribute transactions and their corresponding required-plan steps.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• [cli] handle changing required on links #2364: Modifies migration logic to handle link forward.required flag; related to how required status is tracked and emitted in migration transactions alongside this PR's changes to reflect required status in new-attribute transactions.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: preserving required constraints when creating links, which aligns with the code changes in migrationUtils.ts and the new test case.
Description check	✅ Passed	The description directly relates to the changeset, explaining both the fix (preserving forward.required) and the test addition, which matches the actual code modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/fix-required-new-link-push

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[stopachka]

I think this was actually done on purpose -- because add-attr on a large set of data would throw an error in the transaction. It would take too long. Maybe we need to seperate this into two steps? add attr, and make-required?

[dwwoelfel]

I think it's fine to do that later, since we have the same problem with adding indexed to a field.

But we might not actually validate required on this path. Can you double check that we actually validate that every entity has the field in this path?

[github-actions]

View Vercel preview at instant-www-js-codex-fix-required-new-link-push-jsv.vercel.app.