feat(git): replace isomorphic-git with Effect Git - W-22711959 by mshanemc · Pull Request #888 · forcedotcom/source-tracking

mshanemc · 2026-10-16T03:54:53Z

What does this PR do?

Replace isomorphic-git with an Effect-native Git implementation while keeping the shadow repository at .sf/orgs/<orgId>/localSourceTracking.
Replace the ShadowRepo class with a factory and cached, stream-derived added / modified / deleted status; use recordSync for advancing the local baseline.
Preserve symlink targets and modes, atomically replace the index, and serialize commit ancestry under the index lock.
Migrate the existing worktree changes, including remote tracking and type/name resolution improvements, onto current main dependencies.

Configuration behavior and compatibility

Source-tracking settings are parsed and validated once from a ConfigProvider, then shared through a process-wide ManagedRuntime. Invalid values use documented defaults and emit lifecycle warnings; provider read failures still fail. Tests inject a provider or config layer without changing process.env.

Setting	Accepted value	Missing/invalid default
`SF_SOURCE_TRACKING_FD_PERMITS`	Positive safe integer. Deprecated batch-size aliases are capped at the platform default; an explicit canonical value is not capped.	`64` on Windows; `256` elsewhere
`SF_SOURCE_TRACKING_LOCK_TIMEOUT_SECONDS`	Positive finite number; fractions allowed.	`600` seconds
`SF_SOURCE_TRACKING_LOCK_AUTOCLEAR_SECONDS`	Positive finite number; fractions allowed.	`300` seconds
`SF_SOURCE_MEMBER_POLLING_TIMEOUT`	Positive finite number; fractions allowed. Without an override, timeout remains `Math.ceil(memberCount * 0.05) + 5` seconds.	Count-based timeout; 120 empty results
`SF_SOURCE_TRACKING_FILE_VERSION`	Non-negative safe integer, including zero. `1` selects the current format; other accepted values select the legacy format.	`0`

Numeric strings with radix prefixes (0x, 0b, 0o), non-finite values, malformed text, and whitespace are rejected and defaulted. Positive fractional timeouts remain valid; permits and file version require integers. With the normal environment provider, an empty string is treated as missing.

Boolean flags use Effect Config.Boolean literals: lowercase true/false, yes/no, on/off, y/n, and 1/0. Unlike Kit, parsing is case-sensitive; for example, TRUE warns and defaults to false, while lowercase yes now enables the flag. Other invalid non-empty values warn and default to false.

Legacy SFDX_DISABLE_SOURCE_MEMBER_POLLING and SFDX_SOURCE_MEMBER_POLLING_TIMEOUT remain supported with deprecation warnings. Canonical settings take precedence. The unused shadow-integrity-check flag was dropped.

Known limitation: stale-lock cleanup is mtime-based (five minutes by default); a competing writer could treat an unusually long live lock as stale.

Validation: yarn build, yarn test, yarn check:effect, and yarn check:circular-deps.

What issues does this PR fix or reference?

forcedotcom/salesforcedx-vscode#7174, forcedotcom/salesforcedx-vscode#7278, @W-22711959@

peternhale

Approve in the context that this change solves the event loop perf issues while consuming STL in vscode services extension.

WillieRuemmele · 2026-10-30T15:03:39Z

+    );
+    const ageMs = now - mtimeMs;
+    if (ageMs <= autoclearMs) return false;
+    yield* fs.remove(lockPath).pipe(Effect.catchReason('PlatformError', 'NotFound', () => Effect.void));


This can delete a lock held by a healthy writer. If writer A holds index.lock longer than autoclear, writer B removes it and acquires a new lock. When A completes, releaseLock renames the path now owned by B, so A may publish B’s index alongside A’s updated ref.

The PR description notes this limitation, but this can corrupt the index/ref transaction. Can we fail with RepoLockedError after the wait timeout instead of deleting locks automatically, unless we have a reliable ownership/liveness check?

how are you going to unlock it? how would someone get out of that bind to avoid being perma-stuck?

I chose the timeout based on "how long could an STL write take" after making it do some really, really big ones.

WillieRuemmele · 2026-10-30T15:03:39Z

+
+const EMPTY_CHANGES: LocalChanges = { added: [], modified: [], deleted: [] };
+
+const FD_PERMITS_ENV = 'SF_SOURCE_TRACKING_FD_PERMITS';


This introduces SF_SOURCE_TRACKING_FD_PERMITS, but no longer honors SF_SOURCE_TRACKING_BATCH_SIZE or SFDX_SOURCE_TRACKING_BATCH_SIZE. Those variables were the documented escape hatch for EMFILE in constrained developer and CI environments.

Could the new setting take precedence while the existing variables remain fallbacks? If the old settings are intentionally being retired, we should at least warn when either is set but ignored, so customers can migrate to SF_SOURCE_TRACKING_FD_PERMITS instead of silently losing their descriptor-limit mitigation.

I centralized the config stuff; there was a bunch of nonsense kit would allow (infinity, decimals, etc) that didn't make sense here, but it does change things a tiny bit.

