Skip to content

Python: Support short flags in docker command - #8644

Merged
westey (westey-m) merged 2 commits into
microsoft:mainfrom
westey-m:python-docker-support-short-flags
Sep 22, 2026
Merged

westey (westey-m) merged 2 commits into
microsoft:mainfrom
westey-m:python-docker-support-short-flags

Conversation

@westey-m

Copy link
Copy Markdown
Collaborator

Motivation & Context

Description & Review Guide

  • What are the major changes?
  • What is the impact of these changes?
  • What do you want reviewers to focus on?

Related Issue

Fixes #

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.

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.

Copilot review overview

🟡 Changes recommended

-m, --memory, and --pids-limit can still override and remove configured resource limits.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Adds parsing and validation for attached or clustered Docker short flags.

Changes:

  • Detects blocked -u and -v forms.
  • Adds validation and argument-ordering tests.
File Description
python/​packages/​tools/​agent_framework_tools/​shell/​_docker.py Parses Docker short-option tokens.
python/​packages/​tools/​tests/​test_docker_shell_tool.py Tests short flags and argument ordering.

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

@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 (1 commit(s)): a834df9485b6
Model: gpt-5.6-sol-fast

Overview

The change adds Docker-aware parsing for attached and clustered short options, with fail-fast validation, immutable storage of validated arguments, and focused tests for the newly supported forms. Those guards correctly close the targeted -u/-v bypasses, but the token-by-token validation does not account for detached option values. As a result, some valid existing extra_run_args configurations now fail construction when a value begins with a blocked shorthand.

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

Affected areas: python/packages/tools/agent_framework_tools/shell/_docker.py

@westey-m
westey (westey-m) added this pull request to the merge queue Sep 22, 2026
Merged via the queue into microsoft:main with commit 9ad34d9 Sep 22, 2026
42 checks passed
@westey-m
westey (westey-m) deleted the python-docker-support-short-flags branch September 22, 2026 15:26

This branch was previously deployed

1 inactive deployment
github-app-auth — 13178890 Deployed Sep 22, 2026 by westey-m via add_label #23556
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

None yet

Development

Successfully merging this pull request may close these issues.

3 participants