Skip to content

feat(release): publish installable memd and reconcile /v1/version with client revision pins - #157

Closed
sun-970 wants to merge 1 commit into
bytefolk:mainfrom
sun-970:feat/release-server-artifacts-and-version-preflight-151
Closed

sun-970 wants to merge 1 commit into
bytefolk:mainfrom
sun-970:feat/release-server-artifacts-and-version-preflight-151

Conversation

@sun-970

@sun-970 sun-970 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Closes #151

Summary

  • REQ-001: Release workflow now builds and publishes server binaries (memd, mem-migrate, mem-healthcheck, mem) for darwin/arm64, darwin/amd64, linux/arm64, linux/amd64 alongside the existing mem-mcp builds, with per-group checksum manifests.
  • REQ-002: /v1/version now exposes three distinct fields — version (semver), revision (40-hex git commit), contract (durable-context wire contract) — so clients can pin a revision or accept a version range.
  • REQ-003: Both build paths (release workflow and Dockerfile) inject all three version coordinates at build time via -ldflags.
  • REQ-004: docs/DEPLOYMENT.md now documents the single version grammar and a first-run path that yields a reachable endpoint, workspace, token with write+recall scopes, and the corresponding durable-context grant.

Changes

File Change
server/internal/api/api.go Added Revision and ContractVersion vars; /v1/version returns {version, revision, contract}
server/internal/api/api_test.go Added TestVersionEndpointExposesAllCoordinates
server/cmd/memd/main.go Startup log includes revision and contract
server/cmd/mem/cmds_file.go CLI version display shows all three fields
server/Dockerfile Added REVISION and CONTRACT_VERSION build args
.github/workflows/release.yml New build-server job; release job handles 23 assets
scripts/generate_release_checksums.sh Generates both mem-mcp-checksums.txt and mem-checksums.txt
scripts/test_release_guards.sh Updated asset count, dependency check, asset list
scripts/test_release_helpers_compat.sh Updated asset list and manifest checks
docs/DEPLOYMENT.md Added version coordinate docs and first-run path
CHANGELOG.md Added [Unreleased] entries

Test plan

  • go test ./server/internal/api/... passes (new TestVersionEndpointExposesAllCoordinates)
  • scripts/test_release_guards.sh passes (updated asset inventory)
  • scripts/test_release_helpers_compat.sh passes (updated checksum fixtures)
  • Release workflow dry-run: verify build-server matrix produces 16 assets (4 binaries × 4 platforms)
  • Docker build: verify docker build --build-arg VERSION=0.1.1 --build-arg REVISION=$(git rev-parse HEAD) server produces a memd that answers /v1/version with all three fields

…h client revision pins

