Skip to content

rescue: unpushed work from station01 worktree (fix/E-00051-safe-reads-remediation-3) - #90

Open
andrei-hasna wants to merge 8 commits into
mainfrom
fix/E-00051-safe-reads-remediation-3
Open

rescue: unpushed work from station01 worktree (fix/E-00051-safe-reads-remediation-3)#90
andrei-hasna wants to merge 8 commits into
mainfrom
fix/E-00051-safe-reads-remediation-3

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Rescued unpushed work from a station01 worktree

This branch carried commits that existed on no remote — they lived only in a
worktree on station01 and would have been lost with that disk. Pushing them is a
rescue, not new work.

Found by the station01 worktree census (worktree-fleet-cleanup). The branch was
verified to hold genuinely unique commits: after a fresh git fetch --all (no
--prune, so the count can only fall), git rev-list --count HEAD --not --remotes
still returned a non-zero count.

This PR has not been reviewed and its base has moved since the branch was cut.
Treat it as a recovered artifact to triage, not as a ready-to-merge change. If the
work is obsolete, close the PR — the branch is now backed up on the remote either way.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #90 @ 9507704 — lens: correctness+security+gates, reviewer unresolved-account003 (1 of 1)

Gates run against the exact reviewed head:

  • bun install — exit 0; setup only, 168 packages installed.
  • bun run typecheck — exit 0; tsc --noEmit emitted no itemized pass/fail counts.
  • bun run test — exit 0; 1057 pass, 0 fail, 3741 assertions across 64 files.

Read:

  • git log --oneline origin/main..HEAD and git diff origin/main...HEAD --stat against freshly fetched origin/main at c1f85e7d1f68e050046bab10a0b79f6cde9cc43a.
  • The full diff of all 83 changed files plus surrounding implementation in the message preview/search/export paths, Store adapters, CLI/MCP consumers, HTTP server, incident projector, SQLite/PostgreSQL migrations, SDK/OpenAPI, and tests.

Blocking P0/P1 findings:

  • P1 — the supported cloud search path is broken by an internal query-contract mismatch. ApiStore.searchMessagePreviews() sends order=relevance whenever callers omit sort (the documented default) or explicitly request relevance, but GET /v1/messages accepts only order=asc|desc and the server regression suite explicitly pins order=relevance as HTTP 400. This is reached by the MCP search_messages tool and CLI search when cloud storage is selected. The same adapter forwards the declared until filter, but the server neither parses nor applies until, so once ordering is repaired the cloud path would silently return messages newer than the caller's upper bound. Remedy: send a server-supported order for cloud search and add strict server-side until parsing plus a created_at <= predicate, with adapter and server regressions.

Non-blocking follow-ups:

  • P2 — ChatView increments selectedIndex from 0 to 1 when the first sent or polled preview is appended to an empty list, leaving no selected row until the user presses Up. This is recoverable and does not meet the blocking threshold.
  • No concrete secret exposure, unsafe mutation, data-integrity defect, or additional P0/P1 security finding was found in the reviewed candidate.

Agent: unresolved-account003
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REMEDIATION] #90 @ a466636

Fixed the live cloud-search contract defect named in the NO_GO review:

  • ApiStore now sends the server-supported chronological order while preserving the requested until bound.
  • The server now strictly validates and applies until, so cloud search cannot silently widen the caller's time range.
  • OpenAPI and the generated SDK now publish both since and until for message collection reads.
  • Added regression coverage at the adapter, server, and OpenAPI boundaries.

Focused validation:

  • bun run test -- src/lib/store/api-store.test.ts src/server/api.test.ts src/server/openapi.test.ts — exit 0; 45 pass, 0 fail.
  • bun run sdk:generate — exit 0; generated 26 operations.
  • bun run sdk:check — exit 0; SDK current with 26 operations.
  • bun run typecheck — final exit 0.
  • bun run test — exit 0; 1060 pass, 0 fail, 3753 expect calls across 64 files.
  • git diff --cached --check — exit 0.
  • gitleaks git --staged --redact --no-banner . — exit 0; no leaks found.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] GO — #90 @ a466636 — lens: correctness+security+gates, reviewer unresolved-account003 (1 of 1)

What I read:

  • git log --oneline origin/main..HEAD against freshly fetched origin/main c1f85e7.
  • git diff origin/main...HEAD --stat and the full diff of every changed file.
  • Surrounding production source and regression tests across the message preview/export, CLI/TUI, API/Store, server, OpenAPI/SDK, incident projection, SQLite/PostgreSQL migration, and MCP paths.
  • The focused remediation at a466636.

Commands and exact results:

  • bun install — exit 0; setup only; 168 packages installed.
  • bun run typecheck at 9507704 — exit 0; tsc emitted no pass/fail count.
  • bun run test at 9507704 — exit 0; 1057 pass, 0 fail, 3741 expect calls across 64 files.
  • bun run test -- src/lib/store/api-store.test.ts src/server/api.test.ts src/server/openapi.test.ts — exit 0; 45 pass, 0 fail, 252 expect calls across 3 files.
  • bun run sdk:generate — exit 0; generated 26 operations.
  • bun run sdk:check — exit 0; SDK current with 26 operations.
  • First remediation bun run typecheck — exit 2; the new OpenAPI test cast a readonly parameter array as mutable. Corrected that exact test-only type annotation.
  • Final bun run typecheck — exit 0; tsc emitted no pass/fail count.
  • Final bun run test — exit 0; 1060 pass, 0 fail, 3753 expect calls across 64 files.
  • git diff --cached --check — exit 0.
  • gitleaks git --staged --redact --no-banner . — exit 0; no leaks found.

Blocking P0/P1 findings:

  • None on the current head. The prior reachable P1 was fixed: cloud search no longer sends the rejected order=relevance value, and its until bound is now strictly validated, applied by the server, and represented in OpenAPI and the generated SDK.

Non-blocking follow-up:

  • P2: ChatView can advance selectedIndex from 0 to 1 when the first message is appended to an initially empty list, leaving no selected row until the user presses Up. This is recoverable UI behavior and is not material to this PR's safe-read acceptance gates.

@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #90 @ a466636 — lens: correctness+security+gates, reviewer unresolved-account003 (1 of 1)

This supersedes my earlier GO at the same SHA because the required merge gate had not yet been exercised. The code and repository-declared gates remain green, and the cloud-search P1 remains fixed, but the exact requested merge command exposed a current, material merge blocker.

Blocking P0/P1 finding:

  • P1 merge gate — gh pr merge 90 --repo hasna/conversations --squash --delete-branch exited 1: GitHub could not create the merge commit cleanly. Fresh state reports head a466636, base c1f85e7, mergeable CONFLICTING, mergeStateStatus DIRTY. An independent git merge-tree --write-tree origin/main HEAD check also exited 1 and identified 33 content conflicts across CHANGELOG/package metadata plus the CLI, Store, message core, MCP, SDK, and server paths.

Required remedy:

  • Reconcile the branch with current main as real integration work, preserving both sides' intended behavior across all 33 conflicted files.
  • Re-run bun run typecheck and bun run test on the resolved merge candidate.
  • Run a staged secrets scan before committing and pushing the reconciliation.
  • Re-review the exact new head and merge result before attempting merge again.

Non-blocking follow-up remains unchanged:

  • P2: ChatView can temporarily have no selected row after the first message is appended to an initially empty list; Up recovers it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant