fix(wiki): compare drift against what NodeBB stored, not what we rendered (#104)
build-binaries / build-binaries (push) Successful in 2m56s
build-binaries / build-binaries (push) Successful in 2m56s
Fixes #103. ## What was wrong `crucible wiki deploy` decided a page had drifted by hashing NodeBB's stored copy and comparing it to the hash of the text Crucible rendered. Those match only if NodeBB gives our HTML back byte for byte. It does not, so every `sow-topdata` tag deploy failed: ``` local pages: 1200, updated: 1, skipped: 1176, drifted: 23 remote managed wiki content drifted; rerun with --force to overwrite ``` Nobody had edited those pages. The operator's only way out was to leave `--force` on, which removes the protection the guard exists for. ## What changed - New manifest field `remote_hash`: the managed-region hash of the post NodeBB hands back right after we write it. Drift compares against that, so it means "the live page changed after we last wrote it". - A post with no `sourceContent` is not drift. It predates `sourceContent` sync, reads back as rendered HTML, and belongs to the existing `SourceContentSynced` repair — which the drift refusal used to block. - The error names the drifted pages, capped at ten plus a count. - Deleted the dead `wikiDeployPlan.RemoteHash` field. Old manifests keep the previous comparison until each page is next written, so no re-seed is needed. Cost: one extra post read per page written. Pages that skip are still never fetched. ## Tests `internal/topdata/wiki_deploy_test.go`, same seam as the rest of the file (`DeployWikiWithOptions` against the fake NodeBB): normalized remote copy is not drift, a hand-edited page still is and the error names it, `--force` overwrites it and re-records the hash, a post without `sourceContent` is repaired instead of refused, and an old-format manifest deploys and gains a `remote_hash`. `make check` passes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)Reviewed-on: #104 Co-authored-by: vickydotbat <vickydotbat@tutamail.com>
This commit was merged in pull request #104.
This commit is contained in:
@@ -62,8 +62,11 @@ type DeployResult struct {
|
||||
Purged int
|
||||
Skipped int
|
||||
Drifted int
|
||||
Renamed int
|
||||
Manifest string
|
||||
// DriftedPages names the pages counted in Drifted, in plan order, so an
|
||||
// operator can look at them before deciding whether --force is safe.
|
||||
DriftedPages []string
|
||||
Renamed int
|
||||
Manifest string
|
||||
// ResetPurged counts the deletions queued by --reset-managed-namespaces.
|
||||
// They are also included in Stale and Purged, because callers warn about
|
||||
// destructive policies in terms of the stale count: sow-topdata's
|
||||
@@ -94,7 +97,13 @@ type wikiDeployManifest struct {
|
||||
}
|
||||
|
||||
type wikiDeployManifestPage struct {
|
||||
Hash string `json:"hash"`
|
||||
Hash string `json:"hash"`
|
||||
// RemoteHash is the managed-region hash of the post body NodeBB handed back
|
||||
// right after our last write to it. NodeBB owns that body, so this — not
|
||||
// Hash, which is the hash of the text we rendered — is what the next run's
|
||||
// drift check compares against. Empty on manifests written before this
|
||||
// field existed; the drift check falls back to Hash for those.
|
||||
RemoteHash string `json:"remote_hash,omitempty"`
|
||||
LastSeenHash string `json:"last_seen_hash,omitempty"`
|
||||
ArchivedHash string `json:"archived_hash,omitempty"`
|
||||
Title string `json:"title,omitempty"`
|
||||
@@ -108,12 +117,11 @@ type wikiDeployManifestPage struct {
|
||||
}
|
||||
|
||||
type wikiDeployPlan struct {
|
||||
Page wikiDeployPage
|
||||
Entry wikiDeployManifestPage
|
||||
Action string
|
||||
Content string
|
||||
RemoteHash string
|
||||
Title string
|
||||
Page wikiDeployPage
|
||||
Entry wikiDeployManifestPage
|
||||
Action string
|
||||
Content string
|
||||
Title string
|
||||
// Reset marks a purge queued by --reset-managed-namespaces rather than by
|
||||
// stale computation over the manifest.
|
||||
Reset bool
|
||||
@@ -223,7 +231,7 @@ func DeployWikiWithOptions(p *project.Project, opts DeployWikiOptions, progress
|
||||
return result, nil
|
||||
}
|
||||
if result.Drifted > 0 && !opts.Force {
|
||||
return result, errors.New("remote managed wiki content drifted; rerun with --force to overwrite")
|
||||
return result, fmt.Errorf("remote managed wiki content drifted on %d page(s) (%s); rerun with --force to overwrite", result.Drifted, summarizeDriftedPages(result.DriftedPages))
|
||||
}
|
||||
|
||||
summary := p.EffectiveConfig().TopData.Wiki.DeployEditSummary
|
||||
@@ -259,10 +267,16 @@ func DeployWikiWithOptions(p *project.Project, opts DeployWikiOptions, progress
|
||||
entry.TID = created.TID
|
||||
entry.PID = created.PID
|
||||
nextManifest.Pages[plan.Page.PageID] = entry
|
||||
if err := recordRemoteHash(nextManifest, plan.Page.PageID, created.PID, client); err != nil {
|
||||
return result, err
|
||||
}
|
||||
case "update":
|
||||
if err := client.updatePost(plan.Entry.TID, plan.Entry.PID, plan.Content, summary); err != nil {
|
||||
return result, fmt.Errorf("deploy wiki page %q: update NodeBB post %d: %w", plan.Page.PageID, plan.Entry.PID, err)
|
||||
}
|
||||
if err := recordRemoteHash(nextManifest, plan.Page.PageID, plan.Entry.PID, client); err != nil {
|
||||
return result, err
|
||||
}
|
||||
case "archive":
|
||||
// Archiving rewrites the page rather than removing it, so it goes
|
||||
// through the ordinary post edit the wiki plugin allows; only
|
||||
@@ -680,8 +694,9 @@ func planNodeBBDeploy(pages map[string]wikiDeployPage, manifest wikiDeployManife
|
||||
next.Pages[pageID] = entry
|
||||
}
|
||||
remoteHash := computeManagedHash(remote.Content)
|
||||
if manifest.Pages[pageID].Hash != "" && remoteHash != manifest.Pages[pageID].Hash {
|
||||
if remoteContentDrifted(manifest.Pages[pageID], remote, remoteHash) {
|
||||
result.Drifted++
|
||||
result.DriftedPages = append(result.DriftedPages, pageID)
|
||||
if !opts.Force {
|
||||
continue
|
||||
}
|
||||
@@ -694,7 +709,7 @@ func planNodeBBDeploy(pages map[string]wikiDeployPage, manifest wikiDeployManife
|
||||
result.Updated++
|
||||
entry.SourceContentSynced = true
|
||||
next.Pages[pageID] = entry
|
||||
plans = append(plans, wikiDeployPlan{Page: page, Entry: entry, Action: "update", Content: merged, RemoteHash: remoteHash})
|
||||
plans = append(plans, wikiDeployPlan{Page: page, Entry: entry, Action: "update", Content: merged})
|
||||
}
|
||||
for pageID := range manifest.Pages {
|
||||
if _, ok := pages[pageID]; !ok {
|
||||
@@ -869,6 +884,9 @@ func recoverCreateCollision(plan wikiDeployPlan, manifest wikiDeployManifest, cl
|
||||
entry.Stale = false
|
||||
entry.SourceContentSynced = true
|
||||
manifest.Pages[plan.Page.PageID] = entry
|
||||
if err := recordRemoteHash(manifest, plan.Page.PageID, entry.PID, client); err != nil {
|
||||
return false, err
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
@@ -1206,6 +1224,49 @@ func saveDeployManifest(path string, manifest wikiDeployManifest) error {
|
||||
return os.WriteFile(path, append(raw, '\n'), 0o644)
|
||||
}
|
||||
|
||||
// summarizeDriftedPages names the drifted pages for the operator, capped so a
|
||||
// mass-drift run does not bury the rest of the CI log.
|
||||
func summarizeDriftedPages(pages []string) string {
|
||||
const maxNamed = 10
|
||||
if len(pages) <= maxNamed {
|
||||
return strings.Join(pages, ", ")
|
||||
}
|
||||
return fmt.Sprintf("%s and %d more", strings.Join(pages[:maxNamed], ", "), len(pages)-maxNamed)
|
||||
}
|
||||
|
||||
// remoteContentDrifted answers the only question the drift guard exists to ask:
|
||||
// did the live page change after we last wrote it? It is not "does the live
|
||||
// page differ from what we just rendered" — NodeBB owns the stored body, and a
|
||||
// deploy that regenerates a page always differs from what is live.
|
||||
func remoteContentDrifted(entry wikiDeployManifestPage, remote nodeBBPost, remoteHash string) bool {
|
||||
// A post with no sourceContent was written before the deployer stored one,
|
||||
// so what comes back is NodeBB's rendered HTML and can never hash-equal
|
||||
// anything we wrote. That is a page awaiting the SourceContentSynced
|
||||
// repair, not a human edit.
|
||||
if remote.SourceContent == "" {
|
||||
return false
|
||||
}
|
||||
if entry.RemoteHash != "" {
|
||||
return remoteHash != entry.RemoteHash
|
||||
}
|
||||
// Manifest written before RemoteHash existed: fall back to the old
|
||||
// comparison rather than declaring every tracked page drifted at once.
|
||||
return entry.Hash != "" && remoteHash != entry.Hash
|
||||
}
|
||||
|
||||
// recordRemoteHash reads a page back after writing it and records what NodeBB
|
||||
// actually stored, so the next run compares remote against remote.
|
||||
func recordRemoteHash(manifest wikiDeployManifest, pageID string, pid int, client *nodeBBClient) error {
|
||||
remote, err := client.getPost(pid)
|
||||
if err != nil {
|
||||
return fmt.Errorf("deploy wiki page %q: read back NodeBB post %d: %w", pageID, pid, err)
|
||||
}
|
||||
entry := manifest.Pages[pageID]
|
||||
entry.RemoteHash = computeManagedHash(remote.Content)
|
||||
manifest.Pages[pageID] = entry
|
||||
return nil
|
||||
}
|
||||
|
||||
func computeManagedHash(content string) string {
|
||||
managed, ok := extractManagedRegion(content)
|
||||
if !ok {
|
||||
|
||||
Reference in New Issue
Block a user