fix(dashboard): de-duplicate redacted path listings so a collision can't crash the tree (#7671) by bolichen97 · Pull Request #7678 · kirodotdev/KiroCrew

bolichen97 · 2026-09-01T16:55:15Z

Root cause

Egress redact() is a many-to-one map, so two genuinely different repo paths whose only differing segments are redacted collapse to the same placeholder string (e.g. both a src/... and target/... Maven path flatten to [Manually redacted][REDACTED: credential]_model.txt). GET /api/project/tree handed that list straight to the frontend, and @pierre/trees appendPresortedPaths threw an uncaught Duplicate path inside a useLayoutEffect, crashing the whole route. The changed panel survived the same input because its statusEntries memo already de-duplicated via a seen Set.

Note: the reported filename is a placeholder string and does not exist on disk; no credential was leaked or present — the matcher was over-eager on a filename token.

Changes

Server (src/kiro_crew/dashboard/handlers/files.py):

api_project_tree: de-duplicate paths after redaction with list(dict.fromkeys(...)), preserving order and first occurrence.
api_project_git_status: rework the redaction loop to drop later entries that duplicate an earlier one, preserving order and first occurrence. The key is the (path, status, staged) tuple, not path alone: one file with both staged and unstaged changes (MM, AM, MD) legitimately yields two entries sharing a path, and GitPanel renders them as separate rows keyed ${path}:${staged}. A real redaction collision has an identical tuple, so it still collapses. repoRoot/branch redaction and the files[:500] cap semantics unchanged.

Frontend (website/src/pierre/PierreWorkspaceTreeImpl.tsx):

Defense-in-depth: full-workspace branch of the paths memo now wraps its list in Array.from(new Set(...)), mirroring the changed branch, so any duplicate from a future source degrades to a missing row instead of a render crash. Other branches/memos untouched.

Tests

test/test_project_tree.py and test/test_project_git_status_log.py: regression tests using a real collision (two distinct AKIA…_model.txt tokens that both redact to [REDACTED: credential]_model.txt), asserting the placeholder appears exactly once, no duplicate paths, and raw tokens never leak.
test/test_project_git_status_log.py also pins that a file staged and then modified again keeps both lanes (("M", True) and ("M", False)), so the de-dup cannot regress into dropping the unstaged row.
website/src/test/PierreWorkspaceTreeImpl.test.tsx: asserts resetPaths receives the de-duplicated list.

Verification

Run locally against a CI-parity environment on the rebased branch:

pytest test/test_project_tree.py test/test_project_git_status_log.py test/test_dashboard_files_coverage.py test/test_dashboard_file_io.py test/test_core_path_redact_before_bound.py — 167 passed.
isort --check-only, flake8 src/kiro_crew test, mypy src/kiro_crew/, scripts/check_black_formatting.py — all clean.
npx tsc -b, npx eslint src/ --max-warnings 603 (601 measured), npm run i18n:check, npm run lint:phantom-classes — all clean.
npx vitest run src/test/PierreWorkspaceTreeImpl.test.tsx — 42 passed.
The staged/unstaged regression test was confirmed to FAIL against a path-only de-dup key, so it genuinely pins the fix.

Known non-blocking notes

De-duplication resolves a collision by dropping a genuinely distinct file from the listing with no surfaced signal (acceptable vs. a route crash; a follow-up could log/annotate collapsed collisions). The tree can under-count files when a redaction collision occurs.
The files[:500] / tree entry caps are computed on the raw listing before redaction and de-dup, so a response can return fewer than the cap while still flagged truncated (intentional, documented). Moving the cap after redaction would mean redacting the entire listing on the hot path.
The two credential-shaped fixture ids are bound to key_a / key_b, with key_b kept as a split literal so Semgrep's detected-aws-access-key-id-value rule does not read a synthetic fixture as a leaked key. Two distinct ids are required for the test to prove de-duplication at all.

Pattern harvest

Rule candidate: review-prompt

Pattern: the output of a many-to-one map — an egress redact() — handed to a
consumer that requires unique keys, where appendPresortedPaths throws
Duplicate path. Review prompt: wherever a redacted, normalized or truncated
string list crosses an API boundary, ask whether the consumer assumes injectivity,
and de-duplicate on the producing side.

github-actions · 2026-09-01T17:12:45Z

Design Review (Fable 5) — ✅ PASS