- Add server binaries (memd, mem-migrate, mem-healthcheck, mem) to the
  release workflow for darwin/arm64, darwin/amd64, linux/arm64, linux/amd64
  with per-group checksum manifests (mem#151 REQ-001).
- Expand /v1/version to expose version (semver), revision (40-hex git
  commit) and contract (durable-context wire contract) as distinct fields,
  so clients can pin a revision or accept a version range (REQ-002).
- Inject all three version coordinates at build time via ldflags in both
  the release workflow and the Dockerfile (REQ-003).
- Document the single version grammar and a first-run path that yields a
  reachable endpoint, workspace, token and durable-context grant (REQ-004).
@waterbro-8

Copy link
Copy Markdown
Collaborator

Local verification only — this is not a review approval or request-changes, not an acceptance, and carries no vote. Review ownership stays with @Bindy-lbb; the requirement record on #151 has humanReviewOwner: @Bindy-lbb, implementationOwner: @waterbro-8.

Why everything below was run by hand

CI has not executed on this head. All three check suites on 2813d2ad report status=completed conclusion=action_required, and statusCheckRollup is empty — so every box in the test plan is still unverified, including the two that would have failed loudly. Approval to run is someone with write access's call, not mine.

Environment: Go 1.25.0 linux/amd64, Node v24.19.0. Sources came from codeload tarballs (no git fetch here), so -buildvcs was exercised against a commit I created locally — the stamped revision in my outputs is that synthetic sha, not a real release commit. That affects the value of the sha only; the mechanism results below do not depend on it.


F1 — build-server fails on the first binary (blocks REQ-001 / AC-001)

.github/workflows/release.yml:203 puts -buildvcs=true inside -ldflags, so it is handed to the linker, not to go build. Running the job's own command verbatim:

$ CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -buildvcs=true -trimpath \
    -ldflags="-s -w -buildvcs=true -X .../api.Version=0.1.1 -X .../api.Revision=… -X .../api.ContractVersion=durable-context.v1" \
    -o /tmp/memd-linux-amd64 ./cmd/memd
# github.com/PeterGuy326/mem/server/cmd/memd
flag provided but not defined: -buildvcs
usage: link [options] main.o

The pre-existing mem-mcp job already has the right shape at :137go build -buildvcs=true -trimpath -ldflags='-s -w'. Removing -buildvcs=true from ldflags only (it stays as the build flag) makes all four targets build for linux/amd64 and cross to darwin/arm64, and both assertions then pass:

go version -m /tmp/memd-ok | grep -F "vcs.revision=…"   -> PASS
go version -m /tmp/memd-ok | grep -F 'vcs.modified=false' -> PASS

memd, mem-migrate, mem-healthcheck, mem all build clean.

F2 — mem-mcp-checksums.txt is no longer uploaded; this breaks npm install (blocks AC-001)

The change to the assets=( array replaced mem-mcp-checksums.txt with mem-checksums.txt instead of adding alongside it (:289-312). The manifest appears nowhere else in the workflow — grep -n checksums release.yml gives 312 (upload, mem-checksums.txt only) and 355 / 410 (both inside expected_assets).

Three-way contradiction inside one job:

count includes mem-mcp-checksums.txt
uploaded via gh release create "${assets[@]}" 23 no
expected_assets inventory 24 yes
jq '(.assets | length == 23)' 23

So the length assert passes while the name comparison fails and the job aborts. Independently, the consumer is live: npm/install.js:35 sets CHECKSUM_ASSET = "mem-mcp-checksums.txt" and :615-616 fetches https://github.com/bytefolk/mem/releases/download/v${version}/mem-mcp-checksums.txt. The published v0.1.1 release carries that asset today. Dropping it breaks the npm installer on every platform, which is outside this PR's stated scope.

F3 — REQ-002 is not reachable from this repo alone, and the new shape fails the pinned client on a new axis (blocks AC-002)

digital-employee@main packages/core/src/mem-http-memory-adapter.ts:449-457 gates with exactKeys(version, ["version"]), and exactKeys (:178-188) defaults allowed to required — so any extra key is a contract violation:

if (!record(version) || !exactKeys(version, ["version"]) ||
    version.version !== MEM_HTTP_PINNED_REVISION) {
  throw contractError("The mem server revision is not the pinned revision.");
}

I replayed that gate verbatim against the body actually served by a binary built from this head:

REJECT contract_unsupported: exactKeys(["version"]) false (keys=["contract","revision","version"])  <- PR#157 head, real body
REJECT contract_unsupported: version "0.1.1" !== pin                                                 <- main baseline (semver in `version`)
ACCEPT: preflight passes                                                                              <- control: single key, revision in `version`

Baseline and head both fail — but on different conditions, and the head's failure is not fixable by shipping better values: the control case shows the only response the current client accepts is the single-field grammar carrying a bare 40-hex revision in version, which is precisely what REQ-002 abolishes. MEM_HTTP_PINNED_REVISION = 4c714aa352f7… is a real mem commit (2026-08-23), nine days behind main at 10d4bf7a48fd.

AC-002 asks for "a client pinning either the revision or the version range passes preflight against that same artifact. A test asserts both directions." No such test is present — TestVersionEndpointExposesAllCoordinates asserts the server side only (and it does pass). I also found no open digital-employee PR touching that adapter. This requirement needs a coordinated change in the client repo, where neither this PR nor I has push.

F4 — the Docker/Compose path still can't produce a preflightable binary, and the new docs say it does (blocks REQ-003 / AC-003)

server/Dockerfile now defaults ARG REVISION=unknown, but nothing passes it: deploy/compose/compose.yaml:134 and :174 supply VERSION only, and no Helm file is touched by this diff. A Compose- or Helm-built memd therefore answers revision: "unknown".

The added docs/DEPLOYMENT.md text states the opposite — "the Compose and Helm deployment paths pass the release tag as VERSION and the tag commit as REVISION". Not true of this head. AC-003's "no step in the documented path requires hand-editing a build flag to become compatible" is not met: passing REVISION is the hand-edited build flag.

F5 — the REQ-004 first-run path can't be followed (blocks AC-004)

Checked against commands and routes as they exist on this head:

Documented step Observed
mem config get server Error: unknown command "config" for "mem" (from the binary built at this head; full subcommand list has no config)
"Run mem doctor" Error: unknown command "doctor" for "mem" — that's open PR #149, not merged
POST /v1/durable-context/grants with {"contract","principal","memory_ids":[…]} handlers_durable_context.go:141-144 decodes {principal, memory_id} singular under dec.DisallowUnknownFields()400 bad_body. Requires ScopeAdmin (api.go:371); step 3 mints read,write and no admin token is ever created
POST /v1/memories with {"content","tags"} needs an Idempotency-Key header → 400 missing_idempotency_key (handlers_memory.go:63-66), and tags is an unknown field against rememberRequest (:38-46) → 400 bad_body
POST /v1/durable-context/recall with {contract, principal} correct, ScopeRead (api.go:364)

One note on scope bookkeeping: #151 declares dependencies: [], yet this docs section reads as written against a stack of unmerged work — mem doctor exists only in open PRs #149/#131, and no open PR adds a mem config command at all (I checked all 18 open PRs in the repo). If those surfaces are real prerequisites, that belongs in the requirement record rather than in prose.

F6 — the PR's own release suite goes red on this head

$ bash scripts/test_release_guards.sh          # at 2813d2ad
ERROR: tampered asset: command unexpectedly succeeded
rc=1

$ bash scripts/test_release_guards.sh          # at main, same machine
PASS: release source, notes, asset-set and checksum guards fail closed
rc=0

A regression introduced here, not pre-existing. Cause: verify_manifest (scripts/test_release_guards.sh:25-31) checks mem-mcp-checksums.txt only, while the diff prepends the 16 server assets to assets=, so assets[0] is now memd-darwin-amd64 — tampering it is invisible to the manifest being verified, and the negative test "succeeds".

Applying just two edits locally (drop -buildvcs=true from ldflags; have verify_manifest check both manifests) takes both suites green:

test_release_guards.sh          rc=0  PASS
test_release_helpers_compat.sh  rc=0  PASS

Worth naming the general gap: these guards assert on workflow text (grep -Fq '(.assets | length == 23)', needs: [preflight, build-mcp, build-server]), so they pass while the command those lines describe cannot run. The one check that would have caught F1 is executing go build with the same ldflags — nothing here does that; only the pending CI would.

What does look solid

server/internal/api/api.go — the three vars and the doc comment are clean, go build ./... and go test ./internal/api/ are both green, and the new handler test passes. The cmds_file.go / main.go plumbing is consistent. The restructure of generate_release_checksums.sh is a genuine improvement — verifying both manifests with --strict and dropping --ignore-missing tightens it, and the all_assets[${#all_assets[@]}] form keeps Bash-3 compatibility, which test_release_helpers_compat.sh exists to protect. The CHANGELOG placement is right.

Suggested split

REQ-001/REQ-003 can land in this repo once F1, F2, F4 and F6 are fixed. REQ-002 and the preflight half of AC-002 need a paired digital-employee change to exactKeys/the pin in preflight(), and AC-004 needs live evidence from a release artifact — neither is obtainable by editing mem alone, which is what #151's own "Open question" section already says.

I pushed nothing and changed nothing on this PR. Trees are at ~/pr157/{head,base,fixtest} on my machine if anyone wants the repro re-run against a new head.

@waterbro-8

Copy link
Copy Markdown
Collaborator

Closing under the fork-workflow decision recorded on 2026-09-03: repository
automation is not being enabled for fork pull requests, so a fork head cannot
carry a CI result, and every acceptance gate in this repository is written
against checks that ran. Nothing in this comment is a judgment that the work is
wrong; where it is right, it is re-landed on an organization branch instead.

#151.

The blocking problem is procedural and was recorded on this thread on
2026-09-02: this head has no checks at all. All three suites report
conclusion=action_required and statusCheckRollup is empty, because a first
time contributor's workflows need a maintainer approval that was not given, so
none of the boxes in its test plan has an executed result behind it. #151 is a
release-path change; approving a run to make it reviewable is a decision for a
maintainer, and the fork policy now says repository automation will not be
enabled for fork heads.

The substantive findings from that review stand and belong to #151 rather than
to this branch, so re-landing #151 means carrying them:

  • -buildvcs=true inside -ldflags is passed to the linker, which fails with
    flag provided but not defined: -buildvcs. release.yml:137 shows the
    working spelling.
  • the asset list is replaced rather than extended, so
    mem-mcp-checksums.txt disappears from uploads while npm/install.js:615
    still downloads exactly that name — the next release breaks every platform.
    Three places in the same workflow also disagree about the count: 23 uploaded,
    24 in expected_assets, a jq assertion of 23.
  • scripts/test_release_guards.sh then fails on this head for a reason that is
    its own bug: verify_manifest validates only the checksums asset, and the new
    array orders the server assets first, so tampering with assets[0] passes.
  • adding a field to /v1/version cannot be done in this repository alone:
    digital-employee's mem-http-memory-adapter.ts:449 rejects a version body
    with any unexpected key via exactKeys.

And the new onboarding text in this branch documents mem config get server and
mem doctor, neither of which exists on this head.

The commits are not lost. A closed fork PR keeps its head ref:

git fetch https://github.com/bytefolk/mem.git refs/pull/157/head:pr-157

Every file in this branch was therefore available to the re-doing work, whether
or not it was used.

@waterbro-8 waterbro-8 closed this Sep 3, 2026
waterbro-8 added a commit that referenced this pull request Sep 18, 2026
…) (#216)

## Requirement and scope

Re-lands #157 onto current `main` as an organization branch. #157 was
closed because the fork head could not carry CI, not because the work
was rejected.

Refs #151. Implementation owner on the issue is @waterbro-8; human
review owner is @Bindy-lbb. Original implementation: @sun-970.

## Changes

- Release workflow builds `memd`, `mem-migrate`, `mem-healthcheck`, and
`mem` for linux/darwin amd64/arm64 in addition to `mem-mcp`.
- Two checksum manifests: `mem-mcp-checksums.txt` (6) and
`mem-checksums.txt` (16). The post-#199 output-path protections (no
process-substitution find, no `wc -l`, refuse existing/late output
symlinks, private mktemp staging) are kept and applied to both
manifests.
- `/v1/version` exposes distinct `version` / `revision` / `contract`
fields; CLI `mem version` prints them and still redacts the server URL.
- Docker image and DEPLOYMENT.md first-run notes from #157.

## Validation ledger

| ID | Criterion | Command | Status |
| --- | --- | --- | --- |
| V1 | Checksum output safety | `bash
scripts/test_release_checksum_output_safety.sh` | PASS |
| V2 | Release guards | `bash scripts/test_release_guards.sh` | PASS |
| V3 | Bash 3.2 helper compat | `bash
scripts/test_release_helpers_compat.sh` | PASS |
| V4 | `git diff --check` | local | PASS |
| V5 | Go API/CLI tests | `make test-server` | NOT VERIFIED — host Go is
1.22.12, module requires 1.25.0 |
| V6 | Built release binaries on six platforms | release workflow | NOT
VERIFIED — no tag cut |
| V7 | Client revision pin against a published memd | needs published
artifact | NOT VERIFIED |

No tag, GitHub Release, npm publish, merge, or issue closure is
performed by this PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(release): publish an installable memd artifact and reconcile /v1/version with client revision pins

2 participants