Add wiki save base version option#12
Merged
Merged
Conversation
WalkthroughPR은 wiki save 명령에 optimistic concurrency control을 위한 --base-version 옵션을 추가합니다. 양의 정수 파싱, PATCH 기반 페이지 업데이트 요청, 409 conflict 에러 처리, 그리고 동시성 규약 문서화를 구현합니다. ChangesWiki save optimistic concurrency control
Sequence DiagramsequenceDiagram
participant Client as CLI Client
participant Agent as Wiki Agent
participant Server as Craken Service
rect rgba(100, 150, 200, 0.5)
Note over Client,Server: Update existing wiki page with base version
Client->>Agent: wiki save --existing-title Home --base-version 12
Agent->>Agent: Parse and validate base-version (positiveNumberOption)
Agent->>Server: PATCH /api/workspaces/.../wiki/pages/Home<br/>{ title, content, baseVersionNumber }
Server-->>Agent: 200 OK (success)
Agent-->>Client: Success
end
rect rgba(200, 100, 100, 0.5)
Note over Client,Server: Stale base version conflict
Client->>Agent: wiki save --existing-title Home --base-version 10
Agent->>Server: PATCH /api/workspaces/.../wiki/pages/Home<br/>{ title, content, baseVersionNumber: 10 }
Server-->>Agent: 409 Conflict { stale_base_version }
Agent-->>Client: Error: failed with 409, stale_base_version
end
rect rgba(100, 200, 100, 0.5)
Note over Client,Server: Create new wiki page without base version
Client->>Agent: wiki save --title NewPage --content-file page.md
Agent->>Server: POST /api/workspaces/.../wiki/pages<br/>{ title, content }
Server-->>Agent: 200 OK (created)
Agent-->>Client: Success
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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 docstrings
🧪 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
internal/craken/command_test.go (1)
481-511: 💤 Low value충돌 테스트에서 요청 바디 검증 추가 고려
TestWikiSaveReportsBaseVersionConflict는 409 응답 렌더링은 잘 검증하지만, PATCH 요청 시 실제로baseVersionNumber: 12가 전송됐는지는 확인하지 않습니다.TestWikiSaveAndAgentPlanCheck에서 이미 바디를 검증하므로 중복일 수 있으나, “stale conflict가 발생하려면 클라이언트가 base version을 보냈음”이라는 회귀 보호를 위해 가볍게 디코딩 후 단언 한 줄 추가를 고려해 보세요.♻️ 제안 변경
case "PATCH /api/workspaces/workspace-id/wiki/pages/Home": + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if body["baseVersionNumber"] != float64(12) { + t.Fatalf("expected baseVersionNumber 12, got %#v", body["baseVersionNumber"]) + } w.WriteHeader(http.StatusConflict)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/craken/command_test.go` around lines 481 - 511, In TestWikiSaveReportsBaseVersionConflict add a lightweight assertion in the HTTP test server handler for the "PATCH /api/workspaces/workspace-id/wiki/pages/Home" case: decode the incoming request body (JSON), assert that the payload contains the expected baseVersionNumber (12) and/or baseVersion field sent by Run, and fail the test on mismatch; this ensures TestWikiSaveReportsBaseVersionConflict (and its handler switch case) verifies the client actually sends the base version without duplicating heavy validations from TestWikiSaveAndAgentPlanCheck.internal/craken/util.go (1)
123-133: 💤 Low value에러 메시지에 옵션 이름 포함 고려
expected positive integer, got %s만 출력하면 어떤 플래그가 잘못됐는지 사용자에게 명확하지 않습니다. 호출 측에서--base-version외에도 재사용될 가능성이 있으므로,name인자를 메시지에 포함하면 진단성이 개선됩니다.♻️ 제안 변경
func positiveNumberOption(cmd command, name string) (*int, error) { value := cmd.string(name, "") if value == "" { return nil, nil } parsed, err := strconv.Atoi(value) if err != nil || parsed < 1 { - return nil, fmt.Errorf("expected positive integer, got %s", value) + return nil, fmt.Errorf("--%s expects a positive integer, got %q", name, value) } return &parsed, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/craken/util.go` around lines 123 - 133, The error message in positiveNumberOption does not include which CLI option failed; update the error returned from positiveNumberOption(cmd, name) to include the name parameter (the option/flag) in the formatted message so callers see which flag produced the invalid value—e.g., change the fmt.Errorf call to include name and value and keep the existing behavior of returning nil,+error on invalid parse or non-positive numbers.internal/craken/wiki_agent.go (1)
38-56: ⚡ Quick winPATCH(기존 페이지 업데이트) 시
--base-version누락 허용 여부 재검토
--existing-title이 지정된 PATCH 경로에서baseVersionNumber가 nil인 경우 그대로 요청이 전송됩니다. 아키텍처 문서는 기존 페이지 업데이트 시latestVersionNumber를 읽어 전달하라고 명시하므로, CLI에서 base-version을 생략한 PATCH는 사실상 “blind overwrite”가 되어 동시 편집자의 작업을 덮어쓸 위험이 있습니다. 서버가 강제하지 않는다면 CLI 레벨에서라도 PATCH 시 옵션을 필수화하거나 경고를 출력하는 것을 고려해 보세요.♻️ 예시: PATCH 시 base-version 필수화
if existing := cmd.string("existing-title", ""); existing != "" { + if baseVersionNumber == nil { + return fmt.Errorf("--base-version is required when updating an existing page via --existing-title") + } return printClientJSON( ctx, client, stdout, "PATCH", wikiPagePath(workspaceID, existing), wikiSaveBody(content, cmd.string("title", existing), baseVersionNumber), ) }Also applies to: 103-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/craken/wiki_agent.go` around lines 38 - 56, For PATCH requests when --existing-title is provided, do not allow a nil baseVersionNumber: in the branch that checks cmd.string("existing-title", "") use positiveNumberOption result (baseVersionNumber) and if it's nil/empty return an error telling the user to provide --base-version (or at minimum print a warning and abort); update the logic around existing/title handling (the code calling positiveNumberOption, cmd.string("existing-title", printClientJSON, wikiPagePath, wikiSaveBody and cmd.required("title")) so PATCH always includes a concrete base version rather than sending a blind overwrite.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/craken/command_test.go`:
- Around line 481-511: In TestWikiSaveReportsBaseVersionConflict add a
lightweight assertion in the HTTP test server handler for the "PATCH
/api/workspaces/workspace-id/wiki/pages/Home" case: decode the incoming request
body (JSON), assert that the payload contains the expected baseVersionNumber
(12) and/or baseVersion field sent by Run, and fail the test on mismatch; this
ensures TestWikiSaveReportsBaseVersionConflict (and its handler switch case)
verifies the client actually sends the base version without duplicating heavy
validations from TestWikiSaveAndAgentPlanCheck.
In `@internal/craken/util.go`:
- Around line 123-133: The error message in positiveNumberOption does not
include which CLI option failed; update the error returned from
positiveNumberOption(cmd, name) to include the name parameter (the option/flag)
in the formatted message so callers see which flag produced the invalid
value—e.g., change the fmt.Errorf call to include name and value and keep the
existing behavior of returning nil,+error on invalid parse or non-positive
numbers.
In `@internal/craken/wiki_agent.go`:
- Around line 38-56: For PATCH requests when --existing-title is provided, do
not allow a nil baseVersionNumber: in the branch that checks
cmd.string("existing-title", "") use positiveNumberOption result
(baseVersionNumber) and if it's nil/empty return an error telling the user to
provide --base-version (or at minimum print a warning and abort); update the
logic around existing/title handling (the code calling positiveNumberOption,
cmd.string("existing-title", printClientJSON, wikiPagePath, wikiSaveBody and
cmd.required("title")) so PATCH always includes a concrete base version rather
than sending a blind overwrite.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d8929372-b4ff-483d-9daf-0c7baf0edd2c
📒 Files selected for processing (5)
README.mddocs/architecture.mdinternal/craken/command_test.gointernal/craken/util.gointernal/craken/wiki_agent.go
📜 Review details
🔇 Additional comments (3)
internal/craken/command_test.go (1)
444-451: LGTM!README.md (1)
35-35: LGTM!docs/architecture.md (1)
7-7: LGTM!
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
1 participant
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
craken wiki save --base-versionand sendbaseVersionNumberin wiki save requestsFixes #7
Depends on corca-ai/craken-agents#290
Validation
MISE_TRUSTED_CONFIG_PATHS=$PWD mise exec -- go test ./...Summary by CodeRabbit
주요 변경 사항
새로운 기능
--base-version옵션으로 페이지 버전 충돌 감지 및 재시도 가능문서화