WillieRuemmele · 2026-10-30T15:03:39Z

+        dir: options.projectPath,
+        gitdir: getGitDir(options.orgId, options.projectPath),
+        roots: options.packageDirs.map((directory) =>
+          ensurePosix(path.relative(options.projectPath, directory.fullPath))


A project can validly use its root as a package directory (path: "."). In that case path.relative(options.projectPath, directory.fullPath) returns "", but RepoConfig.roots is decoded as RepoPath, which rejects empty strings. That makes the first local-tracking operation fail for root-package projects.

The directory walker already uses "" to represent the root, so this seems to be an integration-boundary mismatch. Could we model a root separately from tracked file paths, and ensure a root matches every project-relative file?

The path: "." case appears valid, but I couldn’t find a public GitHub project that uses it. Going to see if it actually works with other stuff besides STL. If not, I'd recommend stopping that in sfdx-core rather than trying support it here.

ok, turns out that edge case doesn't currently work the CLI/STL. The other stuff (scratch org creation, SDR deploy) seems to so I think it's just an STL bug we could fix as part of this PR if you want to support that

Created the isolated project at rootPackageDot/ and scratch org root-package-dot-test (test-pziuoybhyxmh@example.com, expires Oct 31, 2026). The org was authenticated and reports source tracking enabled. No existing project files, global default org, or STL PR files were changed.

Creating the scratch org succeeded.
Deploying RootPackageDotProbe from the root package succeeded.
A follow-up deploy without an explicit source path said “Nothing to deploy” despite the local edit.
Deploying that edited Apex class explicitly with --source-dir succeeded.

WillieRuemmele · 2026-10-30T15:03:39Z

+ * → ObjectNotFoundError; truncated/malformed → ObjectCorruptError.
+ */
+/** @internal */
+export const readLooseObject = Effect.fnUntraced(function* (gitdir: string, oid: Oid) {


Two staging fibers can hash identical content, both observe that the object path does not exist, and then concurrently write the same final path. A reader can observe a partial zlib stream, and an interruption can leave a corrupt object at that OID.

Could we write to a unique temporary file in the object directory and atomically link/rename it into the final OID path, treating an already-created destination as success? That is the normal Git loose-object publication pattern.

this came up when I was building it as a gap between isogit and real git;
I preserved isogit's behavior because I didn't want to slow it down more by doubling the amount of fs writes

not sure it has ever caused a problem before.

WillieRuemmele · 2026-10-30T15:03:39Z

+          head.mode === workdirMode &&
+          index.mode === workdirMode
+            ? 'unmodified'
+            : 'modified',


A file that is present in the index and worktree but absent from HEAD is a staged addition. This branch classifies it as modified, because the only unmodified path requires a HEAD entry and every other tracked worktree file falls through to modified.

This matters when interpreting existing/interrupted shadow repositories. The accompanying test accepts added, modified, or unmodified for staged-add.txt, so it does not protect the prior contract. Could we classify head === undefined && index !== undefined && hasWorkdir as added and make that assertion exact?

will do, probably a good idea for the sake of git-correctness and some weird interruption edge case.

for us, it's normally moot. STL only ever does stage-then-immediately-commit; we don't actually use staging for anything so that's never had any tests or nuts to cover it.

WillieRuemmele · 2026-10-30T15:03:39Z

+const FD_PERMITS_ENV = 'SF_SOURCE_TRACKING_FD_PERMITS';
+const FD_PERMITS_FALLBACK = IS_WINDOWS ? 64 : 256;
+
+const readNumberEnv = (name: string, fallback: number) =>


We already depend on @salesforce/kit, which provides environment helpers, but this migration adds local Effect wrappers here, a separate numeric reader in lock.ts, and more inline parsing in remote tracking. The resulting behavior is inconsistent: malformed values fall back here but are fatal in fileOperations.ts.

If Effect-native parsing is the goal, could we centralize it in one shared helper and deliberately preserve the current Kit semantics and legacy aliases? Otherwise, using Kit at the boundary and lifting the result into Effect would avoid duplicating configuration policy.

working on this one.

WillieRuemmele · 2026-10-30T15:03:39Z

@@ -691,14 +742,8 @@ export class SourceTracking extends AsyncCreatable {
    ]);


SourceTracking#setIgnoreConflicts existed on main but is absent from this version. Current VS Code consumers call it when retrying deploy and retrieve operations after a conflict: salesforcedx-vscode-core/src/commands/deployExecutor.ts and salesforcedx-vscode-core/src/commands/retrieveExecutor.ts.

Updating that consumer to this release will fail TypeScript compilation; if types are bypassed, the retry path fails at runtime. Could we restore this public mutator and add a compatibility test covering the consumer flow?

Current VS Code consumers call it when retrying deploy and retrieve operations after a conflict

no, none of that code exists anymore
https://github.com/search?q=repo%3Aforcedotcom%2Fsalesforcedx-vscode+setIgnoreConflicts&type=code

WillieRuemmele · 2026-10-30T15:03:40Z

Could we separate the implementation-required changes from the repository/tooling changes, or document why they need to ship together?

This PR adds several top-level files (.c8rc.json, .circular-deps.json, .gitattributes, .prettierignore, CONTEXT.md, and knip.json) and new package commands/configuration such as knip, circular-dependency checking, Effect diagnostics, and c8 coverage.

A few questions:

Which of these are required for the Effect Git migration versus local developer/agent workflow?
Is .circular-deps.json intentionally a checked-in generated artifact?
Why should NUTs now run under c8? They are longer-running integration tests, and coverage instrumentation can change performance and file-system behavior. Is coverage from NUTs a deliberate CI requirement, or should coverage remain scoped to unit tests?
If the tooling additions are intentional, can we add a short rationale to the PR description and ideally split nonessential tooling/process files into a follow-up?

mshanemc · 2026-10-30T18:09:30Z

Which of these are required for the Effect Git migration versus local developer/agent workflow?

none are necessary, they're dev tooling like lint, tests, snapshots, ts types, coverage docs, etc...all unnecessary

Is .circular-deps.json intentionally a checked-in generated artifact?

no, that should be gitignored, fixing now

Why should NUTs now run under c8? They are longer-running integration tests, and coverage instrumentation can change performance and file-system behavior. Is coverage from NUTs a deliberate CI requirement, or should coverage remain scoped to unit tests?

I found it useful to find nut coverage gaps; not all guardrails are CI requirements.

If the tooling additions are intentional, can we add a short rationale to the PR description and ideally split nonessential tooling/process files into a follow-up?

I'll can remove them if you like once we're done iterating on this PR.

mshanemc · 2026-11-02T18:37:27Z

Could we separate the implementation-required changes from the repository/tooling changes, or document why they need to ship together?

done, the tooling parts are on a stacked pr. hope that helps

mshanemc requested a review from a team as a code owner October 16, 2026 03:54

mshanemc mentioned this pull request Oct 23, 2026

chore(deps): validate source-tracking 9.0.1-qa.2 - W-22711959 forcedotcom/salesforcedx-vscode#8502

Closed

1 task

peternhale approved these changes Oct 29, 2026

View reviewed changes

WillieRuemmele reviewed Oct 30, 2026

View reviewed changes

feat(git): replace isomorphic-git with Effect Git - W-22711959

49102eb

mshanemc mentioned this pull request Nov 2, 2026

feat(config): centralize Effect environment settings - W-22711959 #911

Merged

mshanemc added 2 commits November 2, 2026 11:44

Merge main dependency updates into Effect Git base

ac16b92

chore: refresh base dependency lockfile

f7f686d

mshanemc force-pushed the sm/W-22711959-shadowrepo-effect branch from c9accca to f7f686d Compare November 2, 2026 17:46

mshanemc added this pull request to stack #913 November 2, 2026 18:02

mshanemc mentioned this pull request Nov 2, 2026

chore: add Effect development tooling - W-22711959 #912

Draft

mshanemc added 8 commits November 2, 2026 12:16

chore: move Effect ESLint rule to tooling PR - W-22711959

f9bd429

feat(config): centralize Effect environment settings

20afd01

refactor(git): inline lock timeout conversion

b078fb4

refactor(effect): run source tracking through managed runtime

76ae9c5

fix(config): route warnings through lifecycle

a4ef498

refactor(config): use Effect schemas for numeric settings

5ef1350

refactor(config): resolve polling settings centrally

14c71b2

refactor(config): parse booleans without encoding

8416a5b

mshanemc added 2 commits November 2, 2026 12:58

refactor(test): inject source tracking config through Effect layer

249ae4e

refactor(config): use Effect boolean config

e29ad97

mshanemc added 4 commits November 2, 2026 14:50

chore: remove unused shadow integrity check - W-22711959

50b9bf8

refactor(effect): keep shadow repo creation Effect-native - W-22711959

6e53e71

refactor(effect): keep shadow creation inside SourceTracking

3449942

refactor: consolidate Effect config into Git migration

8651c32


		const EMPTY_CHANGES: LocalChanges = { added: [], modified: [], deleted: [] };

		const FD_PERMITS_ENV = 'SF_SOURCE_TRACKING_FD_PERMITS';

		@@ -691,14 +742,8 @@ export class SourceTracking extends AsyncCreatable {
		]);

Conversation

mshanemc commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Configuration behavior and compatibility

What issues does this PR fix or reference?

Uh oh!

peternhale left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshanemc Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshanemc Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WillieRuemmele commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mshanemc commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mshanemc commented Nov 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mshanemc commented Oct 16, 2026 •

edited

Loading

mshanemc Oct 30, 2026 •

edited

Loading

mshanemc Oct 30, 2026 •

edited

Loading

WillieRuemmele commented Oct 30, 2026 •

edited

Loading

mshanemc commented Oct 30, 2026 •

edited

Loading