Skip to content

refactor(project): tighten the ProjectEvent and create() contracts - #1969

Merged
notgitika merged 1 commit into
refactorfrom
chore/project-event-shape
Aug 11, 2026
Merged

refactor(project): tighten the ProjectEvent and create() contracts#1969
notgitika merged 1 commit into
refactorfrom
chore/project-event-shape

Conversation

@notgitika

Copy link
Copy Markdown
Contributor

Groundwork for agentcore project build, split out so the contract change is reviewable on its own.

What

ProjectEvent was a three-field bag with every field optional:

export type ProjectEvent = {
  message?: string;
  subprocessOutput?: string;
  project?: Project;
};

subprocessOutput and project were never written and never read — I checked every producer and consumer. Only message was ever used, and because it was optional the create handler had to guard it (if (event.message)) on every iteration. So the type is now just:

export type ProjectEvent = { message: string };

and the guard in handlers/project/create/index.ts is gone.

create also declared AsyncGenerator<ProjectEvent>, leaving TReturn as the default any. It is now AsyncGenerator<ProjectEvent, Project>.

The type hole that tightening exposed

Naming the return type surfaced a real mismatch: create was building its return value out of TemplateSpec.runtimes, which is unknown[], while Project.runtimes is ProjectRuntime[]. any had been hiding it.

Rather than cast, create now reads the project back through resolve():

const project = await this.resolve({ filePath: destination });

That honours a comment already sitting in the code ("Return the same shape resolve() would: a created project is a resolvable one"), and means the returned runtimes are schema-validated instead of being the template's loosely-typed spec sections passed through unchecked.

The matching as Project cast in manager.test.ts is dropped too, so the test now type-checks the real thing.

Verification

  • bun test — 1061 pass, 0 fail
  • tsc --noEmit clean, oxlint clean

No behaviour change; the only user-visible difference is that progress messages are no longer conditionally skipped, and they were never absent in practice.

ProjectEvent was a bag of three optional fields, two of which
(subprocessOutput, project) had no producer and no consumer. Narrow it to
the single field that is actually emitted and make it required, so
consumers stop guarding on `if (event.message)`.

Typing create()'s return as Project, rather than leaving the generator's
TReturn as `any`, surfaced a latent hole: the returned runtimes came
straight from the template's loosely-typed `unknown[]` spec section. Read
the project back through resolve() instead, which validates against
ProjectSpecSchema and does what the original comment already claimed.
@github-actions github-actions Bot added agentcore-harness-reviewing AgentCore Harness review in progress and removed agentcore-harness-reviewing AgentCore Harness review in progress labels Aug 11, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.80%. Comparing base (d90fa2b) to head (049d64b).
⚠️ Report is 1 commits behind head on refactor.

Additional details and impacted files
@@            Coverage Diff            @@
##           refactor    #1969   +/-   ##
=========================================
  Coverage     96.80%   96.80%           
=========================================
  Files           306      306           
  Lines         17073    17073           
=========================================
  Hits          16527    16527           
  Misses          546      546           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Hweinstock Hweinstock 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.

LGTM!

// schema-validated instead of the template's loosely-typed spec sections.
const project = await this.resolve({ filePath: destination });
if (!project) {
throw new ProjectStateError(

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.

OOS here but ProjectStateError feels pretty vague to me. What do you think of replacing it with something like ProjectValidationError for when reading a project fails?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ahh yes that makes sense, I will make a note and have it out as a followup!

@notgitika
notgitika merged commit 7537b98 into refactor Aug 11, 2026
13 checks passed
@notgitika
notgitika deleted the chore/project-event-shape branch August 11, 2026 19:03
This was referenced Sep 3, 2026
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.

4 participants