rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 6 Wiki Activity

fix: remove worst-wins escalation logic #31

Merged
rodin merged 4 commits from fix/28-remove-escalation into main 2026-05-02 16:46:06 +00:00
Conversation 12 Commits 4 Files Changed 3
+157 -182
rodin commented 2026-05-02 14:07:15 +00:00
Owner
Copy Link
Copy Source

Closes #28
Closes #29

Summary

Removes the entire worst-wins escalation system (shouldEscalate, pre-posting check, post-posting re-escalation). With separate tokens per reviewer role (security-review-bot user now exists), Gitea handles multi-reviewer blocking natively.

Changes

  • Remove shouldEscalate() function and all calls
  • Remove pre-posting escalation check
  • Remove post-posting escalation block (re-fetch + supersede + repost)
  • Remove TestShouldEscalate (8 test cases)
  • Keep: findOwnReview, reviewUnchanged, update-in-place/supersede logic

Deployment Note

After merging, CI workflow needs updating to use SECURITY_REVIEW_TOKEN pointing to the new security-review-bot Gitea user (already created with write access to grgl/gargoyle and rodin/review-bot).

Closes #28 Closes #29 ## Summary Removes the entire worst-wins escalation system (`shouldEscalate`, pre-posting check, post-posting re-escalation). With separate tokens per reviewer role (security-review-bot user now exists), Gitea handles multi-reviewer blocking natively. ## Changes - Remove `shouldEscalate()` function and all calls - Remove pre-posting escalation check - Remove post-posting escalation block (re-fetch + supersede + repost) - Remove `TestShouldEscalate` (8 test cases) - Keep: `findOwnReview`, `reviewUnchanged`, update-in-place/supersede logic ## Deployment Note After merging, CI workflow needs updating to use `SECURITY_REVIEW_TOKEN` pointing to the new `security-review-bot` Gitea user (already created with write access to grgl/gargoyle and rodin/review-bot).
rodin added 1 commit 2026-05-02 14:07:16 +00:00
fix: remove worst-wins escalation logic (#28)
CI / test (pull_request) Successful in 14s
Details
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
Details
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 36s
Details
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 39s
Details
b78d9972ac
gpt-review-bot approved these changes 2026-05-02 14:07:53 +00:00
gpt-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Gpt Review

Summary

The PR removes all worst-wins escalation logic in favor of multi-user reviewer separation and corrects the security reviewer token in CI. All changes are correct, safe, and remove significant complexity. CI passes and all relevant tests are updated.

Recommendation

APPROVE — The removal of shouldEscalate and related logic is clean, affecting only intended code paths. The update to SECURITY_REVIEW_TOKEN in the workflow is correct. All supporting tests for escalation logic have been removed, and existing positive tests for update-in-place, shared-token protection, and sentinel extraction are intact. With CI passing and no style or correctness issues identified, this PR is ready to merge.

Review by gpt

# Gpt Review ## Summary The PR removes all worst-wins escalation logic in favor of multi-user reviewer separation and corrects the security reviewer token in CI. All changes are correct, safe, and remove significant complexity. CI passes and all relevant tests are updated. ## Recommendation **APPROVE** — The removal of shouldEscalate and related logic is clean, affecting only intended code paths. The update to SECURITY_REVIEW_TOKEN in the workflow is correct. All supporting tests for escalation logic have been removed, and existing positive tests for update-in-place, shared-token protection, and sentinel extraction are intact. With CI passing and no style or correctness issues identified, this PR is ready to merge. --- *Review by gpt* <!-- review-bot:gpt -->
sonnet-review-bot approved these changes 2026-05-02 14:08:07 +00:00
Dismissed
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Security Review

Summary

Security posture is preserved or improved by removing the escalation logic and adding a benign misconfiguration warning. No new injection vectors, path traversal, secrets exposure, or authorization flaws are introduced, and CI has passed.

Recommendation

APPROVE — The changes remove complex escalation behavior and replace it with a simple warning for shared-token misconfiguration, relying on Gitea’s native multi-reviewer blocking. Input validation for reviewer names remains in place to protect the sentinel, and path handling for the system prompt file continues to guard against traversal and symlink attacks. Given CI passed and no security-relevant issues are introduced by this diff, proceed with merge. Ensure deployment updates the workflow to use the new SECURITY_REVIEW_TOKEN as noted.

Review by security

# Security Review ## Summary Security posture is preserved or improved by removing the escalation logic and adding a benign misconfiguration warning. No new injection vectors, path traversal, secrets exposure, or authorization flaws are introduced, and CI has passed. ## Recommendation **APPROVE** — The changes remove complex escalation behavior and replace it with a simple warning for shared-token misconfiguration, relying on Gitea’s native multi-reviewer blocking. Input validation for reviewer names remains in place to protect the sentinel, and path handling for the system prompt file continues to guard against traversal and symlink attacks. Given CI passed and no security-relevant issues are introduced by this diff, proceed with merge. Ensure deployment updates the workflow to use the new SECURITY_REVIEW_TOKEN as noted. --- *Review by security* <!-- review-bot:security -->
sonnet-review-bot approved these changes 2026-05-02 14:08:10 +00:00
Dismissed
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Sonnet Review

Summary

The PR cleanly removes the worst-wins escalation logic and replaces it with a safer warning mechanism for shared tokens. Tests were updated accordingly and CI is green. The remaining review update logic (findOwnReview, reviewUnchanged, supersede/update-in-place) remains intact and consistent.

Findings

# Severity File Line Finding
1 [MINOR] cmd/review-bot/main.go 504 warnSharedToken does not skip stale reviews when detecting shared-token usage; this could produce a warning based on an outdated sibling review. Consider ignoring r.Stale to reduce false positives.
2 [NIT] cmd/review-bot/main.go 512 The sentinel prefix "<!-- review-bot:" is repeated across functions (warnSharedToken, extractSentinelName). Consider extracting a shared constant to avoid typos and ease future changes.
3 [NIT] cmd/review-bot/main.go 504 Log message uses a Unicode em dash (—). While generally fine, some log aggregation systems may not preserve it. Consider using a standard hyphen-minus for maximum compatibility.

Recommendation

APPROVE — This change aligns with the goal of relying on separate reviewer identities rather than in-process escalation, simplifying logic and avoiding surprising state changes post-post. The new warnSharedToken and extractSentinelName helpers are straightforward and covered by tests. I recommend merging as-is. Optionally, refine warnSharedToken to ignore stale reviews and consolidate the sentinel prefix into a shared constant for maintainability.

Review by sonnet

# Sonnet Review ## Summary The PR cleanly removes the worst-wins escalation logic and replaces it with a safer warning mechanism for shared tokens. Tests were updated accordingly and CI is green. The remaining review update logic (findOwnReview, reviewUnchanged, supersede/update-in-place) remains intact and consistent. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `cmd/review-bot/main.go` | 504 | warnSharedToken does not skip stale reviews when detecting shared-token usage; this could produce a warning based on an outdated sibling review. Consider ignoring r.Stale to reduce false positives. | | 2 | [NIT] | `cmd/review-bot/main.go` | 512 | The sentinel prefix "<!-- review-bot:" is repeated across functions (warnSharedToken, extractSentinelName). Consider extracting a shared constant to avoid typos and ease future changes. | | 3 | [NIT] | `cmd/review-bot/main.go` | 504 | Log message uses a Unicode em dash (—). While generally fine, some log aggregation systems may not preserve it. Consider using a standard hyphen-minus for maximum compatibility. | ## Recommendation **APPROVE** — This change aligns with the goal of relying on separate reviewer identities rather than in-process escalation, simplifying logic and avoiding surprising state changes post-post. The new warnSharedToken and extractSentinelName helpers are straightforward and covered by tests. I recommend merging as-is. Optionally, refine warnSharedToken to ignore stale reviews and consolidate the sentinel prefix into a shared constant for maintainability. --- *Review by sonnet* <!-- review-bot:sonnet -->
aweiker requested changes 2026-05-02 14:09:30 +00:00
Dismissed
aweiker left a comment
Contributor
Copy Link
Copy Source

what happens when someone doesn't follow instructions and uses the same token twice? can we detect and fallback into a safe mode?

what happens when someone doesn't follow instructions and uses the same token twice? can we detect and fallback into a safe mode?
rodin added 1 commit 2026-05-02 14:12:12 +00:00
fix: detect shared-token misconfiguration and warn
CI / test (pull_request) Successful in 14s
Details
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 24s
Details
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 42s
Details
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 55s
Details
fd179b891b 
When two review-bot roles share the same Gitea user token (misconfiguration),
log a WARNING identifying which sibling is sharing. The bot continues normally
with its own honest verdict — no escalation, no deadlock. Operators see the
warning in CI logs and can fix the token setup.

Addresses Aaron's review feedback on #28: graceful degradation when someone
doesn't follow the separate-token deployment instructions.
rodin commented 2026-05-02 14:12:19 +00:00
Author
Owner
Copy Link
Copy Source

Good catch. Added warnSharedToken() — if two review-bot roles post under the same Gitea user, it logs:

WARNING: shared token detected — another review-bot role (security) is using the same Gitea user "sonnet-review-bot". Each role should have its own token/user for proper multi-reviewer blocking.

Behavior on misconfiguration:

  • Each role still posts its own honest verdict (no escalation, no deadlock)
  • Warning is visible in CI logs so operators know to fix it
  • No crash, no silent corruption

Safe degradation: worst case is two reviews from the same user (Gitea only enforces the last one posted), which is the same behavior as today minus the deadlock risk.

Good catch. Added `warnSharedToken()` — if two review-bot roles post under the same Gitea user, it logs: ``` WARNING: shared token detected — another review-bot role (security) is using the same Gitea user "sonnet-review-bot". Each role should have its own token/user for proper multi-reviewer blocking. ``` Behavior on misconfiguration: - Each role still posts its own honest verdict (no escalation, no deadlock) - Warning is visible in CI logs so operators know to fix it - No crash, no silent corruption Safe degradation: worst case is two reviews from the same user (Gitea only enforces the last one posted), which is the same behavior as today minus the deadlock risk.
aweiker reviewed 2026-05-02 14:15:04 +00:00
cmd/review-bot/main.go Outdated
@@ -269,2 +269,2 @@
// Worst-wins: escalate if a sibling blocks (need own login from existing review)
ownLogin := ""
// Detect shared-token misconfiguration: warn if sibling sentinel exists from same user
warnSharedToken(existingReviews, sentinel)
aweiker commented 2026-05-02 14:15:04 +00:00
Contributor
Copy Link
Copy Source

this should also make it fall into a mode where it only writes new reviews; never updates any review as we don't want to do that logic. What about review state? That might get clobbered too.

this should also make it fall into a mode where it only writes new reviews; never updates any review as we don't want to do that logic. What about review state? That might get clobbered too.
aweiker requested changes 2026-05-02 14:16:31 +00:00
Dismissed
aweiker left a comment
Contributor
Copy Link
Copy Source

No new tests were added for detecting the same token was used.

No new tests were added for detecting the same token was used.
rodin added 1 commit 2026-05-02 14:21:51 +00:00
fix: skip update-in-place when shared token detected
CI / test (pull_request) Successful in 14s
Details
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 22s
Details
CI / review (gpt-5, security, SECURITY_REVIEW.md, SONNET_REVIEW_TOKEN) (pull_request) Successful in 39s
Details
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m21s
Details
b1f5dd4b5f 
When hasSharedToken() detects two roles sharing the same Gitea user,
the bot now skips ALL update logic (PATCH, supersede) and always POSTs
a fresh review. This prevents clobbering a sibling's review body or
state when misconfigured.

Tests now assert return values (true/false) rather than just verifying
no panic. Added additional test case for three-roles-same-user scenario.

Addresses review feedback: update logic and review state must not
interact with sibling reviews under the same user.
rodin commented 2026-05-02 14:21:58 +00:00
Author
Owner
Copy Link
Copy Source

Addressed both findings:

  1. Shared-token mode now skips ALL update logic — no PATCH, no supersede, just always POST fresh. This prevents clobbering a sibling's review body or state. The function is now hasSharedToken() returning bool; when true, the entire update-in-place branch is bypassed.

  2. Tests assert actual return values — TestHasSharedToken checks true/false for each scenario (no reviews, no own review, separate users, shared token, three-roles-same-user). Plus TestExtractSentinelName covers the helper.

Addressed both findings: 1. **Shared-token mode now skips ALL update logic** — no PATCH, no supersede, just always POST fresh. This prevents clobbering a sibling's review body or state. The function is now `hasSharedToken()` returning bool; when true, the entire update-in-place branch is bypassed. 2. **Tests assert actual return values** — `TestHasSharedToken` checks `true`/`false` for each scenario (no reviews, no own review, separate users, shared token, three-roles-same-user). Plus `TestExtractSentinelName` covers the helper.
sonnet-review-bot approved these changes 2026-05-02 14:22:44 +00:00
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Security Review

Summary

The diff removes the escalation logic and adds shared-token detection without introducing security vulnerabilities. CI is passing and the changes do not affect input validation, secrets handling, or introduce injection or auth issues.

Recommendation

APPROVE — Proceed with merge. The removal of worst-wins escalation is safe given separate tokens per reviewer role and Gitea’s native handling. The new shared-token detection mitigates misconfiguration risks without exposing sensitive data. Ensure deployment notes are followed to use distinct tokens (e.g., SECURITY_REVIEW_TOKEN for the security-review-bot) so multi-reviewer blocking works as intended.

Review by security

# Security Review ## Summary The diff removes the escalation logic and adds shared-token detection without introducing security vulnerabilities. CI is passing and the changes do not affect input validation, secrets handling, or introduce injection or auth issues. ## Recommendation **APPROVE** — Proceed with merge. The removal of worst-wins escalation is safe given separate tokens per reviewer role and Gitea’s native handling. The new shared-token detection mitigates misconfiguration risks without exposing sensitive data. Ensure deployment notes are followed to use distinct tokens (e.g., SECURITY_REVIEW_TOKEN for the security-review-bot) so multi-reviewer blocking works as intended. --- *Review by security* <!-- review-bot:security -->
sonnet-review-bot approved these changes 2026-05-02 14:23:26 +00:00
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Sonnet Review

Summary

The changes cleanly remove the worst-wins escalation logic and replace it with a simpler, safer flow that relies on separate Gitea users per reviewer role. New shared-token detection is sensible, tests are updated accordingly, and CI passed.

Findings

# Severity File Line Finding
1 [MINOR] cmd/review-bot/main.go 269 When shared-token mode is detected, the code skips all update-in-place logic but also skips checking reviewUnchanged. This can lead to duplicate identical reviews being posted under the same login and sentinel. Consider still performing a reviewUnchanged check and returning early to avoid re-posting in this misconfiguration scenario.

Recommendation

APPROVE — Overall the removal of the escalation path is coherent with the new design relying on distinct Gitea users per reviewer role. The shared-token detection helper is a pragmatic safeguard to avoid clobbering another role’s review. As a small improvement, in the shared-token branch you could still check for an unchanged review and return early to prevent duplicate posts. Otherwise, the code is idiomatic, tests reflect the new behavior, and CI is green. I recommend approval.

Review by sonnet

# Sonnet Review ## Summary The changes cleanly remove the worst-wins escalation logic and replace it with a simpler, safer flow that relies on separate Gitea users per reviewer role. New shared-token detection is sensible, tests are updated accordingly, and CI passed. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `cmd/review-bot/main.go` | 269 | When shared-token mode is detected, the code skips all update-in-place logic but also skips checking reviewUnchanged. This can lead to duplicate identical reviews being posted under the same login and sentinel. Consider still performing a reviewUnchanged check and returning early to avoid re-posting in this misconfiguration scenario. | ## Recommendation **APPROVE** — Overall the removal of the escalation path is coherent with the new design relying on distinct Gitea users per reviewer role. The shared-token detection helper is a pragmatic safeguard to avoid clobbering another role’s review. As a small improvement, in the shared-token branch you could still check for an unchanged review and return early to prevent duplicate posts. Otherwise, the code is idiomatic, tests reflect the new behavior, and CI is green. I recommend approval. --- *Review by sonnet* <!-- review-bot:sonnet -->
cmd/review-bot/main.go
@@ -276,3 +269,1 @@
log.Printf("Sibling review has REQUEST_CHANGES; escalating to REQUEST_CHANGES")
event = "REQUEST_CHANGES"
}
// Detect shared-token misconfiguration: if detected, skip all
sonnet-review-bot commented 2026-05-02 14:23:26 +00:00
First-time contributor
Copy Link
Copy Source

