Phase 1: unified detection for setup import redesign - #127
Merged
Conversation
Reviewer's GuideIntroduces a unified detection pipeline for setup import that scans harness configs into a single DetectionResult model, uses content hashing to deduplicate and auto-resolve identical items across harnesses, filters out managed/symlinked resources, and adds a --json flag to emit the detection result without invoking the existing interactive import flow. Flow diagram for dotagents setup --json detection pipelineflowchart LR
U["User runs\n dotagents setup --json"] --> S["runSetup"]
S -->|JSONOutput true| D["runDetection"]
S -->|JSONOutput false| Legacy["scanNativeImports and interactive import"]
D --> H["iterate detected agentConfig list"]
H --> SK["detectNativeSkills"]
H --> RO["detectNativeRoles"]
H --> MC["detectNativeMCPServers"]
SK --> IDX["build itemIndex[Surface:Name]"]
RO --> IDX
MC --> IDX
IDX --> MI["markIdentical on each DetectedItem"]
MI --> FLT["filter Identical items\n increment AutoResolved"]
FLT --> RES["DetectionResult"]
RES --> J["json.Encoder.Encode(detection)"]
File-Level Changes
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 found 4 issues, and left some high level feedback:
- hashDir relies on filepath.WalkDir’s traversal order, which is not guaranteed to be consistent across filesystems or runs; consider collecting paths, sorting them, and then hashing in that deterministic order to make the directory hash truly stable.
- markIdentical currently treats items with a single source as Identical and therefore auto-resolved, which may be surprising given the intent of “identical across all harnesses”; consider only marking items as identical when they appear in multiple harnesses with matching hashes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- hashDir relies on filepath.WalkDir’s traversal order, which is not guaranteed to be consistent across filesystems or runs; consider collecting paths, sorting them, and then hashing in that deterministic order to make the directory hash truly stable.
- markIdentical currently treats items with a single source as Identical and therefore auto-resolved, which may be surprising given the intent of “identical across all harnesses”; consider only marking items as identical when they appear in multiple harnesses with matching hashes.
## Individual Comments
### Comment 1
<location path="cmd/dotagents/detect.go" line_range="236-237" />
<code_context>
+ return out, nil
+}
+
+func markIdentical(item *DetectedItem) {
+ if len(item.Sources) < 2 {
+ item.Identical = true
+ return
</code_context>
<issue_to_address>
**issue (bug_risk):** Single-source items are treated as `Identical` and will be auto-resolved, which likely hides genuinely unique detections.
Because `markIdentical` sets `Identical = true` when `len(item.Sources) < 2`, any skill/role/MCP present in exactly one harness is never surfaced in `result.Items`. This contradicts the docstring, which implies only items from multiple sources that truly match should be auto-resolved. Suggest only marking items identical when `len(item.Sources) >= 2` and all hashes match, and keeping single-source items non-identical (or handling them in a dedicated branch).
</issue_to_address>
### Comment 2
<location path="cmd/dotagents/detect.go" line_range="143-144" />
<code_context>
+ Hash string
+}
+
+func detectNativeSkills(agent agentConfig, skillRoot string, canonicalSkills string) ([]detectedEntry, error) {
+ entries, err := os.ReadDir(skillRoot)
+ if errors.Is(err, fs.ErrNotExist) {
+ return nil, nil
</code_context>
<issue_to_address>
**issue (bug_risk):** Using an empty `skillRoot` will walk the process working directory, which is likely unintended.
If `agent.SkillRoot` is unset or resolves to `""`, `os.ReadDir(skillRoot)` will read the current working directory, potentially treating unrelated directories as skills. Consider mirroring `detectNativeRoles` and returning early when `skillRoot == ""` to avoid accidental CWD scanning.
</issue_to_address>
### Comment 3
<location path="cmd/dotagents/detect_test.go" line_range="45-46" />
<code_context>
+ if err := os.WriteFile(filepath.Join(dir2, "a.txt"), []byte("world"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ h1, _ := hashDir(dir1)
+ h2, _ := hashDir(dir2)
+ if h1 == h2 {
+ t.Fatal("different content produced the same hash")
</code_context>
<issue_to_address>
**issue (testing):** Assert that hashDir returns no error instead of discarding it
By ignoring the `hashDir` errors, this test can pass even if hashing fails. Capture the errors and fail the test if either call returns a non-nil error before comparing the hash values.
</issue_to_address>
### Comment 4
<location path="cmd/dotagents/detect_test.go" line_range="52" />
<code_context>
+ }
+}
+
+func TestMarkIdenticalSingleSource(t *testing.T) {
+ item := &DetectedItem{
+ Sources: []DetectedSource{{Hash: "abc123"}},
</code_context>
<issue_to_address>
**suggestion (testing):** Cover the zero-sources case for markIdentical to clarify intended behavior
There are tests for single- and multi-source cases, but none for when `item.Sources` is empty. Since `markIdentical` currently treats `len(Sources) < 2` as identical (making an empty item `Identical == true`), please add a test that covers this case—either to document the intended behavior or to reveal if the implementation should be changed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebf77d1959
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c74c8464f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This was referenced Jul 19, 2026
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
DetectedItem/DetectedSource/DetectionResulttypes that replace per-surface scan results with a single detection modelhashDir,hashBytes,hashMCPServer) for identity comparison across harnesses--jsonflag ondotagents setupemits detection result and exitsPhase 1 of #126. Existing import flow is unchanged;
--jsonis an early return before the current prompt-based import.Test plan
go test ./...passesdotagents setup --jsonemits valid JSON detection resultSummary by Sourcery
Introduce a unified detection pass for unmanaged skills, roles, and MCP servers and expose it via a new JSON-only setup mode.
New Features:
Enhancements:
Tests: