Skip to content

fix(org): finish interrupted verifications and compare drafts by markup - #167

Merged
mortik merged 4 commits into
mainfrom
fix/org-verification-safety
Oct 7, 2026
Merged

mortik merged 4 commits into
mainfrom
fix/org-verification-safety

Conversation

@mortik

@mortik mortik commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Fixes from a review of #165 (org verification), for fleetyards/fleetyards#5465.

What changed

  • An interrupted run is finished, not skipped. Saving and publishing are now decided separately: saving from the draft, publishing from whether the live page shows the token.
    • Before, if saveDraft worked and publishDraft failed, every retry found the token in the draft and did nothing. The token stayed unpublished, and went live with the next officer edit.
    • A remove that saved but failed to publish is finished the same way.
  • One request per org at a time (and one for the bio). A remove sent while a slow write is still out used to read the draft before the write landed, find nothing, and answer success. The write then published the token.
  • The pending check compares markup by section, not text by position:
    • An unpublished image, link or formatting change counts now.
    • Nested divs from Textile div. blocks no longer cut a section short.
    • A section one page leaves out counts as empty, instead of reading as a permanent difference.
  • Checked again before publishing. After saving, the draft is compared with the live page once more. If another officer's edit arrived in between, the draft is put back and the action answers 409.
  • A token placed elsewhere by hand answers 409 on remove instead of 200 changed: false, so the site warns that it's still public. Whether the token is live is read from the history section alone.
  • Putting the draft back never overwrites an officer's edit: it re-reads the history and restores it only while it still holds exactly what this request saved. A refused restore is reported, with the token marked as possibly left in the draft.
  • The three org pages are fetched in parallel.
  • Token helpers are shared (lib/tokens.ts) instead of the org flow borrowing the bio's.

Test plan

  • pnpm test (95): markup changes, nested divs, missing sections, publish-only retry, re-check with the draft put back, a hand-placed token, and a remove waiting for a write (fails without the queue)
  • pnpm compile, pnpm build
  • End to end with an officer session (by the author): verified a fleet with another unpublished draft change, which stayed unpublished. The same run with main's build (feat: org-verify-write and org-verify-remove actions for fleet verification #165) published it.

🤖

mortik added 3 commits October 7, 2026 17:43
Text alone missed an unpublished image, link or formatting change, and a Textile div block cut the comparison short. A section one page leaves out counts as empty.
Saving and publishing are decided apart, from the draft and the live page, so a run that saved but failed to publish is completed by the next. The draft is compared again right before publishing and put back if another edit arrived. Requests for one org, and for the bio, run one at a time, so a remove cannot overtake a slow write. A token the extension did not place is reported, not skipped.
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 11ab56db-d91d-4cc7-a237-0f305d412404
📥 Commits

Reviewing files that changed from the base of the PR and between 42e4302 and df86637.

📒 Files selected for processing (6)
  • __tests__/message-handler.test.ts
  • __tests__/org.test.ts
  • lib/bio.ts
  • lib/message-handler.ts
  • lib/org.ts
  • lib/tokens.ts
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 potential issues.

Devin Review

Comment thread lib/message-handler.ts Outdated
Comment thread lib/message-handler.ts Outdated
Comment thread lib/message-handler.ts Outdated
Comment on lines +275 to +277
await saveOrgDraft(rsiToken, sid, ORG_VERIFICATION_FIELD, draft);
}
return failed(409, "Unpublished changes");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Failed rollback leaves token queued

When the second check finds another edit and saveOrgDraft refuses rollback, verifyOrg still returns 409. The token remains in the draft and can go live with a later edit.

Learn more

After saving a verification token, the second preview comparison can detect a pending edit. The conflict branch then tries to restore the previous history. RSI can return HTTP 200 with success: 0, which reportsSuccess treats as failure elsewhere; this branch ignores the response. A failed restore leaves the token in the draft, yet the response reports only an unpublished-changes conflict.

Example: A token save succeeds, another officer edits the manifesto, and the restore request returns { success: 0, msg: "ErrCsrf" }. The request returns 409 while history still contains the token, ready to be included in a future publication.

Recommended fix: Inspect rollback's HTTP status and body with reportsSuccess, and return a rollback failure that exposes the unresolved draft state. Avoid claiming restoration when the compensating write fails.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread lib/message-handler.ts Outdated
…ken by its own section

Putting the draft back after a late conflict re-reads the history first and restores it only while it still holds exactly what this request saved; a refused restore is reported, with the token marked as possibly left in the draft. Whether the token is live is read from the history section alone: a token in another section is one the extension never wrote, so a removal refuses it instead of publishing and reporting success. The re-check before publishing no longer fetches the admin form.
@mortik
mortik added this pull request to the merge queue Oct 7, 2026
Merged via the queue into main with commit 64d0d6d Oct 7, 2026
5 checks passed
@mortik mortik mentioned this pull request Oct 7, 2026
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.

1 participant