[MINOR] When shared-token mode is detected, the code skips all update-in-place logic but also skips checking reviewUnchanged. This can lead to duplicate identical reviews being posted under the same login and sentinel. Consider still performing a reviewUnchanged check and returning early to avoid re-posting in this misconfiguration scenario.

**[MINOR]** When shared-token mode is detected, the code skips all update-in-place logic but also skips checking reviewUnchanged. This can lead to duplicate identical reviews being posted under the same login and sentinel. Consider still performing a reviewUnchanged check and returning early to avoid re-posting in this misconfiguration scenario.
aweiker dismissed aweiker's review 2026-05-02 14:24:11 +00:00
rodin added 1 commit 2026-05-02 14:25:46 +00:00
ci: use separate SECURITY_REVIEW_TOKEN for security reviewer
CI / test (pull_request) Successful in 13s
Details
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 23s
Details
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 41s
Details
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 58s
Details
ceefa4c2e0 
The security-review-bot Gitea user now has its own token. This
completes the token separation so each reviewer role posts under
its own identity, enabling native Gitea multi-reviewer blocking.
security-review-bot approved these changes 2026-05-02 14:26:43 +00:00
security-review-bot left a comment
Collaborator
Copy Link
Copy Source

Security Review

Summary

The changes remove the worst-wins escalation logic and adjust the CI secret for the security reviewer, replacing it with a safer shared-token detection that avoids clobbering reviews. No new security risks are introduced and existing input validations (e.g., reviewer-name, system prompt path checks) remain robust.

