281 lines
24 KiB
Markdown
281 lines
24 KiB
Markdown
---
|
|
name: video-editing-change-control
|
|
description: Load before proposing, implementing, reviewing, merging, releasing, or rolling back any change to this video-editing repository, especially changes to runtime behavior, configuration defaults, APIs, persistence, FFmpeg rendering, highlight selection, local CV/TTS/music/SFX models, assets, security, deployment, or production promotion. Also load when deciding what evidence and approvals a change requires.
|
|
---
|
|
|
|
# Video Editing Change Control
|
|
|
|
## Purpose
|
|
|
|
Control every change from intent through evidence and promotion. Treat **promotion** as moving a change into a more trusted environment or declaring it production-ready. Treat a **gate** as evidence or approval that must exist before promotion.
|
|
|
|
Use the repository state dated **2026-07-21** as the current baseline. The Fortune 500 requirements below are the target acceptance standard, not a description of the repository today.
|
|
|
|
## When not to use this skill
|
|
|
|
- Do not use this as a configuration catalog. Load `video-editing-config-and-flags`.
|
|
- Do not use this to diagnose a failure. Load `video-editing-debugging-playbook`, then return here before changing behavior.
|
|
- Do not use this as the test procedure. Load `video-editing-validation-and-qa`, then attach its evidence here.
|
|
- Do not use this to reconstruct an old failure. Load `video-editing-failure-archaeology`.
|
|
- Do not use this to learn module boundaries. Load `video-editing-architecture-contract`.
|
|
- Do not use this to operate schedulers or renders. Load `video-editing-run-and-operate`; never use an operating procedure to bypass the approval gates here.
|
|
- Do not use this to judge cinematic quality by eye. Load the highlight-quality campaign and proof/analysis skills; bring their measurements back to this gate.
|
|
|
|
## Establish the baseline first
|
|
|
|
Run these read-only checks from the repository root before editing:
|
|
|
|
```bash
|
|
git status --short
|
|
git branch --show-current
|
|
git log -10 --oneline --decorate
|
|
git diff --stat
|
|
git diff
|
|
```
|
|
|
|
Record the starting commit, existing uncommitted files, scope, and expected observable outcome. Do not overwrite, reset, clean, or reformat unrelated work.
|
|
|
|
Confirm important current facts rather than trusting a plan checkbox:
|
|
|
|
```bash
|
|
test -f pom.xml
|
|
test -f README.md
|
|
test -x mvnw
|
|
find .github -maxdepth 3 -type f -print 2>/dev/null
|
|
find src/test/java -name '*Test.java' | sort
|
|
rg -n 'enabled:|auto-start:|render-enabled:|require-director-approval:|fallback-to-heuristic:|strict-runtime:' src/main/resources
|
|
rg -n 'pip install|get_pretrained|write_silence|write_fallback_tone' tools src/main
|
|
```
|
|
|
|
As of 2026-07-21, `pom.xml` exists; `README.md`, Maven Wrapper, `.github/` CI, container definitions, and deployment definitions do not. The current working tree passes `mvn -q -o verify` with 245 tests in 62 test classes. Phase 1 also reproduced a clean archived-checkout `mvn verify` failure in `LocalAssetGenerationStageTest` because that revision could depend on an untracked local asset virtual environment or an audible TTS fallback. The working tree removed that fallback dependency, but do not call the build reproducible until a fresh clean, offline run passes on macOS and Linux.
|
|
|
|
## Apply the non-negotiables
|
|
|
|
Stop a change immediately when it violates any of the seven user prohibitions below. **These prohibitions have no waiver or risk-acceptance path.** Existing violations are gaps to remove, never precedent. `video-editing-change-control` is the authoritative home for this policy; sibling skills state only workflow-specific consequences.
|
|
|
|
| Rule | Required behavior | Why and repository evidence |
|
|
|---|---|---|
|
|
| No automatic dependency or model downloads | Provision signed, checksummed dependencies and model weights before startup. Model/media inference and acquisition must make no network connection, including loopback remote procedure calls (RPCs). Fail startup or the affected job when an approved local artifact is absent. | `tools/run_local_asset_worker.sh` still creates a virtualenv and runs `pip install`; do not invoke it. The working-tree runtime verifier no longer bootstraps and the asset worker accepts only existing local model paths with offline hub flags. |
|
|
| No external AI service in the production media path | Keep highlight identification, direction, CV, voiceover, music, and SFX inference within the approved runtime using pre-provisioned local models. | `docs/cinematic-highlight-operator-checklist.md` currently instructs an operator to run Codex, Claude, or another filesystem-capable AI. That is a manual development workflow, not the target production architecture. |
|
|
| No unlicensed asset | Require machine-readable origin, license identifier/terms, checksum, and allowed use for every music, SFX, voice, font, LUT, and model artifact before render approval. | `AssetLicensePolicy` now excludes media without a nonblank adjacent `.license.txt`, preserves that sidecar during copies, and render preflight requires licensed requested audio. A sidecar string alone still does not prove origin, checksum, allowed use, or commercial rights. |
|
|
| No placeholder silence or tones | Fail closed when requested voiceover, music, or SFX cannot be generated or resolved. Never promote synthetic silence or a diagnostic sine tone as a finished asset. | The 2026-07-21 working tree removes Python/Java silence, tone, and host-speech success paths and tests missing-asset render rejection. Preserve that gate. |
|
|
| No unapproved rendering | Require a validated plan, complete licensed assets, measured QA, and an explicit approval artifact or auditable approval record before FFmpeg starts. | Highlight defaults now keep rendering disabled and require `approved.flag` if enabled. The flag is not bound to source/plan/configuration digests, and `POST /v1/edit-projects/{projectId}:render` still has no approval gate, so neither path is production authorization. |
|
|
| No unapproved network access | Use in-process calls or explicitly designed non-network inter-process communication for model/media inference. The current loopback CV HTTP worker is noncompliant and cannot be a certified path. Separately, API, PostgreSQL, object-storage, and telemetry connections are production integrations: each requires explicit approval, authentication/authorization where applicable, transport security, least privilege, bounded timeouts, and tested failure behavior. Permit no external AI or unapproved egress. | Local CV currently uses configured loopback HTTP at `127.0.0.1:8091`; S3 and HTTP APIs also exist. Loopback is still network, and network presence is not approval. |
|
|
| No silent production-default change | List old value, new value, environments affected, migration, rollback, and operator impact. Require explicit review for every default or `matchIfMissing` change. | `application.yml` currently enables folder scheduling, editing, local workers, the local director, and highlight ingestion by default. Highlight rendering now defaults off, but a normal start can still consume files and start the network-capable CV bootstrap path. |
|
|
| No claim without measured evidence | Label plans, generated artifacts, and heuristic checks accurately. A passing test or existing MP4 is not proof of cinematic or production quality. | Commit `5d889b0` explicitly recorded “working version but not cinematic.” Several highlight QA checks in `HighlightFfmpegRenderer.buildQaReport` are hard-coded `true`, including duration, asset, overlay, and mastering claims. |
|
|
|
|
## Classify the change
|
|
|
|
Choose the highest applicable class. Splitting a large change does not lower its class when the parts jointly alter behavior.
|
|
|
|
| Class | Examples here | Minimum route |
|
|
|---|---|---|
|
|
| C0: knowledge-only | Skills, docs, comments, diagnostic scripts that cannot execute in a build or runtime | Ground-truth review, command/path verification, docs consistency review |
|
|
| C1: isolated implementation | Internal refactor with identical API, config, storage, scheduling, render, and media output behavior; focused deterministic test | Focused tests, full offline unit suite, compatibility review |
|
|
| C2: behavior or contract | API/DTO/error change, config/flag, dependency/plugin, scheduler state, filesystem contract, queue/retry, observability, performance, or renderer command | C1 plus integration/contract evidence, migration and rollback, security and operations review |
|
|
| C3: high-impact | Highlight ranking, director plan semantics, local model/runtime, asset generation/licensing, automatic rendering, auth, persistence migration, deployment, data deletion, production default | All gates; adversarial validation; macOS and Linux evidence; explicit security, data, operations, and product/creative approval |
|
|
| Emergency | Active confidentiality, integrity, availability, or legal incident | Use the smallest reversible mitigation, preserve evidence, obtain incident authority, test the failure path, and complete C2/C3 follow-up before normal promotion |
|
|
|
|
Treat any unknown impact as C3 until discriminating evidence lowers it.
|
|
|
|
## Write the change record
|
|
|
|
For every C2/C3 decision, add or update an ADR when the repository has an ADR location. Until that location exists, include this record in the pull request or review artifact; do not invent a docs-of-record location silently.
|
|
|
|
```text
|
|
Requirement:
|
|
Selected approach:
|
|
Alternatives considered:
|
|
Benefits and trade-offs:
|
|
Operational consequences:
|
|
Security implications:
|
|
Verification evidence:
|
|
Revisit conditions:
|
|
Owner and review date:
|
|
Change class and affected environments:
|
|
Rollback or roll-forward procedure:
|
|
```
|
|
|
|
State what will not change. Map each claim to a test, measurement, schema comparison, scan, or reviewed artifact.
|
|
|
|
## Execute the gates
|
|
|
|
### Gate 1: scope and dependency direction
|
|
|
|
- Keep the modular monolith unless measured business requirements justify distribution.
|
|
- Put business rules in domain/application code; keep controllers, persistence, HTTP, filesystem, FFmpeg, and model processes at adapter boundaries.
|
|
- Do not add a dependency, abstraction, repository base class, mapper layer, CQRS infrastructure, cache, circuit breaker, virtual thread, or service merely to satisfy a pattern checklist.
|
|
- For each new dependency, record maintenance status, stable version, license, vulnerability result, why Spring Boot dependency management is insufficient if overridden, and removal plan.
|
|
- Block snapshots, milestones, release candidates, deprecated dependencies, and undocumented overrides.
|
|
- Require automated architecture rules before claiming module boundaries, no cycles, or inward dependencies are enforced. None were found in the 2026-07-21 test inventory.
|
|
|
|
### Gate 2: forbidden-path scan
|
|
|
|
Run before review:
|
|
|
|
```bash
|
|
rg -n 'pip install|curl |wget |get_pretrained|from_pretrained|snapshot_download' . --glob '!target/**' --glob '!.git/**'
|
|
rg -n 'write_silence|write_fallback_tone|fallback-to-heuristic|fallback.*copy' tools src/main docs
|
|
rg -n 'render-enabled:|auto-render|require.*approval|auto-start:|matchIfMissing' src/main src/test docs
|
|
rg -n -i 'api[_-]?key|secret|password|token|private[_-]?key' . --glob '!target/**' --glob '!.git/**'
|
|
rg -n '<version>.*(SNAPSHOT|M[0-9]+|RC[0-9]*)</version>' pom.xml
|
|
```
|
|
|
|
Classify every hit. Existing violations do not authorize new ones. Any new runtime download, placeholder, approval bypass, secret, or prerelease dependency blocks the change.
|
|
|
|
### Gate 3: deterministic verification
|
|
|
|
Use the locally provisioned toolchain without network access:
|
|
|
|
```bash
|
|
mvn -o test
|
|
mvn -o verify
|
|
git diff --check
|
|
```
|
|
|
|
`mvn verify` currently runs Surefire and JaCoCo; JaCoCo enforces 100% instruction, line, and branch coverage only for `org.example.videoclips.folder`. It does not provide the target 90% domain/application or 80% overall gate. Add focused tests at the lowest useful layer and use real FFmpeg/model/PostgreSQL infrastructure where behavior depends on it.
|
|
|
|
For clean-checkout evidence without modifying the current tree:
|
|
|
|
```bash
|
|
tmp_dir="$(mktemp -d)"; git archive HEAD | tar -x -C "$tmp_dir"; (cd "$tmp_dir" && mvn -o verify)
|
|
```
|
|
|
|
Run that only after Maven artifacts and approved model/runtime assets have been provisioned outside application startup. Record OS, architecture, Java, Maven, FFmpeg/ffprobe, Python, model checksums, command, duration, and result. A cache-dependent pass is not a reproducible-build proof.
|
|
|
|
### Gate 4: contract, data, security, and resilience
|
|
|
|
For affected changes, require all applicable evidence:
|
|
|
|
- Validate every public endpoint's success, validation, authentication, authorization, idempotency, bounded-collection, and failure cases against a versioned OpenAPI contract.
|
|
- Use RFC 9457 Problem Details without internal paths, exceptions, stack traces, storage keys, secrets, or sensitive payloads.
|
|
- Exercise Flyway migrations from an empty real PostgreSQL Testcontainer; verify constraints, indexes, transaction/concurrency behavior, backward-compatible rollout, backup/recovery, and roll-forward. H2 compatibility mode is not PostgreSQL evidence.
|
|
- Threat-model trust boundaries and apply server-side authorization, least privilege, restrictive CORS/headers, secret management, audit events, and sensitive-log redaction.
|
|
- Verify explicit timeouts, bounded safe retries, idempotency, partial failure, and resource limits for each network/process integration.
|
|
- Produce dependency, static, secret, container, and Software Bill of Materials (SBOM) evidence. Release with zero unresolved critical/high findings.
|
|
|
|
The current `pom.xml` has no Spring Security, OpenAPI, Testcontainers, ArchUnit/Spring Modulith, mutation testing, dependency lock, or security scanning integration. Do not mark this gate passed from intent or documentation.
|
|
|
|
### Gate 5: media and local-model evidence
|
|
|
|
For highlight, render, or asset changes, require more than structural output:
|
|
|
|
- Pin every local model and asset by immutable version/checksum and approved license; pre-provision it for macOS development and the target Linux/VPS/cloud runtime.
|
|
- Start and run model/media inference with all network access denied, including loopback RPC. Use in-process or approved non-network IPC. Missing or corrupt models must fail closed with a diagnosable, non-sensitive error.
|
|
- Validate highlight choices against versioned representative footage and annotations. Report precision/recall or an explicitly defined ranking metric, category slices, negative cases, and regressions.
|
|
- Probe actual output duration, codecs, streams, frame rate, resolution, black frames, freeze frames, clipping, integrated loudness, true peak, long silence, asset presence, and A/V synchronization.
|
|
- Verify voiceover intelligibility and timing, licensed music/SFX provenance, ducking, and that no silence/tone placeholder entered the final mix.
|
|
- Compare against a named baseline using predicted numeric outcomes and a blinded human rubric. Preserve manifests, commands, model versions, inputs or legal fixture references, measurements, and reviewer scores.
|
|
- Inspect each `qa-report.json` check's mechanism. Current duration/black/silence/sample-peak checks are measured; asset, mastering-filter, and overlay-bound checks remain structural, and the report is not a creative-quality certificate.
|
|
|
|
### Gate 6: operability and promotion
|
|
|
|
Promote the same immutable, platform-neutral application artifact across environments for a given release. Package it into signed, platform-specific runtime/model/image bundles for each supported OS/CPU target. Promote a bundle unchanged between environments with the same target; do not rebuild it per environment.
|
|
|
|
Require:
|
|
|
|
- typed validated external configuration; no embedded secret; safe disabled defaults; an environment diff;
|
|
- structured logs, bounded metrics, traces, secured health/readiness/liveness, business service-level indicators (SLIs), actionable alerts, and linked runbooks;
|
|
- non-root reproducible Open Container Initiative (OCI) image, vulnerability scan, read-only filesystem where practical, graceful shutdown, resource limits, and validated deployment definitions;
|
|
- measured workload with p50/p95/p99, throughput, errors, CPU, memory, GC, pools, threads, a 60-minute endurance run, downstream slowdown, and 2x expected peak behavior;
|
|
- commit-to-artifact provenance, SBOM, controlled approval, smoke/acceptance tests, and tested rollback or roll-forward;
|
|
- named owner, Recovery Time Objective (RTO), Recovery Point Objective (RPO), backup/restore, dependency-outage, migration-failure, and rollback procedures where applicable.
|
|
|
|
The user-provided default API targets of 200 requests/second/instance, p95 below 200 ms, p99 below 500 ms, error rate below 0.1%, no endurance memory growth, and graceful behavior at 2x peak are candidate baselines. Adjust them to the measured asynchronous video workload through an approved decision; do not claim they are currently met.
|
|
|
|
## Review the change
|
|
|
|
Require an independent reviewer for C2 and at least domain/architecture plus security/operations reviewers for C3. Add a creative-quality reviewer for highlight/render/audio changes and a data owner for persistence changes.
|
|
|
|
Review in this order:
|
|
|
|
1. Reproduce the original symptom or requirement.
|
|
2. Verify that one mechanism explains positive and negative observations.
|
|
3. Attempt an adversarial refutation: missing model, denied egress, corrupt asset, invalid plan, duplicate job, process timeout, disk pressure, restart, and approval absence as applicable.
|
|
4. Inspect the complete diff, configuration delta, schema/API delta, dependency tree, generated artifacts, and rollback.
|
|
5. Re-run the narrow test, then all required gates from a clean checkout.
|
|
6. Record residual risk and explicit approval; never infer approval from silence, a passing build, or an existing flag value.
|
|
|
|
Use the target human rubric as the final release scorecard: score architectural clarity, domain modeling, maintainability, security, tests, API, data, resilience, observability, performance, cloud operation, CI/CD, developer experience, docs, and operational readiness from 0 to 4 with concrete evidence. Block reference-architecture status if any category is below 3, the average is below 3.5, or security, data integrity, testing, or operational readiness is below 3.
|
|
|
|
## Promote or reject
|
|
|
|
Promote only when every applicable gate is green and the artifact is approved. The repository does **not** meet the production/reference-architecture definition as of 2026-07-21: the clean build is not reproducible, required security/API/architecture/CI/container/deployment gates are absent, and the production-quality local-model highlight flow has known fail-open behavior.
|
|
|
|
Reject or retain behind a disabled experimental flag when evidence is incomplete. Give every experimental flag an owner, safe default, expiry, removal criterion, and telemetry. Never change a production-facing default merely to make an experiment run.
|
|
|
|
For rollback:
|
|
|
|
- Prefer disabling a new path with an already-reviewed safe flag or rolling forward with a corrective artifact.
|
|
- Never edit an already released Flyway migration; write a new recovery migration and test it on production-shaped data.
|
|
- Preserve render manifests, audit records, failing inputs where legally permitted, and model/artifact checksums.
|
|
- Re-run smoke, integrity, and observability checks after rollback. A process restart alone is not rollback evidence.
|
|
|
|
## Historical rationale
|
|
|
|
Use these incidents to challenge recurring shortcuts; load `video-editing-failure-archaeology` for the full chronology.
|
|
|
|
| Incident | Lesson enforced here |
|
|
|---|---|
|
|
| Upload completion was initially a metadata flip; commit `7e8a214` routed it through object storage. | A state label must reflect the external side effect, not intention. Test the adapter boundary. |
|
|
| Generated clips were metadata-only until commit `66e998e` persisted real object locations. Signed URLs later derived fake paths until commit `7307082` used the persisted `objectKey`. | Verify artifact existence, identity, and retrieval end to end. Do not synthesize locations. |
|
|
| Worker files accumulated until commit `8f4804c` added cleanup in a `finally` block. | Test cleanup on success, failure, retry, and interruption; include disk saturation in operations review. |
|
|
| Internal storage locations leaked through API/events until commit `0d45f89` removed and tested them. | Treat paths, storage keys, and process output as boundary-sensitive data. |
|
|
| Retryable database-queue exceptions became terminal failures until commit `1737d8b` separated retry from DLQ exhaustion. | Model transient and terminal states separately; prove duplicate and retry behavior. |
|
|
| Commit `5d889b0` documented a renderer that worked but was not cinematic. Later work added category planning, validation, effects, QA, and approval. | Technical completion is not product quality. Predict and measure creative outcomes independently. |
|
|
| Commit `9536928` added approval to the local-director path; later highlight work initially defaulted to rendering without approval. The working tree now defaults highlight rendering off and approval on. | Apply policy to every workflow; similar names and flags do not imply equivalent gates. Keep tests on both property defaults. |
|
|
| Commit `97ba827` added automatic bootstrap and local generation with runtime installation/model-resolution and successful silence/tone fallbacks. The working tree removed asset-runtime bootstrap and placeholder success, but standalone bootstrap and the CV path remain network-capable. | “Local” is not the same as offline, provisioned, licensed, or fail-closed. Test absence and denied-egress paths. |
|
|
|
|
## Definition of done
|
|
|
|
Check every applicable item; do not weaken the list in a feature branch.
|
|
|
|
- [ ] Requirement, class, scope, affected environments, and eight-part decision record are explicit.
|
|
- [ ] No forbidden runtime download, external AI, unlicensed asset, placeholder, unapproved render, egress dependency, or silent default change remains.
|
|
- [ ] Core business rules remain independent of adapters and automated architecture tests enforce the intended boundaries.
|
|
- [ ] Clean offline one-command build passes on supported macOS and Linux from committed sources plus separately provisioned approved artifacts.
|
|
- [ ] Focused, full, architecture, API, persistence, integration, security, resilience, and end-to-end tests pass as applicable.
|
|
- [ ] API/schema/config compatibility, migration, idempotency, failure semantics, and rollback are verified.
|
|
- [ ] Dependency, static, secret, container, vulnerability, license, SBOM, signing/provenance, and reproducibility evidence passes as applicable.
|
|
- [ ] Logs, metrics, traces, health, SLI/dashboard/alert, capacity, shutdown, recovery, and runbooks are verified in production-like staging.
|
|
- [ ] Media/model output passes objective probes, offline/fail-closed tests, baseline comparison, representative/negative fixtures, and blinded human review.
|
|
- [ ] The immutable application artifact and target-specific signed bundle are approved and traceable from commit through controlled promotion without per-environment rebuild.
|
|
- [ ] Each target rubric category has linked evidence; no score is below 3 and the average is at least 3.5.
|
|
- [ ] Docs, OpenAPI, diagrams, configuration reference, threat model, testing/deployment/operations guides, and ADRs match implementation.
|
|
|
|
## Provenance and maintenance
|
|
|
|
This skill was grounded in repository source, configuration, tests, docs, and Git history on **2026-07-21**. Re-verify volatile facts before relying on them:
|
|
|
|
```bash
|
|
git log -12 --oneline --decorate
|
|
```
|
|
|
|
```bash
|
|
find src/test/java -name '*Test.java' | wc -l; awk -F'[,: ]+' '/Tests run:/{sum+=$3;fail+=$5;err+=$7;skip+=$9} END{print "tests="sum,"failures="fail,"errors="err,"skipped="skip}' target/surefire-reports/*.txt 2>/dev/null
|
|
```
|
|
|
|
```bash
|
|
for p in README.md mvnw .mvn .github Dockerfile compose.yml docker-compose.yml; do test -e "$p" && echo "present $p" || echo "absent $p"; done
|
|
```
|
|
|
|
```bash
|
|
rg -n 'enabled:|auto-start:|render-enabled:|require-director-approval:|fallback-to-heuristic:|strict-runtime:' src/main/resources/application*.yml
|
|
```
|
|
|
|
```bash
|
|
rg -n 'pip install|get_pretrained|write_silence|write_fallback_tone|local_asset_runtime_degraded' tools src/main
|
|
```
|
|
|
|
```bash
|
|
rg -n 'duration_matches_timeline|required_assets_resolved|text_overlays_safe|audio_mastering_applied' src/main/java/org/example/videoclips/editing/*Renderer.java
|
|
```
|
|
|
|
```bash
|
|
rg -n 'spring-security|springdoc|testcontainers|archunit|modulith|dependency-check|cyclonedx|pitest' pom.xml
|
|
```
|
|
|
|
```bash
|
|
mvn -o verify
|
|
```
|