feat(release): server-side packaging — attach full-repo + entry_point subtree zips on release [#809] #814

Merged

jmiller merged 2 commits from feature/release-packaging-hook into dev 2026-07-19 02:49:45 +00:00

Conversation 1 Commits 2 Files Changed 2

+270 -4

jmiller commented 2026-07-18 21:09:11 +00:00

Owner

Copy Link

Copy Source

Summary

Adds a server-side on-release packaging hook (#809). On publish, MokoGIT now
attaches build artifacts to the release automatically:

• <repo>-<tag>.zip — full-repository archive (always).
• <repo>-<tag>-source.zip — entry-point subtree archive, generated only
  when the repo metadata (repo_manifest.entry_point) declares a non-root
  entry point (""/"."/"/" are treated as root and skipped).

How it works

• New services/release/packaging.go → GenerateReleaseArtifacts(ctx, rel).
• Reads the entry point via repo_model.GetRepoMetadata(ctx, rel.RepoID).
• Archives each target with gitrepo.CreateArchive(ctx, rel.Repo, "zip", w, false, rel.Sha1, paths)
  (full repo = paths == nil; subtree = []string{cleanedEntryPoint}), streaming
  the zip through an io.Pipe straight into attachment_service.NewAttachment
  with ReleaseID = rel.ID and size = -1 — the archive is never buffered in RAM.
• Idempotent: existing attachments with the same target names are deleted
  before re-attaching, so a draft→publish transition (or any re-run) replaces
  rather than duplicates.
• Runs before GenerateReleaseChecksums, so each generated zip receives a
  .sha256 sidecar automatically from the existing checksum pass.
• Error handling mirrors checksum.go: per-artifact failures are logged and do
  not abort the release; only setup failures (metadata/attachment enumeration)
  are returned.

Call sites (services/release/release.go)

• CreateRelease — invoked before the checksum call, guarded by !rel.IsDraft.
• UpdateRelease — invoked in the publish path before the checksum call, guarded
  by !rel.IsDraft (drafts skipped, matching the notify_service guard).

Both checksum guards were widened to also fire when artifacts are generated
(|| !rel.IsDraft) so the new zips get sidecars even when no user attachments
were uploaded.

Refs #809, #812, EPIC #367.

---

Not built/deployed by the author — requires a forge rebuild + redeploy. No
go build/go vet was run (Go toolchain unavailable in the authoring
environment); signatures were matched exactly against the source in this
branch. Please review the gitrepo.CreateArchive and
attachment_service.NewAttachment signatures against the target forge version
before merging.

## Summary Adds a server-side on-release packaging hook (#809). On publish, MokoGIT now attaches build artifacts to the release automatically: - **`<repo>-<tag>.zip`** — full-repository archive (always). - **`<repo>-<tag>-source.zip`** — entry-point subtree archive, generated only when the repo metadata (`repo_manifest.entry_point`) declares a non-root entry point (`""`/`"."`/`"/"` are treated as root and skipped). ### How it works - New `services/release/packaging.go` → `GenerateReleaseArtifacts(ctx, rel)`. - Reads the entry point via `repo_model.GetRepoMetadata(ctx, rel.RepoID)`. - Archives each target with `gitrepo.CreateArchive(ctx, rel.Repo, "zip", w, false, rel.Sha1, paths)` (full repo = `paths == nil`; subtree = `[]string{cleanedEntryPoint}`), streaming the zip through an `io.Pipe` straight into `attachment_service.NewAttachment` with `ReleaseID = rel.ID` and `size = -1` — the archive is never buffered in RAM. - **Idempotent:** existing attachments with the same target names are deleted before re-attaching, so a draft→publish transition (or any re-run) replaces rather than duplicates. - Runs **before** `GenerateReleaseChecksums`, so each generated zip receives a `.sha256` sidecar automatically from the existing checksum pass. - Error handling mirrors `checksum.go`: per-artifact failures are logged and do not abort the release; only setup failures (metadata/attachment enumeration) are returned. ### Call sites (`services/release/release.go`) - `CreateRelease` — invoked before the checksum call, guarded by `!rel.IsDraft`. - `UpdateRelease` — invoked in the publish path before the checksum call, guarded by `!rel.IsDraft` (drafts skipped, matching the `notify_service` guard). Both checksum guards were widened to also fire when artifacts are generated (`|| !rel.IsDraft`) so the new zips get sidecars even when no user attachments were uploaded. Refs #809, #812, EPIC #367. --- > **Not built/deployed by the author** — requires a forge rebuild + redeploy. No > `go build`/`go vet` was run (Go toolchain unavailable in the authoring > environment); signatures were matched exactly against the source in this > branch. **Please review the `gitrepo.CreateArchive` and > `attachment_service.NewAttachment` signatures against the target forge version > before merging.**

jmiller added 1 commit 2026-07-18 21:09:15 +00:00

feat(release): server-side packaging — attach full-repo + entry_point subtree zips on release [#809] ... 2280f02bec

Add GenerateReleaseArtifacts in services/release/packaging.go, invoked from
CreateRelease and the UpdateRelease publish path (drafts skipped). It streams a
full-repository zip (<repo>-<tag>.zip) and, when repo metadata declares a
non-root entry_point, an entry_point subtree zip (<repo>-<tag>-source.zip) via
gitrepo.CreateArchive through an io.Pipe into attachment_service.NewAttachment,
so archives are never buffered in memory. The helper is idempotent (existing
artifacts of the same name are replaced) and runs before GenerateReleaseChecksums
so each zip receives a .sha256 sidecar automatically.

Refs #809 #812 EPIC #367

Authored-by: Moko Consulting

jmiller added 1 commit 2026-07-18 21:39:55 +00:00

fix(release): mark generated artifacts, serialize + reorder attach to prevent data loss [#809] ... 19d498f6c3

Address data-safety review of the server-side packaging hook:

- DATA LOSS (HIGH): tag generated zips with UploaderID = user_model.ActionsUserID
  (-2), a built-in bot sentinel that cannot collide with human uploads (which
  always use a positive doer ID). Only attachments carrying this marker are ever
  deleted, so a user-uploaded asset sharing a generated name is never destroyed.

- CONCURRENCY (HIGH): serialize the per-release delete+attach sequence with
  globallock.LockAndDo, keyed release_packaging_<id>, matching the forge's
  existing working-lock convention. Attachments are re-read inside the lock.

- DELETE-THEN-FAIL (LOW): reorder to attach-new-then-delete-old — the fresh
  archive is stored under the canonical name first and stale generated copies
  are removed only on success, so a failed regeneration never nets asset loss.

- FILENAME (LOW): sanitize the tag segment (replace path separators) so a tag
  like release/1.0 yields a clean asset name.

- ENTRY_POINT (LOW): normalizeEntryPoint trims a leading slash (git archive
  rejects absolute paths) and treats a now-empty result as root/skip.

- CHECKSUM re-hash cost (MEDIUM): documented as a follow-up at the checksum
  call sites in release.go.

The verified-correct io.Pipe streaming is unchanged.

Refs #809 #812 EPIC #367

Authored-by: Moko Consulting

jmiller commented 2026-07-18 21:51:31 +00:00

Author

Owner

Copy Link

Copy Source

Logic review + data-safety fixes

A read-only logic review (no compiler available here) found the io.Pipe/goroutine handling correct (no leak — pr.CloseWithError on the early-return path unblocks the exec copy goroutine and reaps git archive) and size=-1 streaming valid on both Local and Minio backends. It also found two HIGH data-safety bugs and some LOWs, all now fixed in 19d498f:

Sev	Issue	Fix
HIGH	deleteExistingArtifacts matched by name only -> could delete a user-uploaded asset sharing the generated name	Generated artifacts marked with UploaderID = ActionsUserID (-2); delete only matches name AND marker. Human uploads use positive Doer.ID; legacy uploads are 0, never negative -> collision-free
HIGH	generation runs outside a tx -> concurrent Create/UpdateRelease can duplicate or delete-then-lose a zip	Per-release globallock.LockAndDo around delete+attach
LOW	delete-then-fail left net asset loss on transient error	reordered to attach-new-then-delete-old on success
LOW	raw tag used verbatim in asset name (repo-release/1.0.zip)	sanitizeNameSegment (/,\,NUL -> -), matching GetArchiveName()
LOW	leading-slash entry_point (/packages/x) rejected by git archive	normalized (trim leading /, empty -> root/skip)

MEDIUM (documented follow-up, not fixed): a non-draft release edit re-hashes all attachments via the widened checksum guard — O(total asset bytes) on unrelated edits.

Still required before merge: this was authored WITHOUT a Go toolchain — gofmt/go vet/go build did not run. Compile-check + review the archive/attachment signatures against the deployed forge version, then rebuild + redeploy the forge. Not built, not deployed.

Authored-by: Moko Consulting

## Logic review + data-safety fixes A read-only logic review (no compiler available here) found the io.Pipe/goroutine handling **correct** (no leak — `pr.CloseWithError` on the early-return path unblocks the exec copy goroutine and reaps `git archive`) and `size=-1` streaming valid on both Local and Minio backends. It also found two HIGH data-safety bugs and some LOWs, all now fixed in `19d498f`: | Sev | Issue | Fix | |---|---|---| | HIGH | `deleteExistingArtifacts` matched by name only -> could delete a user-uploaded asset sharing the generated name | Generated artifacts marked with `UploaderID = ActionsUserID (-2)`; delete only matches name **AND** marker. Human uploads use positive `Doer.ID`; legacy uploads are `0`, never negative -> collision-free | | HIGH | generation runs outside a tx -> concurrent Create/UpdateRelease can duplicate or delete-then-lose a zip | Per-release `globallock.LockAndDo` around delete+attach | | LOW | delete-then-fail left net asset loss on transient error | reordered to attach-new-then-delete-old on success | | LOW | raw tag used verbatim in asset name (`repo-release/1.0.zip`) | `sanitizeNameSegment` (`/`,`\`,NUL -> `-`), matching `GetArchiveName()` | | LOW | leading-slash `entry_point` (`/packages/x`) rejected by git archive | normalized (trim leading `/`, empty -> root/skip) | MEDIUM (documented follow-up, not fixed): a non-draft release edit re-hashes all attachments via the widened checksum guard — O(total asset bytes) on unrelated edits. **Still required before merge:** this was authored WITHOUT a Go toolchain — `gofmt`/`go vet`/`go build` did not run. Compile-check + review the archive/attachment signatures against the deployed forge version, then rebuild + redeploy the forge. Not built, not deployed. Authored-by: Moko Consulting

jmiller merged commit df67ff07b8 into dev 2026-07-19 02:49:45 +00:00

jmiller deleted branch feature/release-packaging-hook 2026-07-19 02:49:46 +00:00

jmiller referenced this pull request 2026-07-19 03:01:33 +00:00

release: npm/mcp metadata + platform registry + update-server + packaging hook (dev -> main) [#367] #828

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

breaking-change

Breaking API or behavior change

ci-cd

CI/CD pipeline changes

config

Configuration changes

dependencies

Dependency updates

deploy-failure

Deployment failed

docker

Docker/container changes

documentation

Documentation changes

feature: wiki

Wiki system enhancements

good first issue

Good for newcomers

health-check

Repo health check result

help wanted

Extra attention needed

mokostandards

Related to MokoStandards framework

push-failure

Git push operation failed

security

Security vulnerability or hardening

size/l

200-500 lines changed

size/m

50-200 lines changed

size/s

10-50 lines changed

size/xl

500+ lines changed

size/xs

< 10 lines changed

standards-drift

Deviates from MokoStandards

standards-update

MokoStandards compliance update

sync-failure

Sync or mirror failed

tech-debt

Technical debt and TODO/FIXME items

upstream

upstream

Inherited from upstream Gitea

work-in-progress

Draft or incomplete work

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No labels

Priority -

Type -

Milestone

No items

No Milestone

Assignees

Clear assignees

jmiller (Jonathan Miller) moko-deploy (Moko Deploy Bot)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MokoConsulting/MokoGIT#814