test(tasks): preserve authoritative reply SLA mutation response - #1648
test(tasks): preserve authoritative reply SLA mutation response#1648seonghobae wants to merge 5 commits into
Conversation
|
Warning Review limit reachedNext included review available in 46 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
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 |
seonghobae
changed the title
|
@coderabbitai review Please review exact protected base |
|
@codex review Review exact protected base |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="042b0c70531b229af3acbd0421a2f23098d848b3"
head="aa0aa733cc591e5b46376bbb6293d16efebe5530"
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
printf '%s\n' '--- ancestry and effective files ---'
git merge-base --is-ancestor "$base" "$head"
git diff --name-status "$base" "$head"
printf '%s\n' '--- exact test diff ---'
git diff --unified=80 "$base" "$head" -- frontend/src/components/TasksLayout.test.tsx
printf '%s\n' '--- locate candidate implementation files ---'
fd -a -t f 'TasksLayout\.(tsx|test\.tsx)$|task.*\.(py|ts|tsx)$' . | sed -n '1,240p'
printf '%s\n' '--- component structural outline ---'
ast-grep outline frontend/src/components/TasksLayout.tsx --items all 2>/dev/null || true
printf '%s\n' '--- endpoint and response merge references ---'
rg -n -C 5 -P 'reply-sla-escalations|ReplySlaEscalationResponse|reply_sla|setTasks\s*\(|new Map' \
--glob '*.{py,ts,tsx}' . \
-g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' | sed -n '1,520p'Length of output: 46379 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- TasksLayout types, fetch, escalation, and task map ---'
rg -n -C 12 -P 'ReplySlaEscalationResponse|reply-sla-escalations|setTasks\s*\(|new Map|response\.tasks|escalation' \
frontend/src/components/TasksLayout.tsx | sed -n '1,460p'
printf '%s\n' '--- TasksLayout test harness and added test ---'
sed -n '1,240p' frontend/src/components/TasksLayout.test.tsx
printf '%s\n' '--- backend mutation, commit/refresh, and returned entries ---'
sed -n '70,330p' backend/services/reply_sla_escalation_service.py
printf '%s\n' '--- backend API response conversion ---'
sed -n '45,175p' backend/api/tasks.py
printf '%s\n' '--- backend regression coverage for existing reply-SLA tasks ---'
rg -n -C 12 -P 'existing|update|blocked|urgent|created.*0|reply.sla' backend/tests/test_tasks_api.py | sed -n '1,560p'
printf '%s\n' '--- exact head file metadata ---'
git show --no-patch --format=fuller aa0aa733cc591e5b46376bbb6293d16efebe5530
git diff --check 042b0c70531b229af3acbd0421a2f23098d848b3 aa0aa733cc591e5b46376bbb6293d16efebe5530Length of output: 45267
I found no valid findings. The delta changes only The regression loads an existing
The backend updates non- I did not execute the test suite. The sandbox does not permit repository code execution.
|
|
@coderabbitai review Superseding the direct-develop request after canonical owner restack: review exact base |
|
@codex review Review current exact base |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="17a7618eda2b212b691f08fa936e042b34258fc9"
head="8ad819786c6c2a99e0a158d2192054eb8189b2f4"
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
git merge-base --is-ancestor "$base" "$head"
printf '%s\n' '--- exact comparison ---'
git diff --name-status "$base" "$head"
git diff --check "$base" "$head"
git diff --unified=100 "$base" "$head" -- frontend/src/components/TasksLayout.test.tsx
printf '%s\n' '--- relevant frontend structure ---'
ast-grep outline frontend/src/components/TasksLayout.tsx --items all
ast-grep outline frontend/src/components/TasksLayout.test.tsx --items all
printf '%s\n' '--- frontend escalation request and response merge ---'
rg -n -C 14 -P 'reply-sla-escalations|ReplySlaEscalationResponse|mergedTasks|setTasks\s*\(|response\.tasks' \
frontend/src/components/TasksLayout.tsx
printf '%s\n' '--- test scenario context ---'
rg -n -C 30 -P 'reply_sla|reply-sla-escalations|created\s*:\s*0|blocked|urgent|stale|refreshed' \
frontend/src/components/TasksLayout.test.tsx
printf '%s\n' '--- backend candidate files ---'
fd -a -t f 'reply_sla_escalation_service\.py|tasks\.py' backend
printf '%s\n' '--- backend mutation and API response ---'
rg -n -C 14 -P 'ReplySlaEscalationResponse|reply_sla|blocked|urgent|refresh|commit|tasks=' \
backend/services/reply_sla_escalation_service.py backend/api/tasks.pyLength of output: 47479
I found no valid findings. The effective delta changes only The new test starts with an existing
mergedTasks.set(task.id, task)This operation replaces the stale local row with the same-id response row. The test requires the refreshed title and rejects the stale title. It therefore protects response-wins behavior. It does not encode the rejected first-wins hypothesis. The backend updates a non- I treated all pre-retarget CI and review evidence as stale. I did not execute repository tests because the sandbox does not allow repository code execution.
|
1 participant
Current authority — 2026-09-10
#1623@17a7618eda2b212b691f08fa936e042b34258fc9429bc8c7cbaa8d9ef88a820b5ecefbc73da0e0d4a438a5d9fdb903e48e8a675c624de91d720c2f60aa0aa733cc591e5b46376bbb6293d16efebe55304b063f4d6dc4ffdafb8d76d81fdf8ddf951e75a38ad819786c6c2a99e0a158d2192054eb8189b2f4frontend/src/components/TasksLayout.test.tsx(+56/-0).Finding validation
The original proposal treated a repeated task id in
POST /api/tasks/reply-sla-escalationsas a duplicate whose first local value should win. That contract is incorrect for this endpoint.The canonical backend service intentionally mutates an existing non-done reply-SLA task when it is escalated: it refreshes the title from the current email subject, changes status to
blocked, priority tourgent, updates thread identity and timestamp, commits, refreshes persisted task rows, and returns those rows inReplySlaEscalationResponse.tasks. A matching id in that mutation response is therefore newer server authority, not an untrusted duplicate to suppress.The generated
429bc8c...production change would have kept the stale row already loaded byGET /api/tasksand discarded those legitimate server-side updates, making the board disagree with persistence immediately after the customer triggered escalation.Repair and owner topology
a438a5d...preserved the rejected generated head as first-parent provenance while adopting the exact protected tree; the incorrect first-wins production mutation was removed without force-push or destructive history rewriting.aa0aa733...salvaged the useful scenario as a regression that requires same-id escalation results to replace stale local state.The direct-
developcandidate then inherited the known frontend dependency-security failure owned by #1623. That is not a Tasks response-merge defect. Ordinary two-parent4b063f4d...adopts current #1623 as the canonical dependency/security parent;8ad81978...restores only the response-authority regression. No #1623 dependency, lockfile, or security-test source is copied into this lane.The exact test loads an existing
reply_slatask, exercises the real escalation action, returns the same opaque task id with refreshed title/status/priority/timestamp andcreated: 0, and requires the UI to replace the stale title with the mutation response.TasksLayout.tsxhas no effective delta here; response-wins behavior remains canonical product code.Evidence provenance
All workflow evidence admitted while this PR still targeted
developis predecessor evidence after the base retarget, even when the head ancestry contains the same test. In particular, the direct-develop Security Scan failure is inherited dependency-owner evidence, and the Application CI/Bandit/Semgrep success seen on earlier heads does not transfer to this current(base SHA, head SHA)identity.Keep Draft until #1623 is protected-integrated or canonical stacked verification produces a qualifying post-retarget exact-base/head receipt, zero valid unresolved findings remain, and a qualifying post-last-push independent review exists. No self-approval, force-push, destructive rebase, dummy requeue, stale-evidence transfer, source-owner duplication, or gate weakening.