feat: choose project knowledge storage during setup - #2
Conversation
Not-tested: formatter, lint, build, and tests per task constraint
Not-tested: formatter, lint, build, and tests per task constraint
Not-tested: formatter, lint, build, and tests per task constraint
Not-tested: formatter, lint, build, and tests per task constraint
Not-tested: formatter, lint, build, and tests per task constraint
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change adds safe bounded file operations, Git exclusion management for project ChangesProject knowledge safety
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Setup
participant GitExclude
participant Git
CLI->>Setup: resolve project knowledge mode
Setup->>GitExclude: plan_git_exclude(project_root, mode)
GitExclude->>Git: inspect repository and ignore policy
CLI->>Setup: apply_setup(plan, approved)
Setup->>GitExclude: apply_git_exclude(plan)
Setup->>CLI: return postcheck notices
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
코드 리뷰 결과: 수정 필요검토 범위: Findings
확인한 근거
Verdict: REQUEST_CHANGES |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
src/didimlog/project/git_exclude.py (2)
195-202: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAnnotate the
cwdparameter.Every other public function in this module is annotated.
cwdacceptsNone, astr, or aPath.♻️ Proposed fix
-def discover_project_for_setup(cwd) -> Path | None: +def discover_project_for_setup(cwd: str | os.PathLike[str] | None) -> Path | None:🤖 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 `@src/didimlog/project/git_exclude.py` around lines 195 - 202, Annotate the cwd parameter of discover_project_for_setup as accepting None, str, or Path, while preserving its existing return annotation and behavior.
222-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the no-op
try/except.The
except DidimError: raiseclause re-raises without any change. It has no effect.♻️ Proposed fix
- try: - root = _strict_path(result.stdout) - except DidimError: - raise + root = _strict_path(result.stdout) candidate = _root_candidate(source, root)🤖 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 `@src/didimlog/project/git_exclude.py` around lines 222 - 225, Remove the no-op try/except surrounding _strict_path(result.stdout) in the relevant function, leaving the call and its existing DidimError propagation unchanged.src/didimlog/conditional_file.py (2)
259-261: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueClose the parent descriptor even if the lock close fails.
If
os.close(lock_descriptor)raises,os.close(parent_descriptor)never runs and the descriptor leaks.♻️ Proposed fix
- if lock_descriptor is not None: - os.close(lock_descriptor) - os.close(parent_descriptor) + try: + if lock_descriptor is not None: + os.close(lock_descriptor) + finally: + os.close(parent_descriptor)🤖 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 `@src/didimlog/conditional_file.py` around lines 259 - 261, Update the descriptor cleanup block in the surrounding function so closing lock_descriptor cannot prevent closing parent_descriptor; ensure os.close(parent_descriptor) always executes even when os.close(lock_descriptor) raises, while preserving the conditional lock cleanup.
166-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument that a create can succeed while the call reports an error.
After
os.linkpublishes the new inode, a failure ofos.unlinkoros.fsyncstill producesValueError("target could not be written atomically"). The target file exists with the intended bytes at that point.tests/didimlog_tests/test_conditional_file.pylines 229-332 confirm this behavior. Callers such asapply_git_excludeinsrc/didimlog/project/git_exclude.pyline 559 convert theValueErrorintoPROJECT_EXCLUDE_CHANGED, which reads as "nothing was written". State the post-publication semantics in the docstring so callers do not assume a failed call means no change.📝 Proposed docstring change
- """Publish intended bytes only while the planned regular file is unchanged.""" + """Publish intended bytes only while the planned regular file is unchanged. + + An error after publication is possible. If the new inode is already linked + and a later cleanup or directory sync fails, this function raises + ``ValueError`` while the target keeps the intended bytes. + """Also applies to: 244-261
🤖 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 `@src/didimlog/conditional_file.py` around lines 166 - 178, Update the docstring of write_regular_file_if_unchanged to state that the call may raise ValueError after os.link has published the intended bytes, including when cleanup or fsync fails, so the target can exist despite the reported error. Preserve the existing validation and atomic-write behavior.tests/didimlog_tests/test_conditional_file.py (1)
29-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd coverage for
maximum_bytesvalidation.
read_optional_regular_filerejects a non-integermaximum_byteswithTypeError, rejectsbool, and rejects a negative value withValueError(src/didimlog/conditional_file.pylines 153-156). No test exercises these branches.💚 Proposed test
def test_invalid_maximum_bytes_is_refused(self): with tempfile.TemporaryDirectory() as temporary_directory: target = Path(temporary_directory) / "target" target.write_bytes(b"bytes") for invalid in ("64", 1.0, True): with self.subTest(invalid=invalid), self.assertRaises(TypeError): read_optional_regular_file(target, invalid) with self.assertRaises(ValueError): read_optional_regular_file(target, -1)🤖 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 `@tests/didimlog_tests/test_conditional_file.py` around lines 29 - 35, Add a test alongside test_file_larger_than_size_limit_is_refused covering read_optional_regular_file validation: assert TypeError for string, float, and bool maximum_bytes values, and assert ValueError for a negative value. Use a temporary target file and subtests for the TypeError cases.tests/didimlog_tests/project/test_git_exclude.py (1)
166-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a size-boundary case to the round-trip tests.
No test covers an
info/excludefile whose size is near_MAXIMUM_EXCLUDE_BYTES. That is the case in whichlocalmode produces content larger than the read limit and makes later planning fail. See the related comment onsrc/didimlog/project/git_exclude.pylines 350-375.💚 Proposed test
def test_local_refuses_an_exclude_file_at_the_size_limit(self): path = self.exclude_path() path.write_bytes(b"#" * (1024 * 1024 - 1) + b"\n") self.assert_token( "PROJECT_EXCLUDE_UNSAFE", lambda: self.call(plan_git_exclude, self.project, "local"), ) self.assertEqual(len(path.read_bytes()), 1024 * 1024)🤖 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 `@tests/didimlog_tests/project/test_git_exclude.py` around lines 166 - 186, Add a dedicated boundary test near test_local_and_shared_round_trip_exact_user_bytes that writes an info/exclude file exactly _MAXIMUM_EXCLUDE_BYTES bytes using the proposed near-limit content, asserts plan_git_exclude in local mode raises PROJECT_EXCLUDE_UNSAFE via assert_token, and verifies the file size remains unchanged.README.md (1)
44-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider listing the Git exclude stage in the numbered steps.
apply_setupruns the Git exclude stage between the project index stage and the Claude stage. The numbered list at Lines 43-46 does not include it, so the list does not match the actual execution order. Line 50 explains the behavior but not its position in the sequence.Add the exclude stage as its own numbered step, or state in the list that step 2 also applies the local exclude rule.
🤖 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 `@README.md` around lines 44 - 51, Update the numbered setup steps in the README to explicitly include the Git local exclude rule stage between preparing the project knowledge/index and configuring Claude Code, matching the execution order of apply_setup; keep the existing explanation of info/exclude behavior unchanged.tests/didimlog_tests/claude/test_setup_plan.py (1)
9-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the shared Git test harness.
GIT,START,RULE,END,LOCAL_BLOCK,git_environment,_git, and_exclude_pathare duplicated between this file andtests/didimlog_tests/claude/test_setup_apply.py. The exclusion-block constants encode the producer contract ofgit_exclude.py. If that block format changes, two copies must be updated.Move the constants and the Git helpers into one shared test-support module, for example
tests/didimlog_tests/git_support.py, and import them in both test files.Also applies to: 36-41, 87-115
🤖 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 `@tests/didimlog_tests/claude/test_setup_plan.py` around lines 9 - 18, Extract the duplicated Git harness symbols GIT, START, RULE, END, LOCAL_BLOCK, git_environment, _git, and _exclude_path from both test_setup_plan.py and test_setup_apply.py into a shared test-support module. Import those symbols from the shared module in both test files, preserving the existing exclusion-block values and helper behavior.
🤖 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.
Inline comments:
In `@README.md`:
- Around line 52-58: Update the README.md `shared` project-knowledge setup
example to include the `--yes` approval flag, matching the documented behavior
of `apply_setup` and allowing the copied command to complete without prompting.
In `@src/didimlog/claude/connect.py`:
- Around line 443-459: Update the BaseException handler surrounding the recovery
loop so resource restoration failures cannot prevent journal.rollback() from
running or replace the original exception. Ensure the managed resource directory
via _ensure_resource_directory(plan.config_dir) before recovery, tolerating
failures, and isolate each
read_optional_regular_file/write_regular_file_if_unchanged attempt so failures
are ignored; then always call journal.rollback() before re-raising the original
exception.
In `@src/didimlog/cli.py`:
- Around line 228-236: Update the storage-selection prompt loop to catch
EOFError and return the default "local" mode when input ends; also handle
KeyboardInterrupt there unless main already converts it to the CLI error
contract. Preserve the existing valid-selection behavior for blank, "1", and "2"
inputs.
- Around line 220-226: Unify project discovery so Git-unavailable projects are
represented as an unconfigured result rather than raising. Update
_project_knowledge_mode and plan_setup to reuse the same discovery behavior as
indexing._discover_git_root, preserving the distinction between Git being
unavailable and the directory being outside a Git project. Add coverage for
setup, status, doctor, and index when a .git marker exists but Git cannot run.
In `@src/didimlog/conditional_file.py`:
- Around line 214-226: The write_regular_file_if_unchanged publication path must
distinguish successful inode publication from subsequent cleanup or fsync
failure; document these post-publication semantics in its docstring or return a
status that separates publication from cleanup. In
src/didimlog/conditional_file.py lines 214-226, update
write_regular_file_if_unchanged accordingly. In
src/didimlog/project/git_exclude.py lines 534-569, stop mapping every ValueError
to PROJECT_EXCLUDE_CHANGED: re-read the exclude file before selecting that
token, or revise the help text so it does not imply the previous run made no
change.
- Around line 214-220: Update the exception handling around the os.link call to
also catch NotImplementedError and map it to the existing atomic-write
ValueError contract. Preserve follow_symlinks=False in the os.link invocation
and leave the current ValueError/OSError handling unchanged.
In `@src/didimlog/project/git_exclude.py`:
- Around line 478-503: In _build_plan, validate intended before returning
GitExcludePlan and raise the established refusal error when its byte length
exceeds _MAXIMUM_EXCLUDE_BYTES. Add a test in
tests/didimlog_tests/project/test_git_exclude.py covering an info/exclude file
exactly at the limit, asserting local planning is refused and the file remains
unchanged.
In `@tests/didimlog_tests/test_cli_commands.py`:
- Around line 124-132: Update
test_setup_help_registers_exact_project_knowledge_option to pin the help-output
width before invoking the CLI, using the test module’s environment handling and
importing os if needed; restore the prior COLUMNS value afterward so the test
remains isolated while keeping the existing metavar assertion unchanged.
---
Nitpick comments:
In `@README.md`:
- Around line 44-51: Update the numbered setup steps in the README to explicitly
include the Git local exclude rule stage between preparing the project
knowledge/index and configuring Claude Code, matching the execution order of
apply_setup; keep the existing explanation of info/exclude behavior unchanged.
In `@src/didimlog/conditional_file.py`:
- Around line 259-261: Update the descriptor cleanup block in the surrounding
function so closing lock_descriptor cannot prevent closing parent_descriptor;
ensure os.close(parent_descriptor) always executes even when
os.close(lock_descriptor) raises, while preserving the conditional lock cleanup.
- Around line 166-178: Update the docstring of write_regular_file_if_unchanged
to state that the call may raise ValueError after os.link has published the
intended bytes, including when cleanup or fsync fails, so the target can exist
despite the reported error. Preserve the existing validation and atomic-write
behavior.
In `@src/didimlog/project/git_exclude.py`:
- Around line 195-202: Annotate the cwd parameter of discover_project_for_setup
as accepting None, str, or Path, while preserving its existing return annotation
and behavior.
- Around line 222-225: Remove the no-op try/except surrounding
_strict_path(result.stdout) in the relevant function, leaving the call and its
existing DidimError propagation unchanged.
In `@tests/didimlog_tests/claude/test_setup_plan.py`:
- Around line 9-18: Extract the duplicated Git harness symbols GIT, START, RULE,
END, LOCAL_BLOCK, git_environment, _git, and _exclude_path from both
test_setup_plan.py and test_setup_apply.py into a shared test-support module.
Import those symbols from the shared module in both test files, preserving the
existing exclusion-block values and helper behavior.
In `@tests/didimlog_tests/project/test_git_exclude.py`:
- Around line 166-186: Add a dedicated boundary test near
test_local_and_shared_round_trip_exact_user_bytes that writes an info/exclude
file exactly _MAXIMUM_EXCLUDE_BYTES bytes using the proposed near-limit content,
asserts plan_git_exclude in local mode raises PROJECT_EXCLUDE_UNSAFE via
assert_token, and verifies the file size remains unchanged.
In `@tests/didimlog_tests/test_conditional_file.py`:
- Around line 29-35: Add a test alongside
test_file_larger_than_size_limit_is_refused covering read_optional_regular_file
validation: assert TypeError for string, float, and bool maximum_bytes values,
and assert ValueError for a negative value. Use a temporary target file and
subtests for the TypeError cases.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 642b838a-4f8f-4185-8c1a-980d6bb196cc
📒 Files selected for processing (15)
CHANGELOG.mdREADME.mdsrc/didimlog/claude/config.pysrc/didimlog/claude/connect.pysrc/didimlog/claude/probe.pysrc/didimlog/claude/setup.pysrc/didimlog/cli.pysrc/didimlog/conditional_file.pysrc/didimlog/project/git_exclude.pytests/didimlog_tests/claude/test_config.pytests/didimlog_tests/claude/test_setup_apply.pytests/didimlog_tests/claude/test_setup_plan.pytests/didimlog_tests/project/test_git_exclude.pytests/didimlog_tests/test_cli_commands.pytests/didimlog_tests/test_conditional_file.py
💤 Files with no reviewable changes (2)
- tests/didimlog_tests/claude/test_config.py
- src/didimlog/claude/config.py
| 프로젝트 지식을 팀과 공유하려면 다음 명령으로 `shared`를 선택합니다. | ||
|
|
||
| ```sh | ||
| didim setup --project-knowledge shared | ||
| ``` | ||
|
|
||
| `shared`는 로컬 제외 설정에서 Didimlog 관리 표시로 둘러싼 블록만 제거하고 사용자 규칙은 바꾸지 않습니다. 같은 파일의 다른 규칙, `.gitignore`, 사용자의 전역 제외 설정 등이 `knowledge/`를 계속 제외하면 안내를 표시하므로, Git에 포함하려면 해당 규칙을 직접 바꿔야 합니다. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the approval flag to the shared example.
Every other setup example in this section uses --dry-run or --yes. The shared example at Line 55 uses neither. apply_setup raises SETUP_APPROVAL_REQUIRED without approval, so a user who copies this command does not complete the change as the surrounding text implies.
📝 Proposed fix
```sh
-didim setup --project-knowledge shared
+didim setup --yes --project-knowledge shared</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
프로젝트 지식을 팀과 공유하려면 다음 명령으로 `shared`를 선택합니다.
🤖 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 `@README.md` around lines 52 - 58, Update the README.md `shared`
project-knowledge setup example to include the `--yes` approval flag, matching
the documented behavior of `apply_setup` and allowing the copied command to
complete without prompting.
| except BaseException: | ||
| for change in reversed(deleted): | ||
| if _read_optional(change.path) is None and change.original is not None: | ||
| write_if_unchanged(change.path, None, change.original) | ||
| if ( | ||
| read_optional_regular_file( | ||
| change.path, | ||
| _MANAGED_FILE_MAXIMUM_BYTES, | ||
| ) | ||
| is None | ||
| and change.original is not None | ||
| ): | ||
| write_regular_file_if_unchanged( | ||
| change.path, | ||
| None, | ||
| change.original, | ||
| ) | ||
| journal.rollback() | ||
| raise |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guarantee journal.rollback() runs even when resource recovery fails.
The recovery loop calls read_optional_regular_file and write_regular_file_if_unchanged without error handling. Both can raise. read_optional_regular_file raises when the parent directory didimlog no longer exists, because it opens the parent directly. write_regular_file_if_unchanged raises ValueError when another process recreated the target after planning ("target was created after planning").
If either call raises, the exception escapes the except BaseException block. journal.rollback() at Line 458 never runs, so the CLAUDE.md and settings.json writes applied by _apply_writes stay in place. The recovery exception also replaces the original failure.
Separate resource recovery from journal rollback.
🛠️ Proposed fix
except BaseException:
- for change in reversed(deleted):
- if (
- read_optional_regular_file(
- change.path,
- _MANAGED_FILE_MAXIMUM_BYTES,
- )
- is None
- and change.original is not None
- ):
- write_regular_file_if_unchanged(
- change.path,
- None,
- change.original,
- )
- journal.rollback()
- raise
+ try:
+ for change in reversed(deleted):
+ if change.original is None:
+ continue
+ try:
+ if (
+ read_optional_regular_file(
+ change.path,
+ _MANAGED_FILE_MAXIMUM_BYTES,
+ )
+ is None
+ ):
+ write_regular_file_if_unchanged(
+ change.path,
+ None,
+ change.original,
+ )
+ except (OSError, ValueError):
+ continue
+ finally:
+ journal.rollback()
+ raiseNote: restoring a resource also requires the managed directory to exist. If you want recovery to survive a removed didimlog directory, call _ensure_resource_directory(plan.config_dir) before the loop and ignore its failure.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| except BaseException: | |
| for change in reversed(deleted): | |
| if _read_optional(change.path) is None and change.original is not None: | |
| write_if_unchanged(change.path, None, change.original) | |
| if ( | |
| read_optional_regular_file( | |
| change.path, | |
| _MANAGED_FILE_MAXIMUM_BYTES, | |
| ) | |
| is None | |
| and change.original is not None | |
| ): | |
| write_regular_file_if_unchanged( | |
| change.path, | |
| None, | |
| change.original, | |
| ) | |
| journal.rollback() | |
| raise | |
| except BaseException: | |
| try: | |
| for change in reversed(deleted): | |
| if change.original is None: | |
| continue | |
| try: | |
| if ( | |
| read_optional_regular_file( | |
| change.path, | |
| _MANAGED_FILE_MAXIMUM_BYTES, | |
| ) | |
| is None | |
| ): | |
| write_regular_file_if_unchanged( | |
| change.path, | |
| None, | |
| change.original, | |
| ) | |
| except (OSError, ValueError): | |
| continue | |
| finally: | |
| journal.rollback() | |
| raise |
🤖 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 `@src/didimlog/claude/connect.py` around lines 443 - 459, Update the
BaseException handler surrounding the recovery loop so resource restoration
failures cannot prevent journal.rollback() from running or replace the original
exception. Ensure the managed resource directory via
_ensure_resource_directory(plan.config_dir) before recovery, tolerating
failures, and isolate each
read_optional_regular_file/write_regular_file_if_unchanged attempt so failures
are ignored; then always call journal.rollback() before re-raising the original
exception.
| def _project_knowledge_mode(args) -> str: | ||
| if args.project_knowledge is not None: | ||
| return args.project_knowledge | ||
| if args.dry_run or args.yes or not sys.stdin.isatty(): | ||
| return "local" | ||
| if discover_project_for_setup(None) is None: | ||
| return "local" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare how each command determines the Git project and handles an unavailable git executable.
set -euo pipefail
rg -n -C4 'discover_project_for_setup|PROJECT_EXCLUDE_GIT_UNAVAILABLE' --type=pyRepository: zhsks311/didimlog
Length of output: 155
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files | sed -n '1,160p'
printf '%s\n' '--- relevant symbols ---'
rg -n -C5 'discover_project_for_setup|PROJECT_EXCLUDE_GIT_UNAVAILABLE|_project_knowledge_mode|plan_setup' . || trueRepository: zhsks311/didimlog
Length of output: 44306
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- cli structure ---'
ast-grep outline src/didimlog/cli.py
printf '%s\n' '--- command handlers and dispatch ---'
rg -n -C8 '^def _|status_text|doctor_text|run_index|discover_project_for_setup|plan_setup\(' src/didimlog/cli.py
printf '%s\n' '--- Git discovery implementation ---'
cat -n src/didimlog/project/git_exclude.py | sed -n '180,245p'
printf '%s\n' '--- status and indexing implementations ---'
cat -n src/didimlog/claude/status.py | sed -n '1,240p'
cat -n src/didimlog/indexing.py | sed -n '1,220p'Repository: zhsks311/didimlog
Length of output: 27347
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- setup planning path ---'
cat -n src/didimlog/claude/setup.py | sed -n '145,235p'
printf '%s\n' '--- CLI parser and main error handling ---'
cat -n src/didimlog/cli.py | sed -n '103,199p'
cat -n src/didimlog/cli.py | sed -n '454,525p'
printf '%s\n' '--- tests for unavailable Git and command behavior ---'
rg -n -C10 'unavailable|git.*missing|FileNotFoundError|status|doctor|index|same decision|PROJECT_EXCLUDE_GIT_UNAVAILABLE' tests/didimlog_tests/test_cli_commands.py tests/didimlog_tests/claude/test_status.py tests/didimlog_tests/project/test_git_exclude.pyRepository: zhsks311/didimlog
Length of output: 48624
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- documented command behavior and objectives ---'
rg -n -C5 'PROJECT_EXCLUDE_GIT_UNAVAILABLE|status|doctor|index|Git.*없|Git.*실패|설정되지 않음|same|일관' README.md CHANGELOG.md CONTRIBUTING.md pyproject.toml src tests
printf '%s\n' '--- marker detection and Git environment ---'
cat -n src/didimlog/project/git_exclude.py | sed -n '1,180p'
printf '%s\n' '--- diagnostic probe call path ---'
cat -n src/didimlog/claude/probe.py | sed -n '1,220p'Repository: zhsks311/didimlog
Length of output: 50374
🏁 Script executed:
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
files = {
"cli": Path("src/didimlog/cli.py"),
"indexing": Path("src/didimlog/indexing.py"),
"status": Path("src/didimlog/claude/status.py"),
"setup": Path("src/didimlog/claude/setup.py"),
}
trees = {name: ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
for name, path in files.items()}
def function(tree, name):
for node in ast.walk(tree):
if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name == name:
return node
raise LookupError(name)
def called_names(node):
names = []
for child in ast.walk(node):
if isinstance(child, ast.Call):
target = child.func
if isinstance(target, ast.Name):
names.append(target.id)
elif isinstance(target, ast.Attribute):
names.append(target.attr)
return names
for module, funcs in {
"cli": ["_project_knowledge_mode", "_setup", "_index", "_status", "_doctor"],
"indexing": ["_discover_git_root", "run_index"],
"status": ["status_text", "doctor_text", "_diagnostic_problems"],
"setup": ["plan_setup"],
}.items():
print(module)
for name in funcs:
node = function(trees[module], name)
print(f" {name}: calls {called_names(node)}")
cli_mode = function(trees["cli"], "_project_knowledge_mode")
setup_plan = function(trees["setup"], "plan_setup")
index_root = function(trees["indexing"], "_discover_git_root")
assert "discover_project_for_setup" in called_names(cli_mode)
assert "plan_setup" in called_names(function(trees["cli"], "_setup"))
assert "discover_project_for_setup" in called_names(setup_plan)
assert "run_index" in called_names(function(trees["cli"], "_index"))
assert "status_text" in called_names(function(trees["cli"], "_status"))
assert "doctor_text" in called_names(function(trees["cli"], "_doctor"))
assert "shutil" not in called_names(index_root)
print("static call-path assertions: PASS")
PYRepository: zhsks311/didimlog
Length of output: 1774
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- probe implementation ---'
ast-grep outline src/didimlog/claude/probe.py
rg -n -C8 '_discover_git_root|discover_project|git|Problem|def inspect' src/didimlog/claude/probe.py src/didimlog/claude/status.pyRepository: zhsks311/didimlog
Length of output: 10152
Unify Git-unavailable handling across commands
When a .git marker exists and Git cannot run, didim setup raises PROJECT_EXCLUDE_GIT_UNAVAILABLE before printing its plan. The failure occurs in _project_knowledge_mode for interactive setup and in plan_setup for other modes.
status, doctor, and index convert the same condition to None through indexing._discover_git_root and report the project as unconfigured. Use one discovery result that distinguishes Git unavailability from being outside a Git project. Add coverage for all four commands.
🤖 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 `@src/didimlog/cli.py` around lines 220 - 226, Unify project discovery so
Git-unavailable projects are represented as an unconfigured result rather than
raising. Update _project_knowledge_mode and plan_setup to reuse the same
discovery behavior as indexing._discover_git_root, preserving the distinction
between Git being unavailable and the directory being outside a Git project. Add
coverage for setup, status, doctor, and index when a .git marker exists but Git
cannot run.
| while True: | ||
| print("프로젝트 지식을 어디에 둘까요?") | ||
| print("1. 이 컴퓨터에서만 사용 — 기본") | ||
| print("2. Git에 포함해 공유") | ||
| selected = input("선택 [1]: ").strip() | ||
| if selected in ("", "1"): | ||
| return "local" | ||
| if selected == "2": | ||
| return "shared" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle end of input in the storage prompt.
The loop calls input() until the user gives a valid answer. If the user presses Ctrl-D, or the terminal input ends, input() raises EOFError. Nothing catches it, so didim setup exits with a traceback instead of the CLI error contract. The same applies to KeyboardInterrupt unless main already handles it. Return the default mode on end of input.
🐛 Proposed fix
while True:
print("프로젝트 지식을 어디에 둘까요?")
print("1. 이 컴퓨터에서만 사용 — 기본")
print("2. Git에 포함해 공유")
- selected = input("선택 [1]: ").strip()
+ try:
+ selected = input("선택 [1]: ").strip()
+ except EOFError:
+ return "local"
if selected in ("", "1"):
return "local"
if selected == "2":
return "shared"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while True: | |
| print("프로젝트 지식을 어디에 둘까요?") | |
| print("1. 이 컴퓨터에서만 사용 — 기본") | |
| print("2. Git에 포함해 공유") | |
| selected = input("선택 [1]: ").strip() | |
| if selected in ("", "1"): | |
| return "local" | |
| if selected == "2": | |
| return "shared" | |
| while True: | |
| print("프로젝트 지식을 어디에 둘까요?") | |
| print("1. 이 컴퓨터에서만 사용 — 기본") | |
| print("2. Git에 포함해 공유") | |
| try: | |
| selected = input("선택 [1]: ").strip() | |
| except EOFError: | |
| return "local" | |
| if selected in ("", "1"): | |
| return "local" | |
| if selected == "2": | |
| return "shared" |
🤖 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 `@src/didimlog/cli.py` around lines 228 - 236, Update the storage-selection
prompt loop to catch EOFError and return the default "local" mode when input
ends; also handle KeyboardInterrupt there unless main already converts it to the
CLI error contract. Preserve the existing valid-selection behavior for blank,
"1", and "2" inputs.
| os.link( | ||
| temporary_name, | ||
| target.name, | ||
| src_dir_fd=parent_descriptor, | ||
| dst_dir_fd=parent_descriptor, | ||
| follow_symlinks=False, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Python os.link follow_symlinks parameter macOS NotImplementedError supports_follow_symlinks
💡 Result:
In Python, the behavior of the follow_symlinks parameter in os.link is platform-dependent because the underlying system call (link vs. linkat) behaves differently across operating systems [1][2]. On macOS, os.link historically relied on the link system call, which behaves differently than on Linux; specifically, link on macOS follows symbolic links [1][3]. Because of this native behavior, Python's implementation has historically struggled to provide cross-platform consistency for the follow_symlinks=False argument [1][3]. Recent updates in Python (specifically addressing issue gh-81793 in versions leading up to 3.14) have moved toward using the linkat system call whenever it is available to provide more predictable behavior [4][5]. Consequently: 1. NotImplementedError: You may encounter a NotImplementedError if you attempt to use follow_symlinks=False on a version of macOS where the runtime environment does not support the linkat system call [4][5]. The implementation now explicitly raises an error if it cannot fulfill the request to not follow a symlink [4][5]. 2. os.supports_follow_symlinks: This is a set of functions that support the follow_symlinks parameter [6][7]. Whether os.link is included in this set depends on your specific Python version and the underlying OS capabilities (specifically the availability of linkat) [3][5]. You can check if a function supports it by inspecting this set or by attempting the operation and catching the error [6][7]. If you are writing cross-platform code, you should be aware that the availability of this functionality is not guaranteed across all macOS versions or historical Python releases [3][5]. If linkat is not available at runtime, Python cannot enforce non-following behavior for links, resulting in the mentioned error [4][5].
Citations:
- 1: os.link(..., follow_symlinks=False) without linkat(3) python/cpython#85527
- 2: os.link(..., follow_symlinks=True) broken on Linux python/cpython#81793
- 3: Wrong
follow_symlinksdefault fallback implementation python/cpython#119885 - 4: gh-81793: Always call linkat() from os.link(), if available python/cpython#132517
- 5: python/cpython@5a57248
- 6: https://docs.python.org/3/library/os.html
- 7: https://docs.pythonlang.net/3/library/os.html
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- platform and Python support declarations ---'
rg -n -i 'macos|darwin|linux|python_requires|requires-python|python-version|supported' \
CHANGELOG.md pyproject.toml setup.py setup.cfg tox.ini .github 2>/dev/null || true
printf '%s\n' '--- relevant implementation and exception handling ---'
cat -n src/didimlog/conditional_file.py | sed -n '170,250p'
rg -n 'write_regular_file_if_unchanged|NotImplementedError|target could not be written atomically|os\.link' \
src tests 2>/dev/null || true
printf '%s\n' '--- local runtime capability data ---'
python3 - <<'PY'
import os, platform, sys
print("python:", sys.version)
print("platform:", platform.platform())
print("os.link in os.supports_follow_symlinks:", os.link in os.supports_follow_symlinks)
print("supports_follow_symlinks:", sorted(getattr(f, "__name__", repr(f)) for f in os.supports_follow_symlinks))
PYRepository: zhsks311/didimlog
Length of output: 10702
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- temporary-file creation and path validation ---'
ast-grep outline src/didimlog/conditional_file.py
cat -n src/didimlog/conditional_file.py | sed -n '1,165p'
cat -n src/didimlog/conditional_file.py | sed -n '245,285p'
printf '%s\n' '--- tests for link behavior and failure mapping ---'
cat -n tests/didimlog_tests/test_conditional_file.py | sed -n '185,235p'
cat -n tests/didimlog_tests/test_conditional_file.py | sed -n '270,335p'
cat -n tests/didimlog_tests/test_conditional_file.py | sed -n '350,375p'Repository: zhsks311/didimlog
Length of output: 14212
🌐 Web query:
CPython os.link follow_symlinks=False macOS Python 3.11 3.12 3.13 3.14 NotImplementedError os.supports_follow_symlinks
💡 Result:
In CPython, the support for the follow_symlinks argument in os.link is platform-dependent because the underlying system link behavior varies across operating systems [1][2]. On macOS, the system link function behaves like linkat with the AT_SYMLINK_FOLLOW flag, meaning it follows symbolic links by default [1][2]. Historically, CPython's os.link has relied on the platform's link function, causing inconsistent behavior regarding the follow_symlinks argument [3][2]. Recent updates in CPython (notably addressed in issues like gh-81793) have sought to standardize this by utilizing linkat where available at runtime [4][5]. As of the current state of CPython (including 3.11–3.14 branches influenced by these changes): 1. On macOS, if you explicitly pass follow_symlinks=False, CPython will attempt to use the system linkat function to prevent following the symlink [4][6]. 2. If the system linkat function is not available at runtime on your macOS system, attempting to use follow_symlinks=False will raise an error (typically an argument_unavailable_error) [4][6]. This occurs because the standard link function on macOS does not support the ability to not follow a symlink [1][2]. The os.supports_follow_symlinks object in the os module is intended to track such support, but for os.link specifically, the actual availability is often determined by the presence of the linkat system call at runtime rather than static availability [4][6][2]. If your environment lacks linkat support, you cannot reliably use follow_symlinks=False for os.link on macOS [4][6].
Citations:
- 1: os.link(..., follow_symlinks=False) without linkat(3) python/cpython#85527
- 2: https://bugs.python.org/issue41355
- 3: Wrong
follow_symlinksdefault fallback implementation python/cpython#119885 - 4: gh-81793: Always call linkat() from os.link(), if available python/cpython#132517
- 5: python/cpython@5a57248
- 6: hugovk/cpython@5a57248
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
p = Path("src/didimlog/conditional_file.py")
text = p.read_text()
for needle in ("def _target_path", "def _temporary_file", "def write_regular_file_if_unchanged"):
start = text.index(needle)
end = text.find("\ndef ", start + 1)
if end == -1:
end = len(text)
print(f"--- {needle} ---")
print(text[start:end])
PYRepository: zhsks311/didimlog
Length of output: 5424
🏁 Script executed:
#!/bin/bash
set -eu
cat -n src/didimlog/conditional_file.py | sed -n '1,165p'
cat -n tests/didimlog_tests/test_conditional_file.py | sed -n '185,235p'
cat -n tests/didimlog_tests/test_conditional_file.py | sed -n '270,335p'Repository: zhsks311/didimlog
Length of output: 11740
🌐 Web query:
site:docs.python.org/3.11/library/os.html os.link follow_symlinks macOS NotImplementedError
💡 Result:
The NotImplementedError occurs when using the follow_symlinks argument with os.link on platforms where this feature is not supported by the underlying operating system [1]. In Python, you can verify if this argument is supported for a specific function by checking the os.supports_follow_symlinks object [1]. If a platform does not support this parameter, attempting to use it will raise a NotImplementedError [1]. Since the support for parameters like follow_symlinks depends on the underlying system's implementation, you should ensure your code checks availability or handles the exception if you are writing cross-platform code [1].
Citations:
🌐 Web query:
site:github.com/python/cpython issue 119885 os.link macOS follow_symlinks
💡 Result:
Python issue 119885 addressed the inconsistent and platform-dependent behavior of the follow_symlinks parameter in os.link [1][2]. Historically, os.link relied on the platform's system link function, which behaves differently across operating systems—specifically, it follows symlinks by default on macOS but behaves differently (often not following them) on other platforms like Linux [1][3]. The issue highlighted that when follow_symlinks=True was set, Python's implementation could inadvertently map to a syscall path that did not properly honor the parameter, leading to unexpected behavior [1]. This problem has since been superseded and addressed by broader changes in Python, specifically under issue gh-81793 (implemented in PR 132517), which mandates the use of the linkat system call whenever it is available [4][5][6]. The resulting behavior on macOS is as follows: - The system now attempts to use linkat to provide consistent control over symlink behavior [4][5]. - If follow_symlinks=False is passed but the system's linkat function is not available at runtime, Python will now raise an error rather than silently failing to honor the parameter [4][5]. These changes aim to standardize the behavior of os.link across different platforms by ensuring that symlink handling is explicitly supported or restricted based on the underlying system's capabilities [4][5].
Citations:
- 1: Wrong
follow_symlinksdefault fallback implementation python/cpython#119885 - 2: gh-119885: follow_symlinks in os.link() python/cpython#119886
- 3: os.link(..., follow_symlinks=False) without linkat(3) python/cpython#85527
- 4: gh-81793: Always call linkat() from os.link(), if available python/cpython#132517
- 5: python/cpython@5a57248
- 6: gh-81793: Always call linkat() from os.link(), if available python/cpython#132517
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
import urllib.request
urls = {
"cpython-3.11-posixmodule": "https://github.com/@raw/python/cpython/3.11/Modules/posixmodule.c",
"cpython-3.12-posixmodule": "https://github.com/@raw/python/cpython/3.12/Modules/posixmodule.c",
"cpython-3.13-posixmodule": "https://github.com/@raw/python/cpython/3.13/Modules/posixmodule.c",
"cpython-3.14-posixmodule": "https://github.com/@raw/python/cpython/3.14/Modules/posixmodule.c",
}
for name, url in urls.items():
try:
text = urllib.request.urlopen(url, timeout=10).read().decode()
except Exception as exc:
print(name, "ERROR", exc)
continue
lines = text.splitlines()
hits = [i for i, line in enumerate(lines) if "follow_symlinks" in line and ("link" in line.lower() or "linkat" in line.lower())]
print(f"--- {name} ---")
for i in hits[:20]:
lo, hi = max(0, i - 8), min(len(lines), i + 12)
print("\n".join(f"{j+1}: {lines[j]}" for j in range(lo, hi)))
print()
PYRepository: zhsks311/didimlog
Length of output: 819
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import urllib.request
for version in ("3.11", "3.12", "3.13", "3.14"):
url = f"https://github.com/@raw/python/cpython/{version}/Modules/posixmodule.c"
text = urllib.request.urlopen(url, timeout=10).read().decode()
lines = text.splitlines()
print(f"--- CPython {version} ---")
for i, line in enumerate(lines):
if "follow_symlinks" in line and ("link" in line.lower() or "linkat" in line.lower()):
print("\n".join(f"{j + 1}: {lines[j]}" for j in range(max(0, i - 5), min(len(lines), i + 8))))
print()
PYRepository: zhsks311/didimlog
Length of output: 2441
🏁 Script executed:
#!/bin/bash
set -eu
for version in 3.11 3.12 3.13 3.14; do
echo "--- CPython $version ---"
curl -ksSf "https://github.com/@raw/python/cpython/$version/Modules/posixmodule.c" |
rg -n -C 12 'follow_symlinks.*link|linkat.*follow_symlinks|argument_unavailable.*follow_symlinks|HAVE_LINKAT' |
head -n 120 || true
doneRepository: zhsks311/didimlog
Length of output: 17867
🏁 Script executed:
#!/bin/bash
set -eu
for version in 3.11 3.12 3.13 3.14; do
echo "--- CPython $version os.link implementation ---"
curl -ksSf "https://github.com/@raw/python/cpython/$version/Modules/posixmodule.c" |
rg -n -C 35 'posix_link|os\.link|linkat\(' |
head -n 180 || true
doneRepository: zhsks311/didimlog
Length of output: 23717
🏁 Script executed:
#!/bin/bash
set -eu
for version in 3.11 3.12 3.13 3.14; do
echo "--- CPython $version os.link implementation ---"
curl -ksSf "https://github.com/@raw/python/cpython/$version/Modules/posixmodule.c" |
rg -n -C 35 'posix_link|linkat\(' |
head -n 180 || true
doneRepository: zhsks311/didimlog
Length of output: 22687
Handle NotImplementedError without removing follow_symlinks=False.
On macOS runtimes without linkat, CPython raises NotImplementedError for this call. The handler catches only ValueError and OSError, so the existing error contract is bypassed. Map NotImplementedError to the atomic-write ValueError. Keep follow_symlinks=False because another process can replace the temporary pathname with a symlink before os.link.
🤖 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 `@src/didimlog/conditional_file.py` around lines 214 - 220, Update the
exception handling around the os.link call to also catch NotImplementedError and
map it to the existing atomic-write ValueError contract. Preserve
follow_symlinks=False in the os.link invocation and leave the current
ValueError/OSError handling unchanged.
Source: Linters/SAST tools
| os.link( | ||
| temporary_name, | ||
| target.name, | ||
| src_dir_fd=parent_descriptor, | ||
| dst_dir_fd=parent_descriptor, | ||
| follow_symlinks=False, | ||
| ) | ||
| publication_sync_pending = True | ||
| os.unlink(temporary_name, dir_fd=parent_descriptor) | ||
| temporary_name = None | ||
| os.fsync(parent_descriptor) | ||
| publication_sync_pending = False | ||
| return |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
A successful publication can still report an error. After os.link publishes the new inode, a later os.unlink or os.fsync failure raises ValueError("target could not be written atomically"). The target already holds the intended bytes. Callers cannot distinguish "not written" from "written, cleanup failed".
src/didimlog/conditional_file.py#L214-L226: state the post-publication semantics in thewrite_regular_file_if_unchangeddocstring, or return a status that separates publication from cleanup.src/didimlog/project/git_exclude.py#L534-L569: do not map everyValueErrorfrom the write toPROJECT_EXCLUDE_CHANGED. Re-read the exclude file before choosing the token, or extend the help text so the user does not assume the previous run made no change.
📍 Affects 2 files
src/didimlog/conditional_file.py#L214-L226(this comment)src/didimlog/project/git_exclude.py#L534-L569
🤖 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 `@src/didimlog/conditional_file.py` around lines 214 - 226, The
write_regular_file_if_unchanged publication path must distinguish successful
inode publication from subsequent cleanup or fsync failure; document these
post-publication semantics in its docstring or return a status that separates
publication from cleanup. In src/didimlog/conditional_file.py lines 214-226,
update write_regular_file_if_unchanged accordingly. In
src/didimlog/project/git_exclude.py lines 534-569, stop mapping every ValueError
to PROJECT_EXCLUDE_CHANGED: re-read the exclude file before selecting that
token, or revise the help text so it does not imply the previous run made no
change.
| def _build_plan( | ||
| project_root: Path, | ||
| path: Path, | ||
| mode: str, | ||
| original: bytes | None, | ||
| ) -> GitExcludePlan: | ||
| intended, changes = _transform(original, mode) | ||
| if mode == "local" and _tracked_knowledge_exists(project_root): | ||
| raise _tracked() | ||
| planned_ignored = _planned_knowledge_is_ignored(project_root, intended) | ||
| if mode == "local" and not planned_ignored: | ||
| raise _conflict() | ||
| notices: tuple[str, ...] = () | ||
| if mode == "shared" and planned_ignored: | ||
| notices = ( | ||
| "다른 Git 규칙이 knowledge 폴더를 계속 제외하고 있습니다.", | ||
| ) | ||
| return GitExcludePlan( | ||
| project_root=project_root, | ||
| path=path, | ||
| mode=mode, | ||
| original=original, | ||
| intended=intended, | ||
| changes=changes, | ||
| notices=notices, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The transformed exclude content is not bounded by _MAXIMUM_EXCLUDE_BYTES. _read_exclude refuses a file larger than 1 MiB, but _transform adds about 90 bytes and no code checks the result before the write. An original near the limit produces a file that no later plan can read, so the managed block can never be removed.
src/didimlog/project/git_exclude.py#L478-L503: in_build_plan, raise before the plan is returned whenintendedis longer than_MAXIMUM_EXCLUDE_BYTES.tests/didimlog_tests/project/test_git_exclude.py#L166-L186: add a case with aninfo/excludefile at the size limit and assert thatlocalplanning is refused and the file is unchanged.
📍 Affects 2 files
src/didimlog/project/git_exclude.py#L478-L503(this comment)tests/didimlog_tests/project/test_git_exclude.py#L166-L186
🤖 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 `@src/didimlog/project/git_exclude.py` around lines 478 - 503, In _build_plan,
validate intended before returning GitExcludePlan and raise the established
refusal error when its byte length exceeds _MAXIMUM_EXCLUDE_BYTES. Add a test in
tests/didimlog_tests/project/test_git_exclude.py covering an info/exclude file
exactly at the limit, asserting local planning is refused and the file remains
unchanged.
| def test_setup_help_registers_exact_project_knowledge_option(self): | ||
| code, stdout, stderr = invoke(["setup", "--help"]) | ||
|
|
||
| self.assertEqual((code, stderr), (0, "")) | ||
| self.assertIn("--project-knowledge {local,shared}", stdout) | ||
| self.assertIn( | ||
| "프로젝트 지식을 이 컴퓨터에만 둘지 Git으로 공유할지 선택", | ||
| stdout, | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Pin the help width so the metavar assertion stays stable.
argparse formats help with shutil.get_terminal_size(). That call reads the COLUMNS environment variable, then the real terminal. On a narrow terminal argparse wraps the option line, and "--project-knowledge {local,shared}" no longer appears as one substring. The test then fails only in some environments.
💚 Proposed fix
def test_setup_help_registers_exact_project_knowledge_option(self):
- code, stdout, stderr = invoke(["setup", "--help"])
+ with mock.patch.dict(os.environ, {"COLUMNS": "200"}):
+ code, stdout, stderr = invoke(["setup", "--help"])Import os in the test module if it is not imported yet.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_setup_help_registers_exact_project_knowledge_option(self): | |
| code, stdout, stderr = invoke(["setup", "--help"]) | |
| self.assertEqual((code, stderr), (0, "")) | |
| self.assertIn("--project-knowledge {local,shared}", stdout) | |
| self.assertIn( | |
| "프로젝트 지식을 이 컴퓨터에만 둘지 Git으로 공유할지 선택", | |
| stdout, | |
| ) | |
| def test_setup_help_registers_exact_project_knowledge_option(self): | |
| with mock.patch.dict(os.environ, {"COLUMNS": "200"}): | |
| code, stdout, stderr = invoke(["setup", "--help"]) | |
| self.assertEqual((code, stderr), (0, "")) | |
| self.assertIn("--project-knowledge {local,shared}", stdout) | |
| self.assertIn( | |
| "프로젝트 지식을 이 컴퓨터에만 둘지 Git으로 공유할지 선택", | |
| stdout, | |
| ) |
🤖 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 `@tests/didimlog_tests/test_cli_commands.py` around lines 124 - 132, Update
test_setup_help_registers_exact_project_knowledge_option to pin the help-output
width before invoking the CLI, using the test module’s environment handling and
importing os if needed; restore the prior COLUMNS value afterward so the test
remains isolated while keeping the existing metavar assertion unchanged.
리뷰 후속 판정과 로컬 수정 결과
검증
남은 LOW/OS-level 제약: commit 뒤 cleanup |
Reject oversized project excludes before writing, make replacement rollback durable, and report Git discovery failures consistently in setup diagnostics. Constraint: Preserve existing fail-soft index and hook behavior Rejected: Broaden Git discovery semantics across all callers | outside PR scope Confidence: high Scope-risk: moderate
변경 내용
didim setup에서 프로젝트 지식을 이 컴퓨터에서만 쓸지(local) Git으로 공유할지(shared) 선택할 수 있게 했습니다.local은 Git이 알려주는 공용 exclude 파일에 Didimlog 관리 block만 추가하고,shared는 그 block만 제거합니다.검증
uv run --project . python -m unittest discover -s tests -v— 479 tests passeduv build— sdist/wheel 생성 성공git check-ignore, status, doctor, 재실행 metadata no-op, shared 전환 확인.gitignore미생성, Git index 불변, 기존 exclude 사용자 bytes 보존 확인기준
developdocs/superpowers/specs/2026-08-10-local-project-knowledge-exclusion-design.mddocs/superpowers/plans/2026-08-10-local-project-knowledge-exclusion.mdSummary by CodeRabbit
New Features
setupnow supports local or shared project knowledge with--project-knowledge.Bug Fixes
Documentation