Recommendation

APPROVE — Proceed with merging. The removal of escalation logic simplifies behavior without weakening security, and the added shared-token detection helps prevent unintended review edits in misconfiguration scenarios. Ensure the CI environment is updated to provide SECURITY_REVIEW_TOKEN as noted in the deployment instructions.

Review by security

# Security Review ## Summary The changes remove the worst-wins escalation logic and adjust the CI secret for the security reviewer, replacing it with a safer shared-token detection that avoids clobbering reviews. No new security risks are introduced and existing input validations (e.g., reviewer-name, system prompt path checks) remain robust. ## Recommendation **APPROVE** — Proceed with merging. The removal of escalation logic simplifies behavior without weakening security, and the added shared-token detection helps prevent unintended review edits in misconfiguration scenarios. Ensure the CI environment is updated to provide SECURITY_REVIEW_TOKEN as noted in the deployment instructions. --- *Review by security* <!-- review-bot:security -->
sonnet-review-bot approved these changes 2026-05-02 14:27:00 +00:00
sonnet-review-bot left a comment
First-time contributor
Copy Link
Copy Source

Sonnet Review

Summary

The changes cleanly remove the worst-wins escalation logic and replace it with a simpler, safer shared-token detection, aligning with the new per-role token model. Tests and CI config were updated accordingly, and CI passed.

