Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions client/src/components/apps/tabs/UpdateTab.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,10 @@ export default function UpdateTab() {
const n = installState.pendingMigrations.count;
installIssues.push(`${n} pending data migration${n === 1 ? '' : 's'} not yet applied.`);
}
if (installState?.submodules?.stale) {
const n = installState.submodules.paths?.length || 0;
installIssues.push(`${n || 'One or more'} submodule checkout${n === 1 ? ' is' : 's are'} out of sync with the revisions pinned by PortOS.`);
}

return (
<div className="space-y-6">
Expand Down Expand Up @@ -471,7 +475,7 @@ export default function UpdateTab() {
<div className="text-xs text-gray-400 mt-1">
Your checked-out code is ahead of what’s running or installed — this happens after a
manual <span className="font-mono">git pull</span> without <span className="font-mono">./update.sh</span>.
Reconcile to finish the update (install dependencies, rebuild, run migrations, restart).
Reconcile to finish the update (sync submodules, install dependencies, rebuild, run migrations, restart).
</div>
<ul className="text-xs text-gray-300 mt-2 space-y-1 list-disc list-inside">
{installIssues.map((msg, i) => (
Expand All @@ -485,7 +489,7 @@ export default function UpdateTab() {
onClick={handleSyncForkAndReconcile}
disabled={updating || polling || syncingFork}
className="px-4 py-2 bg-port-warning text-black rounded-lg text-sm flex items-center gap-2 hover:bg-port-warning/80 disabled:opacity-50"
title={`Fast-forwards ${remote?.fullName} main from ${upstreamName}, then runs update.sh to reconcile the install.`}
title={`Fast-forwards ${remote?.fullName} main from ${upstreamName}, then runs update.sh to sync pinned submodules and reconcile the install.`}
>
<GitFork size={14} className={syncingFork ? 'animate-pulse' : ''} />
{syncingFork ? 'Syncing fork...' : updating ? 'Reconciling...' : polling ? 'Restarting...' : 'Sync Fork & Reconcile'}
Expand All @@ -505,7 +509,7 @@ export default function UpdateTab() {
onClick={handleReconcile}
disabled={updating || polling || syncingFork}
className="px-4 py-2 bg-port-warning text-black rounded-lg text-sm flex items-center gap-2 hover:bg-port-warning/80 disabled:opacity-50"
title="Run update.sh to install dependencies, rebuild the client, run migrations, and restart."
title="Run update.sh to sync pinned submodules, install dependencies, rebuild the client, run migrations, and restart."
>
<RefreshCw size={14} className={updating ? 'animate-spin' : ''} />
{updating ? 'Reconciling...' : polling ? 'Restarting...' : 'Reconcile Now'}
Expand Down Expand Up @@ -568,7 +572,7 @@ export default function UpdateTab() {
onClick={handleSyncForkAndUpdate}
disabled={updating || polling || syncingFork}
className="px-4 py-2 bg-port-accent text-white rounded-lg text-sm flex items-center gap-2 hover:bg-port-accent/80 disabled:opacity-50"
title={`Fast-forwards ${remote?.fullName} main from ${upstreamName} via gh repo sync, then runs the local update. Refuses to overwrite divergent fork commits.`}
title={`Fast-forwards ${remote?.fullName} main from ${upstreamName} via gh repo sync, then runs the local update including pinned submodules. Refuses to overwrite divergent fork commits.`}
>
<GitFork size={14} className={syncingFork ? 'animate-pulse' : ''} />
{syncingFork ? 'Syncing fork...' : updating ? 'Updating...' : polling ? 'Restarting...' : 'Sync Fork & Update'}
Expand All @@ -588,7 +592,7 @@ export default function UpdateTab() {
onClick={handleUpdateFromForkAsIs}
disabled={updating || polling || syncingFork}
className="px-4 py-2 bg-port-border text-gray-400 rounded-lg text-sm flex items-center gap-2 hover:bg-port-border/80 hover:text-white disabled:opacity-50"
title="Skip the fork sync and pull from your fork's origin as-is. Use this if you already merged upstream into your fork via your own workflow."
title="Skip the fork sync and pull from your fork's origin as-is, then sync pinned submodules. Use this if you already merged upstream into your fork via your own workflow."
>
<Download size={14} className={updating ? 'animate-bounce' : ''} />
Update from Fork As-Is
Expand All @@ -601,6 +605,7 @@ export default function UpdateTab() {
onClick={handleUpdate}
disabled={updating || polling}
className="px-4 py-2 bg-port-accent text-white rounded-lg text-sm flex items-center gap-2 hover:bg-port-accent/80 disabled:opacity-50"
title="Pull PortOS, sync pinned submodules, install dependencies, rebuild, run migrations, and restart."
>
<Download size={14} className={updating ? 'animate-bounce' : ''} />
{updating ? 'Updating...' : polling ? 'Restarting...' : 'Update Now'}
Expand Down
18 changes: 18 additions & 0 deletions client/src/components/apps/tabs/UpdateTab.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,24 @@ describe('UpdateTab reconcile flow', () => {
expect(screen.getByRole('button', { name: 'Reconcile Now' })).toBeTruthy();
expect(mockToast.loading).not.toHaveBeenCalled();
});

it('surfaces stale pinned submodules as a reconcile action', async () => {
mockGetUpdateStatus.mockResolvedValue({
currentVersion: '2.24.0',
installState: {
outOfSync: true,
submodules: { stale: true, paths: ['lib/example'] },
},
});

render(<UpdateTab />);

expect(await screen.findByText(/1 submodule checkout is out of sync with the revisions pinned by PortOS/i)).toBeTruthy();
expect(screen.getByRole('button', { name: 'Reconcile Now' })).toHaveAttribute(
'title',
expect.stringMatching(/sync pinned submodules/i),
);
});
});

describe('UpdateTab — active CoS agent suppression', () => {
Expand Down
15 changes: 15 additions & 0 deletions docs/SELF_UPDATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@ The entry is at the top of `git stash list`. When already on `main`, pre-checkou

**So: an in-app or CLI update run from a feature branch will leave your checkout on `main` with your work parked in the stash.** Commit your work before updating if you would rather not deal with that — pushing alone does not help, since the stash covers uncommitted changes.

### Submodules follow the pulled parent revision

After pulling `main`, both platform scripts run:

```bash
git submodule sync --recursive
git submodule update --init --recursive
```

The sync step refreshes each checkout's local submodule metadata from the newly pulled `.gitmodules`; the update step initializes missing modules and restores every recursive checkout to the commit pinned by PortOS. It intentionally does **not** use `--remote`: a release consumes the reviewed gitlink commit, not an unreviewed newer submodule head.

All restart-triggering UI actions (`Update Now`, `Sync Fork & Update`, both “from Fork As-Is” variants, and the reconcile variants) launch `update.sh` or `update.ps1`, so they inherit this exact sequence. `Sync Fork Only` remains intentionally different: it only fast-forwards the GitHub fork and does not touch the local checkout.

`GET /api/update/status` also compares recursive submodule checkouts with their pinned revisions. An uninitialized, conflicted, behind, or divergent module marks the install out of sync, making the existing Reconcile control available even when no newer release is waiting. A checkout deliberately advanced through the Submodules tab is not treated as stale, and CoS worktrees report submodule state as unknown because they intentionally leave submodules uninitialized and cannot run the primary-checkout update flow.

To prevent that confusion, `POST /api/update/execute` rejects fork runs with **412 `FORK_SYNC_REQUIRED`** unless either:

- the request body sets `acknowledgeFork: true`, or
Expand Down
44 changes: 44 additions & 0 deletions scripts/update-submodules.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* Cross-platform self-update contract. Executing either updater for real would
* pull, install, rebuild, migrate, and restart the live instance, so source
* inspection is the highest safe boundary for pinning their command parity.
*/
import { describe, expect, it } from 'vitest';
import { readFileSync } from 'fs';
import { dirname, join } from 'path';
import { fileURLToPath } from 'url';

const REPO_ROOT = join(dirname(fileURLToPath(import.meta.url)), '..');

const SCRIPT_COMMANDS = [
{
path: 'update.sh',
pull: 'run git pull --rebase --autostash',
sync: 'run git submodule sync --recursive',
update: 'run git submodule update --init --recursive',
},
{
path: 'update.ps1',
pull: 'Invoke-Logged git pull --rebase --autostash',
sync: 'Invoke-Logged git submodule sync --recursive',
update: 'Invoke-Logged git submodule update --init --recursive',
},
];

describe.each(SCRIPT_COMMANDS)('$path submodule update contract', ({ path, pull, sync, update }) => {
const source = readFileSync(join(REPO_ROOT, path), 'utf8');

it('syncs recursive metadata and then checks out pinned commits after pulling', () => {
const pullIndex = source.indexOf(pull);
const syncIndex = source.indexOf(sync);
const updateIndex = source.indexOf(update);

expect(pullIndex).toBeGreaterThanOrEqual(0);
expect(syncIndex).toBeGreaterThan(pullIndex);
expect(updateIndex).toBeGreaterThan(syncIndex);
});

it('does not advance submodules past the commits reviewed by PortOS', () => {
expect(source).not.toMatch(/git submodule update[^\n]*--remote/);
});
});
79 changes: 75 additions & 4 deletions server/services/installState.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* actually running / installed?" so the UI can tell a user who did a bare
* `git pull` (without ./update.sh) that their install is half-updated.
*
* Four independent signals (issue #1779):
* Five independent signals (issue #1779):
* 1. running-stale-code — the server process booted at an older commit than
* what's now on disk (a `git pull` advanced HEAD but nothing restarted).
* 2. stale-deps — a workspace's package.json/lockfile is newer than
Expand All @@ -15,8 +15,10 @@
* 4. pending-migrations — migration files exist on disk that aren't in the
* applied-list. (Boot normally applies these, so a non-zero count means a
* pull landed new migrations and the server hasn't restarted yet.)
* 5. stale-submodules — an initialized checkout differs from the commit
* pinned by the parent, or a declared submodule has not been initialized.
*
* All four self-clear after a proper `update.sh` cycle (install bumps the npm
* All five self-clear after a proper `update.sh` cycle (install bumps the npm
* receipt mtime, build bumps the dist mtime, boot applies migrations and
* captures the new commit) — so no new on-disk marker format is introduced and
* existing installs get accurate detection immediately, with no migration.
Expand All @@ -30,6 +32,7 @@ import { join } from 'path';
import { stat, readdir } from 'fs/promises';
import { PATHS } from '../lib/fileUtils.js';
import { execGit } from '../lib/execGit.js';
import { parseSubmoduleStatusLine } from '../lib/gitOutputParsers.js';
import { listPendingMigrations } from '../../scripts/run-migrations.js';
import { isWorktreeRoot } from '../lib/dataRoot.js';

Expand Down Expand Up @@ -178,6 +181,63 @@ async function detectStaleDeps(rootDir, { statMtime = statMtimeMs } = {}) {
return { stale: workspaces.some(w => w.stale), workspaces };
}

/**
* Compare every recursive submodule checkout with the gitlink pinned by the
* parent repository. Git prefixes an in-sync checkout with a space, while
* `+`, `-`, and `U` mean a different commit, uninitialized, and conflicted.
*
* `stale: null` is intentionally distinct from a successful empty/in-sync
* result: an unavailable git checkout must not be reported as authoritative.
*/
async function detectSubmodules(rootDir, {
getStatus = () => execGit(
['submodule', 'status', '--recursive'],
rootDir,
{ ignoreExitCode: true }
),
isAheadOfPin = async ({ commit, path }) => {
const pinned = await execGit(
['rev-parse', `HEAD:${path}`],
rootDir,
{ ignoreExitCode: true }
);
const pinnedSha = pinned.exitCode === 0 ? pinned.stdout.trim() : '';
if (!pinnedSha) return false;
const ancestor = await execGit(
['merge-base', '--is-ancestor', pinnedSha, commit],
join(rootDir, path),
{ ignoreExitCode: true }
);
return ancestor.exitCode === 0;
}
} = {}) {
const result = await getStatus();
if (result?.exitCode !== 0 || typeof result?.stdout !== 'string') {
return { stale: null, paths: null };
}

// Split before trimming: the leading space is the authoritative in-sync
// status character. A successful empty result means there are no submodules.
const lines = result.stdout.split('\n').filter(line => line.trimEnd());
const parsed = lines.map(parseSubmoduleStatusLine);
if (parsed.some(entry => entry === null)) {
return { stale: null, paths: null };
}

const paths = [];
for (const entry of parsed) {
if (entry.statusChar === ' ') continue;
// The Submodules tab intentionally supports checking out the latest remote
// commit without committing the parent pointer. Git reports that as `+`,
// but it is not a half-finished update when the checkout descends from the
// pin. A behind/divergent `+`, `-` (uninitialized), or `U` (conflicted)
// still needs Reconcile. Comparison failures stay conservative (stale).
if (entry.statusChar === '+' && await isAheadOfPin(entry).catch(() => false)) continue;
paths.push(entry.path);
}
return { stale: paths.length > 0, paths };
}

/**
* Compute the full install-sync picture. Every external dependency is
* injectable so the detection logic is unit-testable without touching real
Expand Down Expand Up @@ -206,6 +266,12 @@ export async function getInstallState({
listPending = () => isWorktreeRoot(migrationRootDir)
? Promise.resolve([])
: listPendingMigrations({ rootDir: migrationRootDir }),
// A CoS worktree cannot switch to its own main branch, and intentionally
// leaves submodules uninitialized. Offering Reconcile there would be a
// permanent false action, so preserve unknown rather than reporting stale.
getSubmoduleState = () => isWorktreeRoot(rootDir)
? Promise.resolve({ stale: null, paths: null })
: detectSubmodules(rootDir),
} = {}) {
const currentCommit = await getCurrentCommit().catch(() => null);

Expand All @@ -229,11 +295,15 @@ export async function getInstallState({
const pendingFiles = await listPending().catch(() => []);
const pendingMigrations = { count: pendingFiles.length, files: pendingFiles };

const submodules = await getSubmoduleState()
.catch(() => ({ stale: null, paths: null }));

const outOfSync =
runningStaleCode ||
staleDeps.stale ||
staleBuild === true ||
pendingMigrations.count > 0;
pendingMigrations.count > 0 ||
submodules.stale === true;

return {
bootCommit: boot || null,
Expand All @@ -242,9 +312,10 @@ export async function getInstallState({
staleDeps,
staleBuild,
pendingMigrations,
submodules,
outOfSync
};
}

// Exported for unit tests of the individual detectors.
export const __internal = { detectStaleDeps, isClientSourceNewer, gitRevParseHead, gitIsAncestor };
export const __internal = { detectStaleDeps, detectSubmodules, isClientSourceNewer, gitRevParseHead, gitIsAncestor };
76 changes: 76 additions & 0 deletions server/services/installState.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ function syncedOpts(overrides = {}) {
}),
clientSourceNewer: async () => false,
listPending: async () => [],
getSubmoduleState: async () => ({ stale: false, paths: [] }),
...overrides
};
}
Expand Down Expand Up @@ -263,6 +264,81 @@ describe('getInstallState — pending migrations', () => {
});
});

describe('getInstallState — submodules', () => {
it('flags a checkout that differs from the parent-pinned commit', async () => {
const state = await getInstallState(syncedOpts({
getSubmoduleState: async () => ({ stale: true, paths: ['lib/example'] })
}));

expect(state.submodules).toEqual({ stale: true, paths: ['lib/example'] });
expect(state.outOfSync).toBe(true);
});

it('distinguishes an unavailable status check from an authoritative empty result', async () => {
const unknown = await getInstallState(syncedOpts({
getSubmoduleState: async () => { throw new Error('git unavailable'); }
}));
const empty = await getInstallState(syncedOpts());

expect(unknown.submodules).toEqual({ stale: null, paths: null });
expect(unknown.outOfSync).toBe(false);
expect(empty.submodules).toEqual({ stale: false, paths: [] });
});

it('parses recursive git status prefixes without trimming the in-sync marker', async () => {
const getStatus = async () => ({
exitCode: 0,
stdout: [
' abc1234 lib/current (heads/main)',
'+def5678 lib/different (heads/main)',
'-123abcd lib/missing',
'U456def0 lib/conflicted',
].join('\n')
});

expect(await __internal.detectSubmodules(ROOT, {
getStatus,
isAheadOfPin: async () => false,
})).toEqual({
stale: true,
paths: ['lib/different', 'lib/missing', 'lib/conflicted']
});
});

it('does not flag a deliberate remote update ahead of the pinned commit', async () => {
const isAheadOfPin = vi.fn(async () => true);
const state = await __internal.detectSubmodules(ROOT, {
getStatus: async () => ({ exitCode: 0, stdout: '+def5678 lib/ahead (heads/main)\n' }),
isAheadOfPin,
});

expect(state).toEqual({ stale: false, paths: [] });
expect(isAheadOfPin).toHaveBeenCalledWith({
statusChar: '+',
commit: 'def5678',
path: 'lib/ahead',
});
});

it('reports submodule state as unknown in a CoS worktree', async () => {
const worktreeRoot = join(ROOT, 'data', 'cos', 'worktrees', 'agent-abc');
const { getSubmoduleState, ...opts } = syncedOpts({ rootDir: worktreeRoot });
const state = await getInstallState(opts);

expect(state.submodules).toEqual({ stale: null, paths: null });
expect(state.outOfSync).toBe(false);
});

it('returns unknown for failed or malformed git status output', async () => {
expect(await __internal.detectSubmodules(ROOT, {
getStatus: async () => ({ exitCode: 128, stdout: '' })
})).toEqual({ stale: null, paths: null });
expect(await __internal.detectSubmodules(ROOT, {
getStatus: async () => ({ exitCode: 0, stdout: 'unexpected output' })
})).toEqual({ stale: null, paths: null });
});
});

describe('getInstallState — resilience', () => {
it('treats a thrown ancestry check as not-ahead', async () => {
const state = await getInstallState(syncedOpts({
Expand Down
10 changes: 8 additions & 2 deletions update.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,14 @@ if ($prePullSha) {
$global:LASTEXITCODE = 0
Write-SafeHost ""

# Update submodules (slash-do and any others)
Step "submodules" "running" "Updating submodules..."
# Refresh local submodule metadata from the just-pulled .gitmodules before
# checking out the commits pinned by PortOS. Without sync, a URL/path change in
# .gitmodules can leave an older instance trying to initialize from stale local
# git config. Deliberately omit --remote: the parent commit is the release
# contract, not whichever submodule commit happens to be newest upstream.
Step "submodules" "running" "Synchronizing and updating submodules..."
Invoke-Logged git submodule sync --recursive
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
Invoke-Logged git submodule update --init --recursive
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
Step "submodules" "done" "Submodules updated"
Expand Down
Loading