Skip to content

feat(ignore matcher): support for .dockerignore for build context#673

Merged
james-rl merged 5 commits intomainfrom
james/ignore-matching
Dec 9, 2025
Merged

feat(ignore matcher): support for .dockerignore for build context#673
james-rl merged 5 commits intomainfrom
james/ignore-matching

Conversation

@james-rl
Copy link
Copy Markdown
Contributor

@james-rl james-rl commented Dec 8, 2025

Description

  • Adds support for .dockerignore files and provides scaffolding for other .ignore file types.

Motivation

  • Prevents accidental inclusion of deliberately excluded files when building a blueprint

Changes

  • Adds ignore matching. Note that this very closely mirrors the moby golang project ignore logic.

Testing

  • Unit tests added
  • Integration tests added
  • Smoke Tests added/updated
  • Tested locally

Breaking Changes

  • None

Checklist

  • PR title follows Conventional Commits format (feat: or feat(scope):)
  • Documentation updated (if needed)
  • Breaking changes documented (if applicable)

CodeAnt-AI Description

Add local directory build contexts with .dockerignore support

What Changed

  • You can now provide a local directory as a blueprint build context; the SDK uploads a gzipped tarball of the directory and attaches it to the Blueprint build.
  • Directory uploads respect ignore rules: the SDK reads a .dockerignore in the directory (or a provided ignore matcher, explicit patterns, or a custom ignore-file path) and excludes matching files from the tarball.
  • Introduced a docker-style ignore matcher that supports globs, ** double-star, parent-directory matches, and "!" exclusions, and added unit tests covering matcher behavior and upload filtering.
  • The directory upload API accepts a TTL for the temporary storage object and preserves existing behavior when a StorageObject is provided directly.

Impact

✅ Can attach local directory as build context
✅ Respects .dockerignore excludes during uploads
✅ Fewer accidental files included in blueprint builds

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Dec 8, 2025

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@codeant-ai codeant-ai bot added the size:L This PR changes 100-499 lines, ignoring generated files label Dec 8, 2025
@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Dec 8, 2025

Nitpicks 🔍

🔒 No security issues identified
⚡ Recommended areas for review

  • Memory / OOM Risk
    The code builds the entire gzipped tarball in memory (collects all chunks into a Buffer) before uploading. For large directories this can lead to high memory usage or OOM. Prefer streaming the tar directly into the HTTP request (or use a temporary file) so uploads scale for large contexts.

  • Directory-only semantics
    normalizePatterns currently strips trailing slashes from patterns. Docker semantics treat a trailing '/' as indicating a directory-only pattern. By removing the slash, the matcher loses that semantic information and cannot distinguish directory-only patterns from others. This may cause patterns that were meant only to match directories to behave differently. Consider preserving and propagating the trailing-slash information into the compiled pattern and making matching logic respect it.

  • Request options leakage
    The options parameter for uploadFromDir is typed as Core.RequestOptions & { ... } and the same object is passed directly to client.objects.create(...). Custom fields (ignore, dockerignorePath) may be included in the options object passed to the lower-level API call, which could cause unexpected behavior or errors. Strip SDK-only keys before forwarding to API client.

  • Options type leak / unexpected fields
    The code spreads options into the object passed to StorageObject.uploadFromDir and then adds ignore and dockerignorePath. It's cast to any, which can hide incompatible fields (e.g., polling config) that the upload call may not accept. This can lead to runtime errors or silently ignored settings.

  • Regex / glob edge-cases
    The custom globToRegExp implementation contains ad-hoc escaping and class handling that is easy to get subtly wrong (character-class escaping, backslashes, special characters). This can introduce incorrect matches for tricky patterns and character classes. Consider using a battle-tested library or tightening escaping/handling code and adding targeted tests for edge cases.

  • Unnormalized patterns
    The test constructs a DockerIgnoreMatcher directly with patterns that include leading/trailing slashes (e.g. "/foo", "/foo/bar/"). The matcher implementation expects patterns to be normalized (leading '/' removed, trailing '/' removed) when created via the factory. Instantiating the class directly with unnormalized patterns will likely produce incorrect regexes and cause these tests to fail or pass incorrectly.

  • Trailing-slash path handling
    The test asserts that a target path with a trailing slash ("foo/bar/") matches patterns that may have been supplied with a trailing slash. The matcher normalizes patterns (when using the factory) but the test constructs the matcher directly and does not normalize the incoming file path aside from removing "./". This mismatch can produce false assumptions — either the matcher should be created from normalized patterns or the test should normalize patterns/inputs consistently.

  • Brittle Test Assertion
    The new test inspects mockTar.create.mock.calls[0][0] and invokes the returned filter function without first asserting that mockTar.create was called and that the call args exist. If tar.create is not invoked or its signature changes, the test will throw with an index error. This makes the test fragile against implementation changes.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 8, 2025

✅ Object Smoke Tests & Coverage Report

Test Results

✅ All smoke tests passed

Coverage Results

Metric Coverage Required Status
Functions 100% 100%
Lines 85.48% - ℹ️
Branches 45.03% - ℹ️
Statements 83.33% - ℹ️

Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)

✅ All tests passed and all object methods are covered!

View detailed coverage report

Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.

📋 View workflow run

Comment thread src/lib/ignore-matcher.ts Outdated
private patterns: CompiledPattern[];

constructor(patterns: string[]) {
this.patterns = patterns.map((raw) => new CompiledPattern(raw));
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.

Suggestion: The DockerIgnoreMatcher constructor stores patterns exactly as passed in, without applying the same normalization as createIgnoreMatcher, so when it's used directly (as in your tests) patterns with leading slashes or trailing slashes (e.g. /foo, /foo/bar/) will never match normalized file paths like foo/bar, breaking the documented Docker-style semantics. [logic error]

Severity Level: Minor ⚠️

Suggested change
this.patterns = patterns.map((raw) => new CompiledPattern(raw));
const cleanedPatterns = normalizePatterns(patterns);
this.patterns = cleanedPatterns.map((raw) => new CompiledPattern(raw));
Why it matters? ⭐

This is a real logic bug. createIgnoreMatcher already calls normalizePatterns before constructing DockerIgnoreMatcher, but the class is public and can be constructed directly (e.g. in tests or other call sites). If callers pass patterns like '/foo' or './bar/', those will not match normalized file paths ('foo' / 'bar') because the class never normalizes inputs. Normalizing in the constructor keeps behavior consistent and matches the documented Docker semantics.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/lib/ignore-matcher.ts
**Line:** 124:124
**Comment:**
	*Logic Error: The `DockerIgnoreMatcher` constructor stores patterns exactly as passed in, without applying the same normalization as `createIgnoreMatcher`, so when it's used directly (as in your tests) patterns with leading slashes or trailing slashes (e.g. `/foo`, `/foo/bar/`) will never match normalized file paths like `foo/bar`, breaking the documented Docker-style semantics.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

Comment thread src/lib/ignore-matcher.ts

if (normalized.startsWith('./')) normalized = normalized.slice(2);
if (!normalized) normalized = '.';

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.

Suggestion: The matcher currently treats .dockerignore like any other file, so unless explicitly excluded by a pattern it will be included in the uploaded context, which diverges from Docker CLI behavior where the .dockerignore file itself is always excluded from the build context. [logic error]

Severity Level: Minor ⚠️

Suggested change
// Always ignore the .dockerignore file itself, matching Docker CLI behavior.
if (normalized === '.dockerignore') {
return true;
}
Why it matters? ⭐

Docker's CLI always excludes the .dockerignore file from the build context. The current implementation applies only the provided patterns and therefore will include '.dockerignore' unless a user pattern excludes it. Adding a small explicit check to always ignore '.dockerignore' reproduces Docker behavior and prevents accidental inclusion of the ignore file in contexts or archives produced by the tooling.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/lib/ignore-matcher.ts
**Line:** 133:133
**Comment:**
	*Logic Error: The matcher currently treats `.dockerignore` like any other file, so unless explicitly excluded by a pattern it will be included in the uploaded context, which diverges from Docker CLI behavior where the `.dockerignore` file itself is always excluded from the build context.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

@codeant-ai
Copy link
Copy Markdown
Contributor

codeant-ai bot commented Dec 8, 2025

CodeAnt AI finished reviewing your PR.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 8, 2025

✅ Object Smoke Tests & Coverage Report

Test Results

✅ All smoke tests passed

Coverage Results

Metric Coverage Required Status
Functions 100% 100%
Lines 85.48% - ℹ️
Branches 45.03% - ℹ️
Statements 83.33% - ℹ️

Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)

✅ All tests passed and all object methods are covered!

View detailed coverage report

Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.

📋 View workflow run

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 8, 2025

✅ Object Smoke Tests & Coverage Report

Test Results

✅ All smoke tests passed

Coverage Results

Metric Coverage Required Status
Functions 100% 100%
Lines 85.48% - ℹ️
Branches 45.03% - ℹ️
Statements 83.33% - ℹ️

Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)

✅ All tests passed and all object methods are covered!

View detailed coverage report

Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.

📋 View workflow run

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 8, 2025

✅ Object Smoke Tests & Coverage Report

Test Results

✅ All smoke tests passed

Coverage Results

Metric Coverage Required Status
Functions 100% 100%
Lines 85.46% - ℹ️
Branches 47.79% - ℹ️
Statements 83.36% - ℹ️

Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)

✅ All tests passed and all object methods are covered!

View detailed coverage report

Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.

📋 View workflow run

@james-rl james-rl requested a review from dines-rl December 8, 2025 22:19
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 9, 2025

⚠️ Object Smoke Tests & Coverage Report

Test Results

✅ All smoke tests passed

Coverage Results

Metric Coverage Required Status
Functions 28.03% 100%
Lines 29.29% - ℹ️
Branches 20.58% - ℹ️
Statements 28.84% - ℹ️

Coverage Requirement: 100% function coverage (all public methods must be called in smoke tests)

⚠️ Some object methods are not covered in smoke tests. Please add tests that call all public methods.

View detailed coverage report

Coverage reports are available in the workflow artifacts. Lines/branches/statements coverage is tracked but not required to be 100%.

📋 View workflow run

@james-rl james-rl merged commit a843d54 into main Dec 9, 2025
9 checks passed
@james-rl james-rl deleted the james/ignore-matching branch December 9, 2025 17:06
@stainless-app stainless-app bot mentioned this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants