feat: add --data-set and --new-data-set flags to add command

[beck-8]

No description provided.

[BigLep]

@beck-8 : sorry this got missed. It wasn't added to the board, which is why I hadn't seen it. It has been now.

@SgtPooki : I have assigned this to you, but feel free to assign to someone else or note if we shouldn't get distracted by this currently.

[Copilot]

Pull request overview

Adds CLI support for selecting an existing dataset or forcing creation of a new dataset when running the add command.

Changes:

• Adds --dataset <id> and --new-dataset flags to the add CLI command and maps them into AddOptions.
• Extends AddOptions with datasetId / createNewDataset and forwards these into createStorageContext dataset options.
• Updates unit tests to cover dataset option forwarding into storage context.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/commands/add.ts	Adds new CLI flags and maps them into AddOptions passed to runAdd.
src/add/types.ts	Extends AddOptions to include dataset selection fields.
src/add/add.ts	Forwards dataset selection fields into CreateStorageContextOptions.dataset.
src/test/unit/add.test.ts	Adds assertions that dataset selection fields are passed into storage context creation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

--dataset and --new-dataset can currently be provided together, which produces ambiguous CLI intent (and results in both datasetId and createNewDataset potentially being set). Please enforce mutual exclusivity (e.g., validate in the action handler and throw a clear error when both are set).

[Copilot]

parseInt(dataset, 10) is used without validation. If the user passes a non-integer (e.g., "abc" or "123x"), this can yield NaN or a surprising value and the flag will be silently ignored downstream. Please validate that the parsed ID is a positive safe integer and surface a helpful error when invalid.

Suggested change

	const addOptions: AddOptions = {
	...addOptionsFromCli,
	filePath: path,
	...(dataset && { datasetId: parseInt(dataset, 10) }),
	let datasetId: number | undefined
	if (dataset !== undefined) {
	const trimmed = String(dataset).trim()
	// Ensure the dataset ID is a positive safe integer and fully numeric
	if (!/^[0-9]+$/.test(trimmed)) {
	console.error(`Invalid value for --dataset: "${dataset}". Expected a positive integer ID.`)
	process.exit(1)
	}
	const parsed = Number(trimmed)
	if (!Number.isSafeInteger(parsed) || parsed <= 0) {
	console.error(`Invalid value for --dataset: "${dataset}". Expected a positive safe integer ID.`)
	process.exit(1)
	}
	datasetId = parsed
	}
	const addOptions: AddOptions = {
	...addOptionsFromCli,
	filePath: path,
	...(datasetId !== undefined && { datasetId }),

[Copilot]

The earlier metadata test now only asserts initializeSynapse/createStorageContext and no longer verifies that metadata is forwarded into the upload call (e.g., via performUpload). Either restore the upload-side assertion or adjust the test description/expectations so it matches the behavior being verified.

[SgtPooki]

@beck-8 can you ensure we don't remove the metadata assertion

[SgtPooki]

makes sense, but one callout:

naming conventions are DataSet is two separate words, so we should stick to that, or add an alias at the very least. dataSet or data-set over dataset, but we have an alias in other places, so I'm good with dataset as long as we also have data-set in the CLI.

vars internally should use camelCase

[SgtPooki]

thanks for the quick updates @beck-8.. a few last things

[SgtPooki]

@beck-8 can you ensure we don't remove the metadata assertion

[SgtPooki]

idk if this solves the mutual exclusivity problem brought up by a different copilot comment. you will want .conflicts('otherOption') for these