docs(deploy): two-phase deploy, revert, by-reference deploys, and OIDC trusted publishing - #599
Draft
dawsontoth wants to merge 20 commits into
Draft
docs(deploy): two-phase deploy, revert, by-reference deploys, and OIDC trusted publishing#599dawsontoth wants to merge 20 commits into
dawsontoth wants to merge 20 commits into
Conversation
…ivate CLI Document the two-phase deploy behavior and the new surface from HarperFast/harper#1849: - deploy_component: cluster-wide stage -> barrier -> activate; new properties activate:false (stage-and-stop, returns a staged deployment_id), deployment_id (activate a previously-staged deployment), revert_on_failure, two_phase, and the ignore_replication_errors / deployment_timeout knobs. - revert_component: fast cluster-wide rollback to the retained previous version. - Deployment lifecycle: stage/activate phase names, staged/staging/activating/ rolled_back statuses, and staged-build retention (deployment_stagingRetention_maxCount). - CLI: harper stage / activate / revert verbs (aliases + examples). Companion to HarperFast/harper#1849 (still a draft). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for the CLI and Operations API to reflect the new two-phase deployment process (staging and activation) and the rollback mechanism (revert_component). It introduces new CLI commands, aliases, API parameters, and examples. The review feedback suggests improving consistency by pointing the new CLI operation category links to the main #components section, and enhancing readability in the API documentation by adding clear labels to the multi-step "Stage now, activate later" JSON examples.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
…tage/activate example Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
dawsontoth
marked this pull request as ready for review
July 29, 2026 17:53
Conflict in reference/operations-api/operations.md was additive: main added the `credentials` parameter plus its "Deploy credentials" section (#581) where this branch adds the two-phase parameters. Kept both — the parameter list now carries activate/deployment_id/revert_on_failure/ignore_replication_errors/ deployment_timeout/two_phase AND credentials, followed by main's credentials section. main's get_deployment_payload / delete_deployment_payload docs (#600) are untouched.
… values Documents harper-pro#594 and syncs with harper#1849's latest behavior. add_ssh_key `generate: true` (harper-pro#594): - Harper mints the ed25519 keypair on the node and returns only `public_key`, so the private key is never carried in a request body, shell history, or CI log. Includes the response shape and the `harper:<name>` comment. - `key` and `generate` are mutually exclusive; ssh-keygen must be on PATH. - Notes that `public_key` is returned ONLY on the generating call — Harper does not retain it, and update_ssh_key requires a key you supply (it cannot mint one), so the recovery path is delete_ssh_key + add_ssh_key generate again. - CLI example added alongside the deploy/stage/activate/revert examples. Payload retention (harper#1849): - Splits the old one-line aside into its own subsection with a table covering both bounds: payloadRetention_maxSize (10 MiB, bounds a single payload) and the new payloadRetention_maxCount (default 1, bounds how many are kept per project). Explains why the default is conservative, that rows are never deleted, and that a reclaimed payload reports payload_blob_present: false. Also: - revert_on_failure / ignore_replication_errors now note they apply to a `deployment_id` activate too, matching the peer-failure gate added to that path. - list_deployments `status` listed only 5 of 13 real values; replaced with terminal vs in-flight groupings. - New note on restartRequired: deploying a brand-new component without a restart marks one as required (per node), while redeploying a live component does not.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
…cument them The SSH rows linked to #certificate-management, but the SSH operations are documented under ## Components — so every one of those seven links landed the reader in the wrong section. Repointed them to #components (the convention the other 15 component rows already use). Auditing the same block turned up the identical defect on the six user/role rows (list_users, add_user, alter_user, drop_user, list_roles, drop_role), which are documented under ## Users & Roles. Repointed those to #users--roles, matching the existing link to that section elsewhere in the docs. The five genuine certificate rows (create_csr, sign_certificate, list_certificates, add_certificate, remove_certificate) were already correct and are unchanged. Category labels updated to match their new targets. Verified every operations.md anchor referenced from this table against the real headings — all 11 resolve.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
This was referenced Jul 29, 2026
Open
Merged
Builds on the two-phase deploy docs (#599) to cover the rest of the deploy-by-reference workflow. reference/components/applications.md - "Deploying by Reference" — `harper deploy by_ref=true`, `ref=`, `credential=`. Explains why a reference pins to a resolved SHA rather than the tag or branch name typed: peers resolve the package independently, so a tag that moves mid-deploy could otherwise leave nodes running different code. Notes that the cluster clones the pushed remote, and that by-reference means the cluster builds from source — so an app whose build can't run on the node should stay on payload deploys. - "Provisioning a Deploy Credential" — `harper deploy setup=true`, the client-side sealing flow, and that reverting to the previous version needs no credential at all. reference/cli/authentication.md - Documents HARPER_CLI_REFRESH_TOKEN / HARPER_CLI_OPERATION_TOKEN and adds "Token credentials for CI/CD", so the CI guidance is no longer "put an admin password in your pipeline". Covers precedence, in-memory-only refresh, and token lifetimes (1d / 30d defaults). - Warns that a user holds only ONE valid refresh token at a time: issuing a new one invalidates the last, so a routine local `harper login` silently breaks a pipeline using the same account. Hence the recommendation to give CI its own user. (Verified against security/tokenAuthentication.ts, which stores a single hashed refresh_token per user record.) reference/cli/commands.md - `harper login --for-ci`, including the stdout/stderr split that makes `| gh secret set --env-file -` work without displaying the token. reference/security/secrets.md - Points the existing private-source deploy credentials section at the CLI flow that automates it. Documents HarperFast/harper#1850, #1851, and #1876. Verified every anchor link in the changed files resolves (16 checked). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Opened #616 stacked on this branch: docs(deploy): by-reference deploys, sealed credentials, CI token auth. It targets Where the two meet: this PR documents the two-phase deploy and |
cb1kenobi
reviewed
Jul 30, 2026
…vert caveat Addresses cb1kenobi's review on #599. - "Terminal" wrongly included `staged`. The implementation's TERMINAL_STATUSES is {success, failed, rolled_back} and that set gates get_deployment_payload and payload-pruning eligibility, so listing `staged` as terminal contradicted the get_deployment_payload section. Split into Terminal / Resting / In flight, and noted why a resting payload is deliberately still held. - get_deployment's `status` row listed 7 of 13 values; now lists all, pointing at the grouped explanation. - The `restart: "rolling"` example's response showed the no-restart message. Corrected to the rolling shape (restartJobId + ", restarting Harper") and noted what the no-restart response looks like instead. - revert_component: added a caution that reverting swaps live directories and does NOT rewrite the stored `package:` reference, so a node provisioned after a revert installs the reverted-away version. Verified in code: revertComponent never calls writeComponentRootConfig, and installApplications() installs from root config. - deployment_id activate: documents that the staged package identifier and credential references are recovered and persisted at activation (fixed in harper#1849), so `package` need not be repeated on the activate call.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
dawsontoth
marked this pull request as draft
August 12, 2026 16:29
…evel Brings the revert documentation in line with HarperFast/harper#1849 after review. The behavior changed in three ways that each invalidated something documented here. - `revert_on_failure` is gone — it is now rejected outright, so the bullet is removed. Replaced with what actually happens on a partial activation (the deployment stays `activating`; recover by rolling forward or by an explicit revert) and why there is no automatic rollback: past the activation barrier, a peer reporting failure does not prove it did not activate, so auto-reverting the failed nodes can split the cluster three ways instead of converging it. - `revert_component` now takes a **required** `to_deployment_id` and is idempotent rather than a bidirectional toggle. Documented as such, with the reason the target is mandatory (a retry after a lost response must not flip the rejected release back in), a parameter table, the response fields including `from_deployment_id`/`rollback_of`, and the one-activation reach of retention. - The `:::caution` saying a revert is not a config-level rollback is **no longer true** and is replaced by the opposite: the revert rewrites the stored `package:` reference and the boot-time application lock as part of the same operation, so a newly joined peer or a rebuilt components directory installs the version the cluster is actually running. Reverting away from a `package` deploy to a payload-deployed version removes the reference entirely. CLI: `harper revert` now requires `to_deployment_id`, and the example and command table say so. The wider CLI table diff is Prettier realigning column widths after the description text changed. Version badges still carry the placeholder release, per this PR's existing note.
Contributor
Author
|
Updated in d000da2 to match the current shape of HarperFast/harper#1849 after review. Three things documented here had become wrong:
Still holding merge until harper#1849 lands, and version badges still carry the placeholder release. |
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
Restacks this branch on #630, which carries the `harper login --for-ci` / `HARPER_CLI_REFRESH_TOKEN` content that used to live here. That content documents behavior shipped in v5.2.0, so it no longer belongs behind this branch's four unmerged upstream dependencies. Three conflicts, resolved as follows. `reference/cli/authentication.md` and `reference/cli/commands.md` — took #630's side wholesale. Everything this branch had added to those two files was the token content that moved, and #630 states it against main's current structure rather than the structure this branch forked from. That also settles the duplicate-precedence hazard @Ethan-Arrowood raised: main grew a canonical numbered Authentication Precedence section after this branch forked, and the token rules are now folded into that list instead of restated beside it. Merging this branch no longer gives the precedence order twice, differently. `reference/cli/operations-api-commands.md` — hand-merged, because neither side was correct alone. Taking ours would have silently dropped the seven backup rows main added since the fork (`create_backup`, `list_backups`, `verify_backup`, `delete_backup`, `purge_backups`, `restore_backup`, `get_backup`); taking theirs would have dropped this branch's four deploy rows and its recategorization. The merged table has all three sets: - main's seven backup rows, unchanged. - This branch's `stage` / `activate` / `revert_component` / `revert` rows. - This branch's recategorization: the six user/role rows point at `#users--roles` and the seven SSH rows at `#components`, instead of the `#certificate-management` anchor that documented neither. The five genuine certificate rows still point there. Compared both sides row by row before resolving: no description text differed, so the column-width churn is only Prettier re-padding. `operations.md` and `applications.md` auto-merged; main's new `host` badge survives. Build clean. The two broken anchors it reports are pre-existing on main (confirmed by building main with these changes stashed) and are in files this branch does not touch.
The v5.2.0 placeholder this branch has carried since July is wrong, and now demonstrably so rather than provisionally. harper `main` is at **v5.2.2** (tagged 2026-08-13) and by_ref merged 2026-08-14, after that tag and in no release. The v5.3.0 upgrade directive on harper#2173 confirms which release is next. So none of this surface can ship in a 5.2.x patch: | Surface | Upstream | Ships in | | --- | --- | --- | | two-phase deploy, `revert_component`, staging retention | harper#1849 | v5.3.0 | | `deploy by_ref=true` | harper#1850 (merged, untagged) | v5.3.0 | | `deploy setup=true` | harper#1851 | v5.3.0 | | `add_ssh_key generate: true` | harper-pro#594 | v5.3.0 | The one piece that genuinely shipped in v5.2.0 — `login --for-ci` and the token env vars (harper#1876) — left this branch for #630, so nothing here is v5.2.0 any more. Addresses @kriszyp's two versioning findings. Both plain-text `Available since` lines are now `<VersionBadge>`, standalone under their headings per CONTRIBUTING.md, and the API reference no longer reads as though these behaviors exist throughout v5: - `deploy_component` — changed badge; it is existing surface whose cluster behavior changed. - `revert_component` — added badge, and the row it was missing from the Components summary table. - `add_ssh_key`'s `generate` subsection — added badge. Release-notes coverage, the other half of those findings, follows in a separate commit once the OIDC surface is in.
@kriszyp's eight findings, re-checked against the current state of harper#1849 and harper-pro#594 rather than the state they were filed against. Four had been fixed at the source in the meantime; four needed the docs changed. **"All-or-nothing at go-live" was wrong, and self-contradictory.** The page claimed it in the opening paragraph and then correctly described a version split twenty lines later. Reframed around what the two phases actually buy: an all-nodes staging barrier, which eliminates the common failure (a node that can't fetch or install) but not a per-node activation failure. The activation-failure paragraph is now its own `#### Activation failures` section instead of sitting mid-parameter-list, and the opening paragraph links to it. **The deploy modes are now documented as a matrix**, which core made possible by adding the validation @kriszyp asked for. `activate: false` and `deployment_id` are rejected outright when combined with `two_phase: false`, `replicated: false`, or a cluster whose `system` database isn't replicated, so a request that says stage-only can no longer activate. `two_phase: true` is likewise rejected without the replication to support it. `revert_on_failure` is refused rather than accepted and ignored. **The `system`-replication precondition is stated** — @Ethan-Arrowood's second question. A plain deploy on a cluster with `system` excluded from replication still silently takes the one-shot path, and that is the case worth warning about, because it also means no rollback target. **`revert_component`'s guarantees are scoped to the two-phase path.** The retained `.deploy-previous` copy is created by activation, so a component deployed one-shot has nothing to revert to however many times it has been deployed — the docs implied repeated deploys build up a rollback target. Also documents that `restart` is optional (a reverted component keeps serving the rolled-away-from code until something restarts it) and adds the `force` parameter the validator accepts. **The SSH-key encryption guarantee is qualified.** `sealSSHKey` in harper-pro still falls back to storing and replicating the private key in plaintext with only a WARN when no secret custody is registered, and that fallback is deliberate — SSH keys predate custody. So encryption at rest is a property of the deployment's configuration, not of the operation, and `generate: true` in particular reads as though the key could never be exposed. Custody is present by default, so the warning says that too rather than implying the risk is likely. Fixed at the source, no doc change needed: - **Explicit refs now resolve to a SHA** (harper 5c85714b8, 26cce986d, 21043f994) — the guarantee the page claimed is now real. The fuller treatment, including remote resolution and failing closed, is in #617. - **`auth_*` vs. ordinary `username=` precedence** — `main` grew a canonical numbered precedence section covering exactly this, and #630 folds the token rules into it. - **Version badges** — previous commit.
…ed credential Companion to HarperFast/harper#2173, which had no docs PR. Four operations, a CLI credential source, and a workflow shape. `reference/operations-api/operations.md` — a new **OIDC Trusted Publishing** subsection under Token Authentication, since a trust policy is an authentication mechanism rather than a component or security-config concern: - `add_oidc_trust` / `list_oidc_trust` / `drop_oidc_trust` — super_user only, with the full parameter table, and the replace-not-merge semantics (a partial update to a claim set is how an over-broad policy gets created by accident). - `exchange_oidc_token` — unauthenticated by design, one-hour token, no refresh token issued. The parts a reader has to know and cannot guess: - **Policy specificity for GitHub Actions**, as a table of the three structural requirements — pin the repository, pin the workflow, gate the ref — with what each leaves open when unsatisfied. The ref gate is stricter than npm's model and is the rule most likely to block a real workflow, so it gets the reason: without it, anyone who can push a branch can add the trusted workflow to that branch and mint a token. - The consequences that follow: a tag-triggered release can't pin `workflow_ref` and must pin `workflow_path` plus an environment; `ref_type: tag` is not a gate; `sub` is not a valid pin for GitHub and *is* the required pin for everyone else; `pull_request_target` is denied unless the policy constrains `event_name`. - **Audience must be instance-specific**, and why `https://github.com/<owner>` is refused. - **A constrained claim absent from the token fails**, so a policy can't be weakened by an issuer that stops emitting a claim. - **Single-use tokens**, recorded as a hash before minting, and the honest account of the cross-node replay race and why it is tolerable. - **One rejection message for every failure**, with the reason in the `oidc-trust` log — otherwise the docs would send readers hunting for a specific error the endpoint will never give them. `reference/cli/authentication.md` — the exchange as precedence entry 8, plus a **Workload identity (OIDC)** section under Method 2. The emphasis is that it ranks *last*: adding `id-token: write` to a workflow that still sets `HARPER_CLI_REFRESH_TOKEN` does not change which identity deploys, which is a deliberate property someone migrating needs to know. Also covers what the CLI prints, and that an unrecognized runtime falls through silently rather than failing. Verified against the branch as it stands, not the PR description, which has drifted in three places: the code is now issuer-agnostic with GitHub as a profile (so a generic-issuer path is documented), replay keys on a token hash rather than `issuer|jti`, and the auth-audit-log gap listed as not-done is implemented.
…rface Completes @kriszyp's discoverability findings. The reference pages carry version badges now, but a user who only reads release notes had no way to find any of this — and there was no 5.3 page at all. The directory is autogenerated into the sidebar, so the new page needs no registration. Covers, in the house style of 5.2.md: two-phase cluster deploys, stage-now/activate-later, component rollback, deploying by git reference, sealed deploy credentials, OIDC trusted publishing, and server-side SSH key generation. Each entry states the limits alongside the capability rather than leaving them to the reference page, because these are the ones that change whether a reader should adopt the feature: the two-phase barrier is not an all-or-nothing go-live, it silently degrades on a cluster whose `system` database isn't replicated, rollback reaches back exactly one two-phase activation, and the GitHub Actions ref-gate rule will refuse policy shapes that npm accepts. `login --for-ci` and the token env vars are not here — they shipped in v5.2.0 and belong in 5.2.md, which is #630's.
Three things this section described had changed upstream since it was written. **The recommended token is now a fine-grained PAT, not the `gh` session token.** Review on #1851 pointed out that offering the `gh` token first made it the default selection — putting least privilege on the path of most resistance, next to guidance recommending Contents: Read-only on one repo. The prompt now leads with the PAT and warns when `gh` is chosen. The docs listed both neutrally, `gh` first, so they now lead with the PAT and say why it matters here specifically: what this flow seals is durable and replayed on every cold deploy and rollback. **It is two calls, not one.** `set_secret` stores the ciphertext and `grant_secret` grants the component in a separate locked step; the client-side grant merge was removed because it opened a TOCTOU with `revoke_secret`. The docs said "granted to the component" as though `set_secret` did both. **The tier is now explicit.** `set_secret` sends `processEnv: false`, so a pre-existing global row at the derived name is converted to the component-scoped tier rather than the pasted token being written into the tier every component and child process reads. Worth stating, since the difference is invisible from the CLI output and is what "Granted to component X" actually promises.
dawsontoth
changed the title
Five defects found by a claim-versus-source pass over this branch's own diff. The first is the one that mattered. **The OIDC exchange was documented one position too low.** The precedence list put it at 8, below the legacy `username=`/`password=` fallback, and the prose said it "ranks last". In `bin/cliOperations.ts` the exchange is an `else if` on the token branch, so it runs before the legacy fallback, which is gated on no Authorization header already being set. Real order is 7 = exchange, 8 = legacy. That inversion matters in the same direction as the v5.2.0 precedence change: on a runner with `id-token: write` and no configured credential, `harper add_user username=svc password=... target=...` authenticates as the trust policy's user, not as `svc`. The docs promised the opposite. Now stated, including the migration hazard for a script that was passing those fields as credentials. **`force` on `revert_component` was described with the wrong semantics** — "bypass the safety checks on the revert target". It is consumed only by `assertNotProtectedCoreComponent`, so it permits writing over a protected core component name and has no effect on `to_deployment_id` validation. As written it invited reaching for `force` when a revert is refused for naming an un-revertable target, where it cannot help, while hiding what it does do. **The deploy-modes table omitted the two knobs that defeat the barrier.** The section calls `replicated` one of the interacting parameters and then never gives it a row, and `ignore_replication_errors: true` makes stage failures non-fatal (`operations.js:875`) so activation proceeds despite the barrier having failed. The barrier is the feature's headline guarantee; its opt-out belongs in the table a reader consults to decide which mode is safe, not only in a parameter bullet. **The blank-token paragraph contradicted itself** — "an error, not a fallback", then described the fallback, then denied it. The CLI warns and continues down the precedence list, which does mean running as whoever last logged in; the warning is the only thing that makes it non-silent. Rewritten to say that, and to treat the warning as a CI failure signal. **Audience instance-specificity was stated as universal validation.** Only the GitHub profile enforces it; the generic profile's `assertAudienceIsSpecific` is empty. Scoped, with a pointer to the `sub` pin that binds a generic policy — the same guidance-versus-enforcement distinction this branch just applied to the SSH-key encryption claim. Also: `reference/operations-api/operations.md` was unformatted and would have failed CI. `npm run format` is `prettier .` with no `--write`, so it prints and changes nothing; the gate is `format:write` / `format:check`, per AGENTS.md.
…ckage.json The existing guidance says to derive the badge from "the core release the change ships in" and points at prerelease suffixes, which reads as an instruction to look at the feature branch's package.json. That is what went wrong on this PR: the branch said `5.2.0-beta.3`, so a `v5.2.0` badge looked confirmed, and it stayed wrong for about a month while 5.2.0, 5.2.1 and 5.2.2 were all cut without the feature. The whole surface had become v5.3.0. Records the two commands that actually answer the question and the re-check obligation for a long-lived docs PR, since a release cut between refresh passes invalidates a badge that was right when it was written.
dawsontoth
added a commit
that referenced
this pull request
Aug 18, 2026
The other half of @kriszyp's discoverability finding, for the piece that actually shipped in v5.2.0. A user who only reads release notes had no way to discover that a pipeline can authenticate with a token instead of an admin password. States the two properties that change how someone sets this up: the token variables outrank a saved login (so a runner with both uses the CI identity), and one refresh-token hash per user means issuing a token for CI revokes your own if you run it as yourself. The 5.3 surface goes in 5.3.md, added on #599.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
… claims Cross-model review (codex leg) against 4d1084d raised 13 findings. Three were false positives from reading the wrong tree — it checked `revert_component` against harper `main`, where harper#1849 is unmerged, so the operation looks deleted; it read `add_ssh_key generate` before ab11290 moved keygen in-process; and its claim that activation rebuilds a missing staged tree is the opposite of `activateStagedApplication`, which throws. The other ten were real. The largest is that harper#2173 was rebased and gained eleven commits adding an operation-scoping feature this branch never documented. **`operations` on a trust policy.** 1-100 unique names, validated at write time against the registry `add_role` uses, carried into the minted token as a narrowing-only intersection with the user's role. Documented with the two things that decide whether it is the control an operator thinks it is: - It covers the Operations API and SQL (`verifyPerms`/`verifyPermsAST`) and **not** an application's REST/GraphQL resource path, which authorizes through `checkPermission` — so a scoped token still carries the role's full table CRUD there. It bounds what CI can administer, not what data it can reach, and the policy's user still has to be a least-privilege role. - A runtime-registered operation (`server.registerOperation`) is not recognized, so a policy naming one is rejected. Fails closed, and shared with add_role. Also that a scoped token cannot trade itself for a login session. **`job_workflow_ref` no longer gates the ref**, and the removal is deliberate: for a reusable workflow it names the workflow that ran, not the caller, so its `@ref` is constant however it is invoked and would admit any caller branch. It still pins the workflow. The table said otherwise, which would have produced policies an admin believed were ref-gated. Six claims narrowed to what the code actually guarantees — the same guidance-versus-enforcement distinction applied earlier to the SSH-key text: - Audience: Harper rejects GitHub's shared default shape, but does not compare the value against its own identity, so a typo is accepted at write time and fails at exchange time instead. Not checked at all for a generic issuer. - The one-hour token is a reduced exposure window, not "worthless by the time it reaches a log" — inside the hour it is a live credential. - `drop_oidc_trust` stops future exchanges; it cannot revoke an already-issued stateless JWT. Names the user/role action that actually contains a leak. - A rejected OIDC exchange does not reliably 401: the CLI continues down precedence and a payload `username=`/`password=` pair will authenticate, so a policy mismatch can look like a working deploy under the wrong identity. - A pinned SHA fixes the source revision, not the built artifact — the cluster builds from source per node. - `credential=true` is the git-by-reference path; an npm-registry credential is attached with the printed entry naming `registry`. Release notes gain the replay race (records replicate asynchronously, so simultaneous replays on different nodes can both land) and that auth-event auditing is off by default. Ref resolution in `applications.md` also corrected here rather than only in #617, since #599 would otherwise publish the local-only description on its own.
dawsontoth
added a commit
that referenced
this pull request
Aug 18, 2026
One conflict, in the `by_ref` SHA-pinning paragraph both branches now touch. Kept this branch's version: it is the superset (it names annotated-tag peeling) and it already carries the fail-closed behavior as its own following paragraph, which is what #599's sentence was adding inline. `git checkout --ours` on that file took the whole file rather than the hunk, so it silently dropped #599's two other edits to the same section — the corrected "same source revision" wording and the new source-versus-artifact paragraph. Re-applied both, then diffed against #599 to confirm nothing else was lost. The remaining differences are only this branch's intended replacements.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
…aims Round 3 of the cross-model review (codex at 5d4ce0c) re-raised as major something I had rejected as a false positive twice. It was right and I was wrong. **A `deployment_id` activate can re-fetch and re-install.** I checked `activateStagedApplication`, saw it throw on a missing staged tree, and concluded no rebuild path existed. The recovery is at the call site, not in the callee: `deployComponentActivateExisting` tests `hasCompleteStagedApplication` and, when it fails, calls `sourceStagedPayload` and `stageApplication` before activating. So "no new payload is fetched or installed" is only true while every staged tree is intact — a restart or disk repair between staging and activation makes that node rebuild, which is slower and can fail outright if the source or credential is no longer reachable. Documented in both operations.md and the release note. The lesson is narrow and worth stating: verifying a negative claim by reading the function that would perform the action is not enough when a caller can do it first. **Rollback retention is conditional.** `activateStagedApplication` only records a revertable copy when the caller passes `activationSpec` — its own docblock says omitting it leaves the component "not revertable afterwards". The release note claimed every two-phase activation retains its predecessor. Three narrower corrections: - `deploy setup=true` supports exactly two providers, `github` and `npm`, and rejects anything else. The release note implied any git host. - That flow calls `get_secrets_public_key` and `set_secret`, both super_user, so it needs an administrative credential rather than the CI identity it is provisioning. - The SHA-pins-source-not-artifact correction reached applications.md last round but not the release note, which still said "an exact redeploy". Also drops the inline `<VersionBadge>` from the precedence list item added last round. CONTRIBUTING.md reserves the component for standalone placement after a heading, and I had cited that same rule while declining a bot's inline-badge suggestion on #617 — then did it myself. Fourteen further findings from this round are unaddressed and unverified; they are in the PR description rather than silently dropped.
🚀 Preview DeploymentYour preview deployment is ready! 🔗 Preview URL: https://preview.harper-documentation.harperfabric.com/pr-599 This preview will update automatically when you push new commits. |
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
4 participants
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.
Depends-on: HarperFast/harper#1849, HarperFast/harper#1851, HarperFast/harper#2173
Depends-on: HarperFast/harper-pro#594
What this documents
Six files, not the two the previous description listed. The cluster-wide deploy lifecycle, rollback, deploy-by-reference, sealed deploy credentials, server-side SSH keygen, and OIDC trusted publishing.
revert_component, staging retentiondeploy by_ref=true/ref=/credential=deploy setup=trueadd_ssh_key generate: trueTwo things changed since the last review pass
This is v5.3.0 surface, not v5.2.0. The placeholder is resolved, and it resolved differently than assumed. harper
mainis at 5.2.2 (tagged 2026-08-13) and by_ref merged 2026-08-14 — after that tag, and in no release. harper#2173 addsupgrade/directives/5-3-0.ts, confirming which release is next. Every badge here is nowv5.3.0.The one piece that genuinely shipped in v5.2.0 has left.
login --for-ciand the token env vars (harper#1876, in v5.2.0 since 2026-07-31) are now #630, againstmain, mergeable today. That answers @Ethan-Arrowood's first sequencing question — the shipped half no longer waits on four unmerged PRs — and it puts the token rules insidemain's canonical numbered Authentication Precedence section rather than in a competing paragraph beside it.Review feedback
@kriszyp's eight findings (CHANGES_REQUESTED), each re-checked against upstream as it stands now rather than as it stood when filed. Four had been fixed at the source in the meantime.
refnot resolved to a SHA5c85714b8,26cce986d,21043f994).restartdocumented as optional.auth_*vs. ordinaryusername=precedencemain's canonical list covers it, #630 extends it.5.3.md.revert_componentrow.Worth expanding on three:
#### Activation failuressection rather than sitting mid-parameter-list, and the opening paragraph points at it.activate: falseanddeployment_idare rejected when combined withtwo_phase: false,replicated: false, or an unreplicatedsystemdatabase, so a request that says stage-only can no longer activate. There is now a valid-combinations table, including thatrevert_on_failureis refused rather than ignored.sealSSHKeystill stores and replicates the private key in plaintext with a WARN when no secret custody is registered, deliberately, since SSH keys predate custody. So encryption at rest is a property of the deployment's configuration, not of the operation — stated as a warning, while noting custody is present by default so the risk doesn't read as likely.@Ethan-Arrowood's structural hold is addressed:
main— resolved by hand, and it was the hazard he predicted. The ops table needed a three-way merge:main's seven backup rows (create_backup,list_backups,verify_backup,delete_backup,purge_backups,restore_backup,get_backup), this branch's four deploy rows, and this branch's recategorization. Taking either side wholesale would have silently dropped one set. I compared both sides row by row first and confirmed no description text differed, so the width churn is only Prettier re-padding.authentication.mdno longer states precedence twice, since docs(cli): token credentials in the canonical auth precedence (v5.2.0) #630 owns that file's content.(draft)in the title — the first is gone with the split, this description is rewritten, the title marker is dropped in favour of GitHub's own draft state.system-replication precondition (his second question) — now documented. A plain deploy on a cluster withsystemexcluded from replication silently takes the one-shot path, which also means no rollback target; the staged parameters are rejected rather than downgraded.@cb1kenobi's five findings were fixed in the previous pass and remain so.
Still open
deployment_stagingRetention_maxCountanddeployment_payloadRetention_*are documented inline, not inreference/configuration/options.md. @Ethan-Arrowood is right that operators look there, and right that the original justification ("matching howpayloadRetention_maxSizeis surfaced") was false — it isn't surfaced anywhere. Left as-is deliberately: three config keys for an unreleased feature seem better added when the options page can be updated in one pass against the shipping release. Happy to add them here instead.COMPANION_CHECK_TOKENconfigured before Add companion-check workflow: docs PRs auto-merge once their companion code PR lands #629's check can resolve it; without the token it fails closed rather than passing.Verification
npm run buildandnpm run format:checkclean. The build reports two broken anchors; both are pre-existing onmain(confirmed by buildingmainwith these changes stashed) and neither is in a file this PR touches. Every claim re-verified against the current upstream branches — including three places where harper#2173's own description has drifted from its code: it is now issuer-agnostic with GitHub as a profile, replay keys on a token hash rather thanissuer|jti, and the auth-audit-log gap it lists as not-done is implemented.Description drafted by Claude Code (Opus 5).