Findings

# Severity File Line Finding
1 [MINOR] cmd/review-bot/main.go 494 hasSharedToken does not filter out stale reviews when detecting shared-token misuse. If a stale sibling review from a prior misconfiguration remains under the same user, this may trigger a false positive and permanently skip update-in-place logic even after the configuration is fixed. Consider ignoring r.Stale in the detection loop.

Recommendation

APPROVE — The removal of pre/post escalation logic is coherent with the new design where each reviewer role uses a distinct Gitea user, allowing native multi-reviewer blocking. The update-in-place logic remains intact and guarded by a sensible shared-token detection. Tests have been adjusted and expanded to cover the new helpers, and CI passed. As a minor improvement, consider filtering out stale reviews in hasSharedToken to avoid false positives when old reviews from a previous shared-token misconfiguration linger. Otherwise, the PR looks solid and ready to merge.

Review by sonnet

# Sonnet Review ## Summary The changes cleanly remove the worst-wins escalation logic and replace it with a simpler, safer shared-token detection, aligning with the new per-role token model. Tests and CI config were updated accordingly, and CI passed. ## Findings | # | Severity | File | Line | Finding | |---|----------|------|------|--------| | 1 | [MINOR] | `cmd/review-bot/main.go` | 494 | hasSharedToken does not filter out stale reviews when detecting shared-token misuse. If a stale sibling review from a prior misconfiguration remains under the same user, this may trigger a false positive and permanently skip update-in-place logic even after the configuration is fixed. Consider ignoring r.Stale in the detection loop. | ## Recommendation **APPROVE** — The removal of pre/post escalation logic is coherent with the new design where each reviewer role uses a distinct Gitea user, allowing native multi-reviewer blocking. The update-in-place logic remains intact and guarded by a sensible shared-token detection. Tests have been adjusted and expanded to cover the new helpers, and CI passed. As a minor improvement, consider filtering out stale reviews in hasSharedToken to avoid false positives when old reviews from a previous shared-token misconfiguration linger. Otherwise, the PR looks solid and ready to merge. --- *Review by sonnet* <!-- review-bot:sonnet -->
cmd/review-bot/main.go
@@ -539,6 +494,46 @@ func reviewUnchanged(reviews []gitea.Review, newBody, newEvent, sentinel string)
return false
sonnet-review-bot commented 2026-05-02 14:27:00 +00:00
First-time contributor
Copy Link
Copy Source

[MINOR] hasSharedToken does not filter out stale reviews when detecting shared-token misuse. If a stale sibling review from a prior misconfiguration remains under the same user, this may trigger a false positive and permanently skip update-in-place logic even after the configuration is fixed. Consider ignoring r.Stale in the detection loop.

**[MINOR]** hasSharedToken does not filter out stale reviews when detecting shared-token misuse. If a stale sibling review from a prior misconfiguration remains under the same user, this may trigger a false positive and permanently skip update-in-place logic even after the configuration is fixed. Consider ignoring r.Stale in the detection loop.
rodin merged commit b51a19d8b9 into main 2026-05-02 16:46:06 +00:00
rodin deleted branch fix/28-remove-escalation 2026-05-02 16:46:06 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
gpt-review-bot
aweiker
security-review-bot
sonnet-review-bot
Labels
Clear labels
ai-review
bug
needs-review
ready
Ready for human review
 self-reviewed
Self-review passed since last push
 wip
Work in progress - do not start new work on this PR
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
security-review-bot rodin (Rodin)
No Assignees
5 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#31
Delete Branch "fix/28-remove-escalation"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?