feat: lock-management surface — unlock + promote (#391) - #395
Merged
Merged
Conversation
New UnlockResult dataclass and unlock() function. Clears lock_level, locked_at, and demotion_pressure without touching origin. Idempotent — re-unlock of a non-locked belief is a no-op (no audit row, returns already_unlocked=True). Refusal: belief not found → ValueError. Writes lock:unlock audit row via insert_feedback_event on the active path.
…ity (#391) Both cli._cmd_demote and tool_demote previously cleared lock fields inline without writing an audit row. Now both delegate to unlock() so the lock:unlock event is recorded in feedback_history. Behavior is otherwise unchanged — demote still prints "demoted:" and returns the same dict shape.
unlock: explicit lock-drop with lock:unlock audit row. Idempotent — already-unlocked belief returns exit 0 with "already unlocked:" message. promote: user-facing alias of validate; routes to the same handler (_cmd_validate) and accepts the same --source flag. Both subcommands are visible in help (not hidden like validate/demote).
tool_unlock: pure handler, idempotent. Returns {kind, id, unlocked,
audit_event_id?} on active path; {kind, id, unlocked=False} on
already-unlocked; {kind, id, unlocked=False, error} on not-found.
tool_promote: alias of tool_validate, identical semantics and return
shape. Both registered with @mcp.tool() alongside their peers.
31 tests covering: unlock() idempotency + field clearing + audit row + error path; demote lock-drop audit row regression; promote/validate CLI parity via file-backed stores; tool_unlock + tool_promote MCP shape parity; lock state machine round-trip (locked → unlocked → re-locked).
COMMANDS.md: add unlock and promote rows with idempotency and audit-row callouts; clarify demote delegates to unlock for parity. MCP.md: bump tool count 9→11, add aelf:unlock + aelf:promote rows with return-shape columns, expand footer note.
…les (#391) New slash_commands/unlock.md and promote.md following the existing format. Register both in tests/test_slash_commands.py EXPECTED_COMMANDS so the slash/CLI parity test passes.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR implements lock-management operations ( ChangesLock Management Surface
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 54 minutes and 34 seconds.Comment |
Reviewer's GuideExtends the lock-management surface by introducing a shared unlock primitive (with audit logging) and a promote alias, wiring them through CLI, MCP tools, docs, and tests, while refactoring demote to use the new unlock path for consistent behavior and audit coverage. Sequence diagram for CLI unlock and demote lock-drop using shared unlock primitivesequenceDiagram
actor User
participant CLI_unlock_cmd as CLI _cmd_unlock
participant CLI_demote_cmd as CLI _cmd_demote
participant Promotion_unlock as promotion.unlock
participant MemoryStore as MemoryStore
participant FeedbackStore as feedback_history
User->>CLI_unlock_cmd: run aelf unlock belief_id
CLI_unlock_cmd->>MemoryStore: _open_store()
activate MemoryStore
CLI_unlock_cmd->>Promotion_unlock: unlock(store, belief_id)
alt belief_not_found
Promotion_unlock->>MemoryStore: get_belief(belief_id) -> None
Promotion_unlock-->>CLI_unlock_cmd: raise ValueError
CLI_unlock_cmd-->>User: stderr "belief not found" (exit 1)
else already_unlocked
Promotion_unlock->>MemoryStore: get_belief(belief_id) -> belief(lock_level != LOCK_USER)
Promotion_unlock-->>CLI_unlock_cmd: UnlockResult(already_unlocked=True, audit_event_id=None)
CLI_unlock_cmd-->>User: stdout "already unlocked" (exit 0)
else active_unlock
Promotion_unlock->>MemoryStore: get_belief(belief_id) -> belief(lock_level == LOCK_USER)
Promotion_unlock->>MemoryStore: update_belief(cleared lock_level, locked_at, demotion_pressure)
Promotion_unlock->>FeedbackStore: insert_feedback_event(source=SOURCE_LOCK_UNLOCK, valence=0.0)
FeedbackStore-->>Promotion_unlock: audit_event_id
Promotion_unlock-->>CLI_unlock_cmd: UnlockResult(already_unlocked=False, audit_event_id)
CLI_unlock_cmd-->>User: stdout "unlocked" (exit 0)
end
CLI_unlock_cmd->>MemoryStore: close()
deactivate MemoryStore
User->>CLI_demote_cmd: run aelf demote belief_id
CLI_demote_cmd->>MemoryStore: _open_store()
activate MemoryStore
CLI_demote_cmd->>MemoryStore: get_belief(belief_id)
alt lock_level == LOCK_USER
CLI_demote_cmd->>Promotion_unlock: unlock(store, belief_id)
Promotion_unlock-->>CLI_demote_cmd: UnlockResult(...)
CLI_demote_cmd-->>User: stdout "demoted" (lock tier only)
else origin == ORIGIN_USER_VALIDATED
CLI_demote_cmd->>Promotion_unlock: devalidate(store, belief_id)
CLI_demote_cmd-->>User: stdout "demoted" (origin tier)
else
CLI_demote_cmd-->>User: stdout "no-op demote" (already lowest tier)
end
CLI_demote_cmd->>MemoryStore: close()
deactivate MemoryStore
Sequence diagram for MCP aelf_unlock and aelf_promote toolssequenceDiagram
actor Agent
participant MCP as MCP_server
participant Tool_unlock as tool_unlock
participant Tool_promote as tool_promote
participant Promotion_unlock as promotion.unlock
participant Tool_validate as tool_validate
participant MemoryStore as MemoryStore
Agent->>MCP: call aelf_unlock(belief_id)
MCP->>Tool_unlock: tool_unlock(store, belief_id)
Tool_unlock->>Promotion_unlock: unlock(store, belief_id)
alt belief_not_found
Promotion_unlock-->>Tool_unlock: raise ValueError
Tool_unlock-->>Agent: {kind: unlock.not_found, id, unlocked: false, error}
else already_unlocked
Promotion_unlock-->>Tool_unlock: UnlockResult(already_unlocked=True, audit_event_id=None)
Tool_unlock-->>Agent: {kind: unlock.already, id, unlocked: false}
else unlocked
Promotion_unlock-->>Tool_unlock: UnlockResult(already_unlocked=False, audit_event_id)
Tool_unlock-->>Agent: {kind: unlock.unlocked, id, unlocked: true, audit_event_id}
end
Agent->>MCP: call aelf_promote(belief_id, source)
MCP->>Tool_promote: tool_promote(store, belief_id, source)
Tool_promote->>Tool_validate: tool_validate(store, belief_id, source)
Tool_validate-->>Agent: {kind, id, prior_origin, new_origin, audit_event_id?}
Class diagram for promotion results and unlock primitiveclassDiagram
class PromotionResult {
+str belief_id
+str prior_origin
+str new_origin
+int audit_event_id
+bool already_validated
}
class UnlockResult {
+str belief_id
+bool already_unlocked
+int audit_event_id
}
class PromotionModule {
+promote(store, belief_id, source_label, now) PromotionResult
+devalidate(store, belief_id, source_label, now) PromotionResult
+unlock(store, belief_id, source_label, now) UnlockResult
}
class MemoryStore {
+get_belief(belief_id) Belief
+update_belief(belief)
+insert_feedback_event(belief_id, valence, source, created_at) int
}
class Belief {
+str id
+int lock_level
+str origin
+str locked_at
+int demotion_pressure
}
class Constants {
+int LOCK_NONE
+int LOCK_USER
+str SOURCE_PROMOTION_USER_VALIDATED
+str SOURCE_REVERT_TO_AGENT_INFERRED
+str SOURCE_LOCK_UNLOCK
}
PromotionModule ..> UnlockResult : returns
PromotionModule ..> PromotionResult : returns
PromotionModule ..> MemoryStore : uses
MemoryStore o--> Belief : stores
PromotionModule ..> Constants : uses constants
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The MCP docs now describe
aelf:validate/aelf:promoteas returning{prior_origin, new_origin, audit_event_id?}, but the existingtool_validateimplementation still appears to return a singleoriginfield; consider updating either the implementation or the docs so the response shape matches exactly. - When
tool_demotefollows the lock-drop path it now delegates tounlock()but discards theaudit_event_id; if consumers might care about the audit row for symmetry withtool_unlock, consider including that ID in thedemote.demotedresponse.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The MCP docs now describe `aelf:validate`/`aelf:promote` as returning `{prior_origin, new_origin, audit_event_id?}`, but the existing `tool_validate` implementation still appears to return a single `origin` field; consider updating either the implementation or the docs so the response shape matches exactly.
- When `tool_demote` follows the lock-drop path it now delegates to `unlock()` but discards the `audit_event_id`; if consumers might care about the audit row for symmetry with `tool_unlock`, consider including that ID in the `demote.demoted` response.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Owner
Author
|
[claim:review:Toug:2026-05-03T20:51:10Z] |
Owner
Author
|
[claim:review:Setr:2026-05-03T20:51:38Z] |
Owner
Author
|
[release:review:Setr:2026-05-03T20:51:43Z] |
robotrocketscience
enabled auto-merge (rebase)
May 3, 2026 20:55
auto-merge was automatically disabled
May 3, 2026 20:56
Branch protection rule check failed
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/MCP.md`:
- Around line 39-40: Update the MCP docs table entries for the `aelf:validate`
and `aelf:promote` return shapes to be a union that explicitly includes both the
success object `{kind, id, prior_origin, new_origin, audit_event_id?}` and the
error variant `{kind: "validate.error", error}`; mention that the error variant
is returned on invalid requests so clients can parse the union safely for
`aelf:validate` and `aelf:promote`.
- Line 45: Update the wording for the aelf:unlock description so it no longer
claims it “always writes” a lock:unlock audit row; change the phrase to indicate
that a lock:unlock audit row is written only when a lock is actually removed
(i.e., not in the already-unlocked idempotent path). Reference the existing term
"aelf:unlock" and the audit row name "lock:unlock" when making this single-line
edit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fb088468-c9e3-401b-ba97-00266951bbec
📒 Files selected for processing (9)
docs/COMMANDS.mddocs/MCP.mdsrc/aelfrice/cli.pysrc/aelfrice/mcp_server.pysrc/aelfrice/promotion.pysrc/aelfrice/slash_commands/promote.mdsrc/aelfrice/slash_commands/unlock.mdtests/test_lock_management.pytests/test_slash_commands.py
… path (CodeRabbit)
robotrocketscience
enabled auto-merge (rebase)
May 3, 2026 21:00
auto-merge was automatically disabled
May 3, 2026 21:01
Branch protection rule check failed
Owner
Author
|
[release:review:Toug:2026-05-03T21:07:28Z] |
robotrocketscience
added
the
attn:review
This was referenced May 9, 2026
Merged
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
2 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.
Summary
Closes #391 (Track E of #382). Extends the lock-management surface
with
unlockandpromote(CLI + MCP), and gives the existingdemotelock-clearing path an audit row for parity.aelf unlock <belief_id>/aelf_unlockMCP tool — pure inverseof
aelf lock. Clearslock_level/locked_at/demotion_pressure; does not touch origin. Idempotent(re-unlock of an unlocked belief is a no-op, no audit row).
aelf promote <belief_id>/aelf_promoteMCP tool — alias ofaelf validate. Functionally identical; both forms ship forsurface-discoverability per the issue.
demotelock-clearing path now writes alock:unlockaudit row(was previously silent). Devalidate path unchanged
(
promotion:revert_to_agent_inferred).promotion.unlock(store, belief_id, *, source_label, now)alongside
promote()/devalidate(). ReturnsUnlockResult.Refuses (
ValueError) on belief-not-found.Audit-row inventory after this PR
aelf lock(re-assert)aelf unlock/demote-lock-droplock:unlock(new)aelf promote/aelf validatepromotion:user_validateddemote-devalidatepromotion:revert_to_agent_inferredTest plan
tests/test_lock_management.py— 31 tests:idempotency, error paths, lock-state machine round-trip
(lock → unlock → re-lock), CLI parity (
unlock≡demote-lock-drop),
validate≡promote, MCP shape parity.Out of scope
aelf locksemantics.demote-devalidate path (already audited).aelf reason/aelf wonder(siblingsof [v2.0] graph + UX bucket — 6 edges (per-edge bench-gated), aelf reason/wonder, aelf health, polymorphic onboard, MCP ports #382).
Rollback
git revert <pr-merge-sha>— schema-additive only (new audit rowsource label), no migrations, no shipped consumers depend on the
new label yet.
Summary by Sourcery
Extend lock management with explicit unlock and promote operations across CLI, MCP tools, and audit logging, and add comprehensive acceptance tests for the lock state machine.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
Release Notes
New Features
unlockcommand to remove user-locks from beliefs while preserving their origin tier and audit trail.promotecommand as an alias tovalidatefor promoting agent-inferred beliefs to user-validated status.aelf:unlockandaelf:promotefor programmatic lock and promotion management.Bug Fixes
demotecommand to properly log unlock operations through audit events.Documentation
unlockandpromote.Tests