Skip to content

Preserve document task progress when form fields need correction - #189

Open
madara-1112 wants to merge 1 commit into
CopilotKit:mainfrom
madara-1112:fix/document-input-recovery
Open

madara-1112 wants to merge 1 commit into
CopilotKit:mainfrom
madara-1112:fix/document-input-recovery

Conversation

@madara-1112

Copy link
Copy Markdown

What changed

Document tasks previously failed when delegated input contained an unknown PDF field or a value with the wrong type. Retrying reused the same invalid input, so the task failed again even after it had already found and checkpointed the source document.

This change validates delegated values against the PDF's actual field definitions before filling it. When only part of the input is invalid, the task now:

  • keeps the checkpointed source document and fields that already passed validation;
  • enters waiting_input with field-specific errors instead of failing;
  • pre-fills the valid values in the task UI; and
  • merges the person's corrections before resuming the same task and continuing to action review.

The implementation deliberately does not coerce strings such as "true" into booleans. A value with the wrong type still requires an explicit correction.

Verification

  • pnpm lint
  • pnpm typecheck
  • pnpm test (485 passed)
  • pnpm build:server
  • pnpm build:web
  • pnpm build:ios
  • pnpm build:android
  • pnpm --dir apps/worker typecheck
  • Manually verified in the fictional local workspace: retry a document task with four valid values and one string-valued checkbox, preserve the original PDF and valid values, correct only the checkbox, continue to review, approve the local email action, and complete the same task.

Regression coverage includes both a wrong field type and an unknown field name, and verifies that no filled artifact is created until the invalid input is corrected.

Integration limits

  • The manual flow used local sample mail and a fictional fillable PDF; no Google/Gmail provider credentials were used.
  • Web UI behavior was exercised manually. iOS and Android exports passed, but native binaries were not run.

@jerelvelarde jerelvelarde left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed bb1e7c262017ee18c1b6b3cd9e87ae78446005d1. Keeping this open for integration, not closing it.

Value and template fit: recovering from invalid document field names/types without losing valid values is useful and fits the persisted document workflow. The source PDF remains checkpointed and corrections continue to the existing action-review gate. The description provides sample-workspace verification and native-device limits; please also attach the screenshot or short recording requested by the repository PR template for the changed recovery UI.

Security review: traced owner-scoped source retrieval, field-name/type validation, preservation of the original PDF, avoidance of filled artifacts before correction, and the subsequent email action-approval boundary. Error strings render as React text. No actionable security defect found in the reviewed diff. This is fixture/static review, not live-provider, adversarial-PDF or native-device testing.

Verification: all 17 workflows tests passed on this exact head in a read-only, network-disabled Linux container (Node 24.17.0 + tsx, telemetry disabled). Dependencies were installed from main's manifest rather than the pnpm lockfile. The PR's GitHub CI supplies lockfile-based checks.

Merge blocker: #191 has now merged and changes the same production AgentService.answer state transition from state.answer to accumulated state.answers. Applying this PR on the combined tree conflicts in that method; blindly taking this PR's state assignment would discard the new answer-history behavior. Please rebase, retain #191's question/answer accumulation and legacy migration, then layer document-field merging plus clearing missingFields/fieldErrors onto that state. Run both tests/model-outcome.test.ts and tests/workflows.test.ts together, refresh CI, and include the recovery UI evidence. This is an integration hold rather than a demonstrated standalone security finding.

@madara-1112
madara-1112 force-pushed the fix/document-input-recovery branch from bb1e7c2 to ce7e753 Compare October 26, 2026 07:42
@madara-1112

madara-1112 commented Oct 26, 2026 •

Copy link
Copy Markdown
Author

Rebased onto the latest main and resolved the overlap with #191.
The updated AgentService.answer preserves #191’s accumulated { question, answer } history and legacy state.answer migration. Document tasks additionally merge corrected fields and clear missingFields / fieldErrors. I also added an integration assertion confirming that a recovered Document task retains its state.answers history.
Local validation:

  • Combined model/workflow tests: 23 passed
  • Full test suite: 511 passed
  • pnpm lint
  • pnpm typecheck
  • pnpm --dir apps/worker typecheck
  • pnpm build:server

The upstream GitHub Actions run is currently marked action_required and is waiting for maintainer approval.
Could you please re-review the rebased commit when convenient?

This branch has not been deployed

No deployments
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.

2 participants