Skip to content

fix(web): bind approval decisions to rendered request - #3712

Closed
djm204 wants to merge 3 commits into
mainfrom
resolve/issue-3663-bind-approval-card-decisions-to-a-single-approva
Closed

fix(web): bind approval decisions to rendered request#3712
djm204 wants to merge 3 commits into
mainfrom
resolve/issue-3663-bind-approval-card-decisions-to-a-single-approva

Conversation

@djm204

@djm204 djm204 commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • bind approval callbacks to the rendered session/request timestamp/command/token scope
  • lock both approval controls synchronously after the first decision
  • carry approval identity over REST and WebSocket and reject stale decisions server-side
  • preserve retry behavior after an inline approval error
  • add component, hook, API, REST, and WebSocket regression coverage

Test plan

  • npm test --workspace @franken/web -- --run tests/components/approval-card.test.tsx tests/hooks/use-chat-session.test.ts tests/lib/api.test.ts (77 passed)
  • npm test --workspace @franken/orchestrator -- --run tests/unit/http/chat-routes-approval.test.ts tests/integration/chat/ws-chat-server.test.ts (55 passed)
  • npm run typecheck --workspace @franken/types
  • npm run typecheck --workspace @franken/web
  • npm run typecheck --workspace @franken/orchestrator
  • npm run lint --workspace @franken/web (0 errors; pre-existing warnings)
  • npm run lint --workspace @franken/orchestrator (0 errors; pre-existing warnings)
  • npm run build --workspace @franken/web
  • npm run build --workspace @franken/orchestrator

Closes #3663

@djm204

djm204 commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17232a5e09

ℹ️ 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".

Comment thread packages/franken-web/src/components/approval-card.tsx
@djm204

djm204 commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e0dacbc02

ℹ️ 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".

Comment thread packages/franken-orchestrator/src/http/ws-chat-server.ts
Comment thread packages/franken-web/src/hooks/use-chat-session.ts
Comment thread packages/franken-orchestrator/src/chat/approval-input.ts
@djm204

djm204 commented Jul 24, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6784a9f805

ℹ️ 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".

Comment thread packages/franken-orchestrator/src/http/ws-chat-server.ts
Comment thread packages/franken-web/src/hooks/use-chat-session.ts
@djm204

djm204 commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

Closing without merge after fresh closeout review of immutable head 6784a9f8058588e2d5044534b0738ef333b86fe0.

Evidence:

  • Branch is 86 commits behind current main; mergeable/CLEAN, but it overlaps materially with later chat/conversation work.
  • Exact-head GitHub CI run 30065511186 is green in all four jobs, but dates to July 24 and does not address the remaining behavioral findings.
  • Fresh isolated-worktree verification: npm ci succeeded; root build passed 10/10 packages; types/web/orchestrator typechecks passed after the canonical workspace build; 77/77 web and 55/55 orchestrator focused tests passed; web/orchestrator lint completed with zero errors (existing warnings only); git diff --check passed.
  • Fresh Codex CLI closeout assessment validated three distinct residual P2 classes and advised against merging this head without repair: WebSocket stale-decision reconciliation, HTTP 409 stale-session refresh, and collision-prone/optional approval identity.
  • Residuals were filed and linked as Bug: reconcile WebSocket stale approval errors immediately #4129, Bug: refresh pending approval after stale HTTP 409 #4130, and Security: require a unique approval request identifier #4131. Every Codex thread was replied to and resolved; fully paginated audit reports zero unresolved Codex-authored threads.
  • No additional @codex review was triggered and no P2 implementation was added, consistent with the terminal closeout cap/policy.

The intended approval-binding work is still needed, but this stale branch is unsafe to merge as-is. Issue #3663 and the three concrete follow-ups remain open for a fresh current-main implementation. No admin or bypass merge was used.

@djm204 djm204 closed this Aug 9, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

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.

Bind approval card decisions to a single approval request

1 participant