chore: merge main into development - #147
Merged
Merged
Conversation
…ubmit Accept Story music jobs only after persisting command/task/candidate IDs. The same idempotency key and spec replay; a reused key with a different spec conflicts. Retry/new-version mint a new attempt. Existing 202 jobs and the sync compatibility route stay.
Default remains a labeled fast pre-push mode. --full runs the CI-safe guards, pytest suite, ratchet, UI checks, budget and simulated E2E. A missing ratchet base or a failed analyzer is now a hard failure.
Technical review belongs to agents; a human merge click is operational. Document the intended required checks and the admin steps that still need authorization. Do not change remote protection in this PR.
bash -lc could replace nvm/Pinokio PATH after require_cmd npm. Use a non-login helper so the same npm binary is invoked.
An AND list under set -e does not abort. Run build and budget as separate steps so a failed build never reaches E2E or a CI-equivalent PASS.
Omit a key to start a new attempt. Retry needs its own key so a lost response can replay. Replay without a MiniMax checkpoint starts the worker again. Default candidate count matches the jobs endpoint.
Insert the in-memory job under the existing lock; if the reserved job_id is already live, return that snapshot instead of starting a second provider thread.
feat: reserve music generation IDs before inference with idempotent submit
fix(ci): make local validation explicit and fail closed
docs: record agent QA policy without applying GitHub rulesets
Owner
Author
|
cursor review |
PR Review — Loreframe StudioRisk: low Automated review from Findings
Changed files
CONTRIBUTING checklist
Posted by the repo PR review workflow. Re-runs on each push to the PR. |
Code healthQuality score: 49.9/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.1 points.
Markdown, JSON catalogs and tests are out of this table. Only Most complex functions
Trend vs baseline
Warnings
Ratchet passed. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Publish wipes reserved task identity
- El upsert ahora fusiona metadata reservada y conserva workflow/title, y el publish de MiniMax proyecta generation_id, candidate_id, command_id e idempotency_key desde el job.
Or push these changes by commenting:
@cursor push 90761cd2ee
Preview (90761cd2ee)
diff --git a/app/_launch_runtime.py b/app/_launch_runtime.py
--- a/app/_launch_runtime.py
+++ b/app/_launch_runtime.py
@@ -35994,6 +35994,22 @@
# cancellation/completion won. Canonical resume explicitly transitions
# the registry before the next active adapter snapshot arrives.
return existing
+ existing_metadata = existing.get("metadata") if isinstance(existing.get("metadata"), dict) else {}
+ incoming_metadata = fields.get("metadata") if isinstance(fields.get("metadata"), dict) else None
+ if incoming_metadata is not None:
+ merged_metadata = dict(existing_metadata)
+ for key, value in incoming_metadata.items():
+ if value is not None:
+ merged_metadata[key] = value
+ fields["metadata"] = merged_metadata
+ # Reservation owns attempt identity. Adapter snapshots must not
+ # replace workflow/title after submit_music_generation wrote them.
+ if any(
+ existing_metadata.get(key)
+ for key in ("generation_id", "candidate_id", "command_id", "idempotency_key")
+ ):
+ fields.pop("workflow", None)
+ fields.pop("title", None)
mutable = {
key: value for key, value in fields.items()
if key not in {"id", "created_at"}
@@ -36328,9 +36344,16 @@
"actor": provenance.get("actor") or "unknown",
"tool": provenance.get("tool") or adapter,
"capability": provenance.get("capability"),
- "command_id": command.get("command_id"),
+ "command_id": (
+ command.get("command_id")
+ or record.get("commandId")
+ or record.get("command_id")
+ ),
"workflow_id": command.get("workflow_id"),
"run_id": command.get("run_id"),
+ "generation_id": record.get("generationId") or record.get("generation_id"),
+ "candidate_id": record.get("candidateId") or record.get("candidate_id"),
+ "idempotency_key": record.get("idempotencyKey") or record.get("idempotency_key"),
},
)
diff --git a/tests/test_task_adapter_helpers.py b/tests/test_task_adapter_helpers.py
--- a/tests/test_task_adapter_helpers.py
+++ b/tests/test_task_adapter_helpers.py
@@ -256,3 +256,81 @@
assert captured["id"] == "task-model3d-backend-id"
assert captured["root_id"] == "task-series-root"
+
+
+def test_generic_adapter_publishes_reserved_music_identity():
+ publish, captured = _load_publisher("_publish_generic_legacy_task")
+
+ publish({
+ "jobId": "minimax-music-abc123def456",
+ "taskId": "task-minimax-music-abc123def456",
+ "workspace": "default",
+ "status": "queued",
+ "generationId": "gen-1",
+ "commandId": "cmd-1",
+ "candidateId": "song-1",
+ "idempotencyKey": "idem-1",
+ }, "minimax-music")
+
+ assert captured["metadata"]["generation_id"] == "gen-1"
+ assert captured["metadata"]["command_id"] == "cmd-1"
+ assert captured["metadata"]["candidate_id"] == "song-1"
+ assert captured["metadata"]["idempotency_key"] == "idem-1"
+
+
+def test_upsert_keeps_reserved_task_identity():
+ existing = {
+ "id": "task-minimax-music-abc",
+ "status": "queued",
+ "workflow": "generate_story_song",
+ "title": "Story song",
+ "metadata": {
+ "generation_id": "gen-1",
+ "candidate_id": "song-1",
+ "command_id": "cmd-1",
+ "idempotency_key": "idem-1",
+ },
+ }
+ updated = {}
+
+ class Registry:
+ def get(self, _task_id):
+ return existing
+
+ def update(self, task_id, **fields):
+ updated.update(fields)
+ return {**existing, **{key: value for key, value in fields.items()
+ if key not in {"force", "event_type", "event_exclude_fields"}}}
+
+ namespace = {
+ "_task_registry": lambda _workspace: Registry(),
+ }
+ node = _function("_upsert_canonical_task")
+ exec(compile(ast.Module(body=[node], type_ignores=[]), str(LAUNCH_PATH), "exec"), namespace)
+
+ result = namespace["_upsert_canonical_task"](
+ "default",
+ "task-minimax-music-abc",
+ workflow="minimax-music",
+ title="MiniMax Music",
+ status="queued",
+ metadata={
+ "adapter": "minimax-music",
+ "actor": "unknown",
+ "tool": "minimax-music",
+ "capability": None,
+ "command_id": None,
+ "workflow_id": None,
+ "run_id": None,
+ },
+ )
+
+ assert "workflow" not in updated
+ assert "title" not in updated
+ assert result["workflow"] == "generate_story_song"
+ assert result["title"] == "Story song"
+ assert result["metadata"]["generation_id"] == "gen-1"
+ assert result["metadata"]["candidate_id"] == "song-1"
+ assert result["metadata"]["command_id"] == "cmd-1"
+ assert result["metadata"]["idempotency_key"] == "idem-1"
+ assert result["metadata"]["adapter"] == "minimax-music"You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b1c2510. Configure here.
| # missed the checkpoint). Do not start a second worker. | ||
| public = _public_minimax_music_job(existing_live) | ||
| public["replay"] = True | ||
| return public |
There was a problem hiding this comment.
Publish wipes reserved task identity
Medium Severity
submit_music_generation stores generation_id, candidate_id, command_id, and idempotency_key on the TaskRegistry row, then start_story_music_candidates_job publishes the MiniMax job to the same taskId. That publish upserts a replacement metadata object from job provenance (usually empty) and overwrites workflow/title, so the reserved identity does not survive on the canonical task.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b1c2510. Configure here.
This was referenced Sep 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumen ejecutivo
Qué cambia
Merge commit de
main(43f75b90, #143–#145) endevelopment(afdc0de7, #146).Única resolución: intro de
SLICE_QUEUE.md(base de integración + AGENT_QA_POLICY).Para qué sirve
Dejar
developmentcomo base de integración al día. A partir de aquí los PRs ordinarios apuntan adevelopment.Impacto para el usuario
Sin cambio de producto más allá de lo ya publicado en main (#143 música, #144 validación, #145 política QA) más la política de ramas ya en development (#146).
Riesgo
Estado
Summary
Authorized one-time sync: merge
mainintodevelopmentwith a merge commit (no squash, no force-push). Keep both the development integration-base paragraph and the agent QA merge wording.Overview
Parents:
afdc0de7(development / #146) and43f75b90(main / #145). Future ordinary PRs target development.Detailed changes
Backend
Music submission contract from #143.
UI and Wizard
None in the conflict resolution.
Data, provenance and compatibility
No data migrations.
Files and ownership
Conflict only in
docs/development/SLICE_QUEUE.md. Other files take the already-merged main or development side via default merge.Validation
pytestmusic + validate wrapper + development policy — 29 passedscripts/check_documentation_links.py— PASSscripts/verify_clean_repo.py— PASSBASE_SHA=origin/development bash scripts/check_code_health_pr_base.sh— Ratchet passedBASE_SHA=origin/development bash scripts/validate_local.sh— fast passedCode quality
No new production design. Incoming hotspot growth is from already-merged #143.
Risks and rollback
Revert this merge commit on development if needed. Do not force-reset. main is unchanged.
Dependencies and acceptance
User authorized this one-time merge of main into development. No auto-merge of later PRs.
Task cost report
External provider calls: 0. Real media generations: 0.
Note
Medium Risk
Changes the Story music job acceptance path in
_launch_runtime.pywith durable idempotency and concurrency rules; mistakes could duplicate jobs or strand reservations, though contract tests cover replay and races.Overview
Syncs development with landed work from main: an idempotent music submission contract wired into the async MiniMax Story song job route, plus fast vs
--fulllocal validation, agent QA policy docs, and PR template tweaks.Music (phase 4): New
music_submissionservice reserves command/generation/task/candidate IDs and a TaskRegistry row before the provider worker starts, with JSON dedup by idempotency key + spec hash (replay vs 409), Story destination checks by ID (not title), and optional retry/new-version intents.POST …/music-candidates/jobsnow calls that reservation first: replays return the live job or restart a single worker when the checkpoint is missing; concurrent replays must not spawn duplicate workers. The 202 body gains additive fields (generationId,commandId,candidateId,idempotencyKey,replay).Tooling & process:
validate_local.shdefaults to a fast pre-push path (explicitly not CI-equivalent) and adds--fullCI-like guards, full pytest, UI budget, logging, and fail-closed behavior when the code-health base SHA cannot be resolved;check_code_health_pr_base.shis stricter. Docs addMUSIC_SUBMISSION.md,AGENT_QA_POLICY.md, updatedLOCAL_VALIDATION.md/SLICE_QUEUE.md, and PR checklist items for validation scope and current HEAD CI/review evidence.Reviewed by Cursor Bugbot for commit b1c2510. Configure here.