Skip to content

Python: update ChatKit attachment handling - #7846

Merged
SergeyMenshykh merged 6 commits into
microsoft:mainfrom
SergeyMenshykh:sergeymenshykh-investigate-attachment-paths
Aug 25, 2026
Merged

Python: update ChatKit attachment handling#7846
SergeyMenshykh merged 6 commits into
microsoft:mainfrom
SergeyMenshykh:sergeymenshykh-investigate-attachment-paths

Conversation

@SergeyMenshykh

Copy link
Copy Markdown
Contributor

Motivation & Context

Improve attachment handling consistency in the ChatKit integration sample.

Description & Review Guide

  • What are the major changes? Updates request handling and analysis annotations.
  • What is the impact of these changes? Attachment lifecycle checks are applied consistently.
  • What do you want reviewers to focus on? The updated request flow.

Related Issue

Follow-up to #7038.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 104d342a-090c-4feb-9bb3-662915b9c390
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 104d342a-090c-4feb-9bb3-662915b9c390
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 104d342a-090c-4feb-9bb3-662915b9c390
Copilot AI balanced review requested due to automatic review settings August 24, 2026 17:07
@SergeyMenshykh
SergeyMenshykh deployed to github-app-auth August 24, 2026 17:08 — with GitHub Actions Active
@SergeyMenshykh
SergeyMenshykh deployed to github-app-auth August 24, 2026 17:08 — with GitHub Actions Active
@SergeyMenshykh
SergeyMenshykh deployed to github-app-auth August 24, 2026 17:08 — with GitHub Actions Active
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Aug 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Improves ChatKit attachment lifecycle validation and preview metadata handling.

Changes:

  • Validates attachment existence and upload completion before writing.
  • Uses stored MIME types for previews and adds CodeQL annotations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
attachment_store.py Annotates validated path resolution.
app.py Adds lifecycle checks and metadata-backed previews.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/samples/05-end-to-end/chatkit-integration/app.py Outdated
Comment thread python/samples/05-end-to-end/chatkit-integration/attachment_store.py Outdated
Comment thread python/samples/05-end-to-end/chatkit-integration/app.py

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (4 commit(s)): a428806cb382, 67c7eb083e05, 524c77a7cd3b, 81da60b38274
Model: gpt-5.6-sol

Overview

The PR strengthens attachment handling by validating metadata before upload and preview, preserving path containment checks, and rejecting sequential replay. However, the upload route reads a field that does not exist in the locked ChatKit model, so valid uploads fail before reaching its error handler. Even with the correct field, the separate load/check/write/save sequence does not enforce single completion under concurrency.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (1 high, 1 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/samples/05-end-to-end/chatkit-integration/app.py

Comment thread python/samples/05-end-to-end/chatkit-integration/app.py Outdated
Comment thread python/samples/05-end-to-end/chatkit-integration/app.py
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 104d342a-090c-4feb-9bb3-662915b9c390
Comment thread python/samples/05-end-to-end/chatkit-integration/app.py Outdated
Comment thread python/samples/05-end-to-end/chatkit-integration/app.py Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 104d342a-090c-4feb-9bb3-662915b9c390
@SergeyMenshykh
SergeyMenshykh deployed to github-app-auth August 25, 2026 09:01 — with GitHub Actions Active
@SergeyMenshykh
SergeyMenshykh added this pull request to the merge queue Aug 25, 2026
Merged via the queue into microsoft:main with commit 7181af5 Aug 25, 2026
34 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in Agent Framework Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants