Fix #43: MemOS CLI 记忆新增后立即查询不可见 - #44
Open
Memtensor-AI wants to merge 2 commits into
Open
Conversation
… list alias Add client-side polling of /get/status inside memos add so the CLI returns only after the async pipeline reaches a terminal state, restoring the "add then list" mental model. New --wait / --no-wait / --wait-timeout options let automation opt out. Register memos list as an alias of memos get to match documented usage and avoid the "command not found" error that skill middleware surfaces as "memory not found." Fixes MemTensor#43.
Collaborator
Author
🤖 Open Code ReviewTarget: PR #44 ✅ OpenCodeReview: Review complete: 0 finding(s) across 5 selected item(s). Generated by cloud-assistant via Open Code Review. |
Collaborator
Author
🔧 Open Code Review requested Agent fixOpen Code Review found 6 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
Address open code review findings from PR MemTensor#44: - memory_cmd._poll_task_status now logs swallowed backend.get_status exceptions at WARNING so a broken /get/status endpoint or a latent bug in the backend client stays diagnosable instead of returning a silent empty payload. (fingerprints MemTensor#1, MemTensor#3) - cmd_add treats wait_timeout <= 0 as an explicit opt-out of waiting so we no longer advertise waited=True while _poll_task_status immediately returns without a single status call. The reported waited flag now reflects whether polling actually ran. (fingerprint MemTensor#2) - format_add_result merges the two duplicate success arms ("completed" vs {"success","succeeded","done"}) into a single branch; the split carried no observable difference. (fingerprint MemTensor#4) - _build_add_success_context skips the "poll memos status" hint when the task is already in a terminal-failure state (failed / error) so agents don't get told to poll a dead task. (fingerprint MemTensor#5) - _build_agent_payload("add", ...) now round-trips `waited` in the returned dict, symmetrical with task_id and final_status, so downstream consumers can distinguish "waited to completion" from "fired and forgotten". (fingerprint MemTensor#6) Tests added: - transport error is emitted via memory_cmd.logger at WARNING - wait=True + wait_timeout=0 skips polling and reports waited=False - all four success-status variants print the success line - terminal-failure status omits the poll hint - add payload includes waited=True/False Refs MemTensor#43.
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
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.
Description
Fixes MemOS-Cloud-CLI#43. Root cause is the same class as archived issue #39: POST /add/message is asynchronous — the server returns data.status="running" + data.task_id — but cmd_add ignored task_id, returned "success" immediately, and memos list wasn't even registered as a command. The intermediate hooks/skills refactor (#38) had never folded in the #39 sync-wait patch, so the symptom regressed and users saw "add succeeds, list returns empty."
The fix ports the #39 design to the current codebase: cmd_add now extracts task_id and polls /get/status (already exposed as memos status) until a terminal state or a soft --wait-timeout expires. Two new options on memos add — --wait/--no-wait (default --wait) and --wait-timeout (default 30s) — give automation an opt-out. memos list is registered as an alias of memos get so users following MemOS CLI.md no longer hit a typer usage error that skill middleware swallows as "memory not found." format_add_result and the agent-envelope success context surface task_id, final status, and a next-step hint. Behaviour on servers that omit task_id is unchanged (strict superset).
Verification: python3 -m unittest discover -s tests reports 82/82 passing (54 pre-existing + 28 new cases in tests/test_memory_add_wait.py, covering the helpers, poll loop, cmd_add wait/no-wait/no-task-id/already-completed branches, typer forwarding, list-vs-get callback identity, and formatter output for each terminal state). memos --help now lists list; memos add --help shows the new options; memos list --help mirrors memos get. No integration test against a live backend (repo has no fixture).
Files: src/memos_cli/commands/memory.py, src/memos_cli/commands/memory_cmd.py, src/memos_cli/main.py, src/memos_cli/output.py, tests/test_memory_add_wait.py. Single conventional commit; branch pushed; opsp artifacts (task.md + proposal/spec/design/verification-report) synced to memos-autodev-specs main.
Related Issue (Required): Fixes #43
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Not run; documentation-only change.
Checklist
@ljjicode please review this PR.
Reviewer Checklist