Design-level review of 088b66808729d5bd65b9f1182d09153a1dd4e887 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Crash traced to its real mechanism (many-to-one redaction meets a uniqueness-assuming consumer), fixed at the producing boundary with a lane-preserving key, plus frontend backstop.

Suggestions

The silent-drop tradeoff is acknowledged; when you do the follow-up, prefer disambiguating the placeholder (per-response ordinal, not a content hash — a hash is a verification oracle for the redacted token) over logging, so distinct files stay visible.

[DESIGN-REVIEWED] 088b668

github-actions · 2026-09-01T17:12:56Z

UX Review (Fable 5) — ✅ PASS

UX-level review of 088b66808729d5bd65b9f1182d09153a1dd4e887 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Crash-to-degraded fix with no new UI surface: a route that previously died on redaction collisions now renders, and the collapsed-row tradeoff is deliberate and rare.

[UX-REVIEWED] 088b668

github-actions · 2026-09-01T17:15:04Z

Opus 4.8 Review — ✅ no blocking findings

Reviewed 088b66808729d5bd65b9f1182d09153a1dd4e887 — this comment is updated in place on each push.

Review details

No findings.

The sole candidate — GitPanel's React key ${f.path}:${f.staged} colliding when two redaction-collapsed files share staged but differ in status — dies under falsification. GitPanel.tsx:146 is not touched by this diff, and its per-row key predates this change; the backend dedup this PR adds only reduces GitPanel collisions (it collapses same-(path,status,staged) entries that previously reached the panel unfiltered), so the diff neither introduces nor worsens the key mismatch. Its minimal fix lives in an untouched file, and its triggering condition (a credential-shaped-token redaction collision that also happens to produce the same staged with a different status) is a constructed compound edge case, not an input that occurs in practice — condition (a) resolves to "could," and the only outcome is a console warning / reconciliation glitch, not a crash or data loss. Dropped.

[OPUS-REVIEWED] 088b668

Verdict parsed from the review's SHA-scoped output markers for commit 088b66808729d5bd65b9f1182d09153a1dd4e887.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 088b66808729d5bd65b9f1182d09153a1dd4e887: <one-sentence reason>

github-actions · 2026-09-01T17:15:06Z

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 088b66808729d5bd65b9f1182d09153a1dd4e887 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. Producing the review.

First-Principles-Verdict: CONCERNS

The crash fix lands at the cause; the frontend Set wrap rides on a future-only premise once the server can no longer emit a duplicate.

What this change ships

