Skip to content

GitHub actions build pipeline#1

Merged
cgillum merged 4 commits into
mainfrom
build-validation
Jan 12, 2022
Merged

GitHub actions build pipeline#1
cgillum merged 4 commits into
mainfrom
build-validation

Conversation

@cgillum
Copy link
Copy Markdown
Member

@cgillum cgillum commented Jan 10, 2022

Enables automatic build validation for all PRs and commits to main.

This PR will also include work to run tests with a self-hosted sidecar.

Enables automatic build validation for all PRs and commits to `main`.
- Self-host sidecar in test process
- gRPC channel sharing to make tests faster
- Improved logging
- Updated protobuf package version
@cgillum cgillum merged commit 0998d17 into main Jan 12, 2022
@cgillum cgillum deleted the build-validation branch January 12, 2022 00:05
@cgillum cgillum restored the build-validation branch March 10, 2022 02:01
@jviau jviau deleted the build-validation branch October 11, 2023 18:38
YunchuWang added a commit that referenced this pull request Mar 27, 2026
Issue #1 (shell cmd over-reliance):
  Every shell block now has an "Else (no terminal)" fallback that
  instructs the agent to infer from visible context only and to
  explicitly label each inference — preventing fake command output.

Issue #3 (evidence anti-leak):
  Added Evidence Integrity section: do not fabricate file paths,
  line numbers, or snippets. If exact line unknown → cite file only.
  If snippet unavailable → state "snippet not verified". If nothing
  concrete → do not write the rule.

Issue #4 (rigid structure kills reasoning):
  Plan template changed from "Output this exact structure" to
  "Use this structure if applicable. Do NOT fabricate content to
  fill fields." Fields with no evidence write INSUFFICIENT EVIDENCE.
  Added TERMINAL ACCESS field so confidence level is explicit.

Issue #5 (no cost model):
  Added Cost Model section: each file adds maintenance cost,
  cognitive load, and contradiction risk. Rule: only add when
  value > long-term cost. This is now also a required field in
  the plan (Cost justification per proposed change).

Issue #6 (staleness not a decision condition):
  Staleness Filter is now a pre-write gate, not just post-run output.
  If a rule is stale-prone within 3 months → move to skill or omit.
  Stale risk is also a required field in the plan per proposed change.

Issue #7 (conflict arbitration incomplete):
  Added Conflict Resolution Priority table (1=copilot-instructions.md
  highest, 5=agents lowest). Each proposed change now includes
  "Authority level: [1-5]" in the plan. Never duplicate across levels.

Issue #8 ("under 100 lines" is arbitrary):
  Changed to "Prefer under 100 lines. If longer, justify why each
  section earns its place." Applied same principle to path
  instructions (<60), skills (<150).

Issue #9 (commit fallback already OK, confirmed):
  No change needed — fallback was already added in prior round.

Co-Authored-By: Claude <noreply@anthropic.com>
torosent added a commit that referenced this pull request May 8, 2026
…validation, helper-conflict throw, line endings

Closes review findings #1, #5, #6, #8 from PR #695:

- ToVersionSuffix collision (#1): the previous encoder left underscore unescaped, so '1.0' and '1_x002E_0' both produced '_1_x002E_0'. Now every non-alphanumeric character (including '_') is escaped as '_xHHHH_', making the encoding injective and avoiding silent generated-helper collisions.

- Whitespace version validation (#5): DurableTaskVersionAttribute and the versioned AddOrchestrator/AddActivity overloads now reject whitespace-only version strings (null/empty still means 'unversioned'). The source generator emits a new error diagnostic DURABLE3005 so the misconfiguration is caught at compile time rather than first instance construction.

- Helper-conflict throw (#8): generated helpers ApplyGeneratedVersion / ApplyGeneratedActivityVersion now throw InvalidOperationException at runtime when a caller-supplied options.Version disagrees with the version baked into the generator-emitted helper name. Previously the caller's version silently won, defeating the purpose of the version-suffixed method. Updated the generator-output snapshot tests accordingly.

- TaskOptions.cs CRLF/LF normalization (#6): the file was the only one in the repo with mixed/CRLF line endings, polluting the PR diff. Normalized to LF to match the rest of the codebase.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
torosent added a commit that referenced this pull request May 8, 2026
…flict, TaskVersion null-safety, integration test refresh, migration recipe

Closes the second-pass review findings from PR #695:

#1 — Stale tag references in integration tests: VersionedClassSyntaxTestOrchestration.cs and VersionedClassSyntaxIntegrationTests.cs were still using the pre-rename 'microsoft.durabletask.activity.explicit-version' constant. The spoof-protection test in particular was passing for the wrong reason since the SDK no longer recognizes that key. Updated both files to use ActivityVersioning.VersionSourceTagName / ExplicitSource / InheritedSource and added IVT for Worker.Grpc.IntegrationTests in Worker.csproj so the integration tests can reference the SDK constants directly.

#2 — Generator helper conflict check missed explicit-unversioned: ApplyGeneratedVersion (StartOrchestrationOptions / SubOrchestrationOptions) and ApplyGeneratedActivityVersion all used patterns that required '.Version.Length > 0' or '!IsNullOrWhiteSpace', which excluded TaskVersion.Unversioned. A user calling CallProcessPayment_2Async(..., new ActivityOptions { Version = TaskVersion.Unversioned }) silently got v2 instead of the contradicting throw the helper was supposed to enforce. Now the patterns match any non-null Version (including the empty/unversioned case) and the diagnostic message distinguishes 'explicit-unversioned' from a specific version. Generator-output snapshot tests updated.

#3 — TaskVersion null-storage caused null/empty mismatch and GetHashCode crash: the constructor stored 'null' verbatim when given null. Effects: TaskVersion.Unversioned.Equals(new TaskVersion('')) was false, and TaskVersion.Unversioned.GetHashCode() called StringComparer.OrdinalIgnoreCase.GetHashCode(null) which throws — making any user who put TaskVersion in a dictionary key crash at runtime. Constructor now normalizes null/empty to string.Empty; Equals and GetHashCode are null-safe and treat null and empty as identical. (default(TaskVersion) still has Version=null at the field level — that's a struct-default constraint — but Equals/GetHashCode handle it.)

#4 — Whitespace TaskVersion ctor validation: the registry rejected whitespace-only TaskVersion, but new TaskVersion('  ') itself accepted the value, so it could leak into ActivityOptions / StartOrchestrationOptions / SubOrchestrationOptions and route silently to 'no exact match'. The constructor now throws ArgumentException for non-empty whitespace. The redundant ValidateRegistrationVersion method and the explicit whitespace check in DurableTaskVersionAttribute were removed because TaskVersion's constructor handles them centrally.

#7 — Migration recipe: added a new section to the proposal documenting the 'add [DurableTaskVersion] to an existing class' migration path. Recommended recipe is keep the unversioned class registered until in-flight unversioned instances drain or ContinueAsNew to the new version. Reverse migration (removing [DurableTaskVersion]) is documented as unsupported.

#8 — Tag rename pre-release note: added a one-paragraph note in the proposal acknowledging the in-flight rename from 'explicit-version' (boolean) to 'version-source' ('explicit'/'inherited') and recommending pre-release deployments drain before pointing at the new contract since the worker now fail-closes on missing tag for versioned activity requests.

Tests: 84 generator + 127 worker.tests + 135 worker.grpc.tests + 159 abstractions.tests + 8 integration = 513 total, all passing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

1 participant