Intent: stop a redaction collision from crashing the dashboard file-tree route (#7671) — a FIX.

Tree listing no longer repeats a collided redacted path — justified, cause-level fix of the reported crash
Git-status listing collapses collided entries (keyed path+status+staged) — justified, same-cause sibling; duplicate GitPanel rows/keys and inflated changed-count
Pierre tree de-dups its own input as defense-in-depth — declared, but future-only justification
Regression tests pin collision de-dup and the staged/unstaged two-lane case — justified

Watch

Item 3's stated harm is "any duplicate from a future source" — an inherited requirement, not a today-harm. I counted producers: api.projectTree is the only data source feeding PierreWorkspaceTreeImpl's full-workspace branch (grep projectTree in website/src, non-test consumers: PierreWorkspaceTreeImpl.tsx:154, plus FileBrowserRail.tsx:37 which uses only readiness, not paths), and the dist is served by the same backend that now de-dups, so no version-skew path delivers a duplicate. Its zero option costs nobody anything observable today.
Sibling sweep for the root cause came back clean: grepping redact(...) for across src/ finds only display-string lists (dev_fleet/repository.py, workflows/library.py) with no uniqueness-requiring consumer — items 1–2 cover the class.

Subtractions

Drop the Array.from(new Set(...)) wrap, its 10-line comment, and the matching PierreWorkspaceTreeImpl.test.tsx case; the server-side de-dup (items 1–2) already removes the only counted duplicate source.

[FIRST-PRINCIPLES-REVIEWED] 088b668

bolichen97

Description ↔ code mismatch: the dedup key collapses dual-status files, which the description does not disclose.

What the description claims. "api_project_git_status: rework the redaction loop to keep a seen_paths set, dropping later entries whose redacted path duplicates an earlier one. repoRoot/branch redaction and the files[:500] cap semantics unchanged." The whole change is scoped as the "Same collision class as api_project_tree" — i.e. a redaction-collision fix.

What the code does. The new guard keys on the path alone:

if f["path"] in seen_paths:
    continue

But _run() already emits two entries with the same path and different staged for a file that is both staged and modified — src/kiro_crew/dashboard/handlers/files.py:4727-4731 on main sets staged=True when x not in (" ", "?", "!") and staged=False when y not in (" ", "?", "!"). Those two path values are byte-identical, so the second (unstaged) row is dropped. This fires on ordinary input, with or without any redaction collision.

Why it is user-visible. website/src/components/GitPanel.tsx:144-146 maps over status.files and renders one row per entry with key={${f.path}:${f.staged}} — it deliberately keys on .staged in order to show the staged and unstaged rows for the same file as two distinct rows. It also derives fileCount from status.files.length (line 85). So after this change, any staged-and-then-modified file loses its unstaged row in GitPanel and fileCount under-reports.

(For contrast, the Pierre changed panel is unaffected: PierreWorkspaceTreeImpl.tsx:203 already collapses per path, so nothing regresses there. GitPanel is the consumer that relies on both rows.)

Required fix — either one:

Key the dedup on the pair, so only genuine redaction collisions collapse: if (f["path"], f.get("staged")) in seen_paths: continue; or
Keep the path-only key and disclose it in the description — state that dual-status files are collapsed to one row and that GitPanel's row list and fileCount change accordingly.

Option 1 preserves the stated scope ("git-status semantics unchanged") and is what the description already promises.

Related: Opus 4.8 Review flagged this same files.py dedup key as an advisory, but scoped it to the handler without tracing the GitPanel consumer, so it did not escalate.

_{Posted as a comment rather than a change request because the reviewer account is the PR author; GitHub does not allow REQUEST_CHANGES on one's own PR. Treat it as blocking.}

github-actions · 2026-09-01T18:16:36Z

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 088b66808729d5bd65b9f1182d09153a1dd4e887 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/dashboard/handlers/files.py:4802 -- (path, status, staged) preserves same-staged collisions with differing statuses, creating duplicate GitPanel row keys -> Fix: de-duplicate by (path, staged).

[GPT-REVIEWED] 088b668

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 088b66808729d5bd65b9f1182d09153a1dd4e887: <one-sentence reason>

Egress redact() collapses genuinely-different paths (e.g. distinct credential-shaped filename tokens) to the same placeholder, producing duplicate entries. The dashboard tree hands these straight to @pierre/trees, whose appendPresortedPaths throws 'Duplicate path' on adjacent identical rows and crashes the workspace panel. De-duplicate after redaction in both api_project_tree and api_project_git_status, preserving order and first occurrence. Does not change 'truncated' or the files[:500] cap (both applied to the raw listing before redaction). Adds backend regression tests exercising a real ls-files/status collision. git-status keys the de-dup on (path, status, staged) rather than path alone: one file with both staged and unstaged changes legitimately emits two entries sharing a path, and GitPanel renders them as separate rows keyed on path+staged. A real redaction collision has an identical tuple, so it still collapses. Also de-dup the full-workspace branch of the frontend paths memo (mirroring the changed branch's existing statusEntries seen-Set), since the appendPresortedPaths throw fires from inside the resetPaths useLayoutEffect and takes down the whole workspace route uncaught.

iamwhatever

Tier 1 auto-approve: fix (5 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix with a clear root cause -- redact() can collapse two genuinely-different paths onto one placeholder and @pierre/trees throws on adjacent duplicates, crashing the tree; the fix de-duplicates after redaction, keying on (path, status, staged) so a file's staged and unstaged lanes both survive (#7671).

chenmingwei23

Tier 1 auto-approve: fix (5 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: fix, de-duplicates already-redacted path listings so a redaction collision cannot crash the @pierre/trees panel (consumes redact() output; no change to redaction/secret handling, no new parsing/auth).

… survive (kirodotdev#8042) redact()'s bare-secret matcher has '/' in its payload class (base64 alphabet), so a deep slash-only path chains into ONE >=40-char candidate run; the whole-run amplification in redact_credentials then replaces the entire path with a single placeholder. Two genuinely different files collapse to a byte-identical key, and the kirodotdev#7678 de-dup then silently drops the later entries from the workspace tree. The listing endpoints (api_project_tree, api_project_git_status) KNOW their values are project-relative POSIX paths, so a separator is a hard boundary there: _redact_path_display() redacts per segment on the slow path (fast path: unchanged values return after one redact() call). A genuine >=40-char secret as a single segment still redacts; the general redact() behaviour for unknown-provenance text is untouched.

bolichen97 · 2026-09-04T06:39:23Z

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

PR #3987 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #3987: REBASE. Merged code now owns the block this PR extends, and the two interact on more than text: the rebase has to re-establish the one-pass-covers-every-row invariant on top of main's dedupe rebind. Files: src/kiro_crew/dashboard/handlers/files.py.
PR #6905 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6905: MERGE_DISCUSSION. The merged dedup is what makes PR #6905 stale; the rebase is mechanical -- keep main's dict.fromkeys dedup and apply the wrapper only to result['root']. Files: src/kiro_crew/dashboard/handlers/files.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

bolichen97 requested a review from a team September 1, 2026 16:55

bolichen97 requested a review from a team as a code owner September 1, 2026 16:55

bolichen97 requested a review from chenmingwei23 September 1, 2026 16:55

github-actions Bot added the readiness: checking Automated validation is still running label Sep 1, 2026

github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 1, 2026

bolichen97 commented Sep 1, 2026

View reviewed changes

bolichen97 force-pushed the fix/7671-dedupe-redacted-tree-paths branch from b0a9f28 to 088b668 Compare September 2, 2026 00:52

iamwhatever enabled auto-merge (squash) September 2, 2026 02:07

iamwhatever approved these changes Sep 2, 2026

View reviewed changes

chenmingwei23 approved these changes Sep 2, 2026

View reviewed changes

iamwhatever merged commit fd0e26a into main Sep 2, 2026
69 checks passed

iamwhatever deleted the fix/7671-dedupe-redacted-tree-paths branch September 2, 2026 02:08

github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 2, 2026

github-actions Bot mentioned this pull request Sep 2, 2026

feat: add github pull request monitor probe #5183

Merged

5 tasks

mcwilliamsbrian mentioned this pull request Sep 3, 2026

redact() treats a deep path as one secret run, collapsing distinct paths to one key #8042

Open

1 task

jeeshofone mentioned this pull request Sep 3, 2026

fix(dashboard): redact known-path listings segment-wise so deep paths survive #8055

Open

4 tasks

bolichen97 mentioned this pull request Sep 4, 2026

feat(dashboard): find git repos below the chat's project dir, grouped per repo #3987

Draft

bolichen97 mentioned this pull request Sep 4, 2026

fix(dashboard): preserve macOS temp project paths #6905

Open

5 tasks

pankaj-singh-ac mentioned this pull request Sep 9, 2026

[bug] Duplicate path crash in session file-change tree (PierreWorkspaceTreeImpl.resetPaths) — not covered by #7678 fix #9716

Open

1 task

Navigation Menu

Uh oh!

fix(dashboard): de-duplicate redacted path listings so a collision can't crash the tree (#7671) - #7678

fix(dashboard): de-duplicate redacted path listings so a collision can't crash the tree (#7671)#7678
iamwhatever merged 1 commit into
mainfrom
fix/7671-dedupe-redacted-tree-paths

bolichen97 commented Sep 1, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 1, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 1, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 1, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 1, 2026 •

edited

Loading

Uh oh!

bolichen97 left a comment

Uh oh!

github-actions Bot commented Sep 1, 2026 •

edited

Loading

Uh oh!

iamwhatever left a comment

Uh oh!

chenmingwei23 left a comment

Uh oh!

Uh oh!

bolichen97 commented Sep 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

bolichen97 commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root cause

Changes

Tests

Verification

Known non-blocking notes

Pattern harvest

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design Review (Fable 5) — ✅ PASS

Suggestions

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

UX Review (Fable 5) — ✅ PASS

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Opus 4.8 Review — ✅ no blocking findings

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

First Principles Review (Fable 5) — 🟡 CONCERNS

What this change ships

Watch

Subtractions

Uh oh!

bolichen97 left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GPT 5.6 Review — ✅ no blocking findings

Uh oh!

iamwhatever left a comment

Choose a reason for hiding this comment

Uh oh!

chenmingwei23 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bolichen97 commented Sep 4, 2026

Open PR relationship audit

Relationship findings

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bolichen97 commented Sep 1, 2026 •

edited

Loading

github-actions Bot commented Sep 1, 2026 •

edited

Loading

github-actions Bot commented Sep 1, 2026 •

edited

Loading

github-actions Bot commented Sep 1, 2026 •

edited

Loading

github-actions Bot commented Sep 1, 2026 •

edited

Loading

github-actions Bot commented Sep 1, 2026 •

edited

Loading