Implement metadata versioning, history tracking, and rollback capabilities#1061
Implement metadata versioning, history tracking, and rollback capabilities#1061
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/f60fe20f-aa2a-44ef-9eea-fb72d6cc454f Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/f60fe20f-aa2a-44ef-9eea-fb72d6cc454f Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/f60fe20f-aa2a-44ef-9eea-fb72d6cc454f Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/f60fe20f-aa2a-44ef-9eea-fb72d6cc454f Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/f60fe20f-aa2a-44ef-9eea-fb72d6cc454f Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
github-actions
bot
added
the
documentation
Claude
AI
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@copilot fix all ci build and test errors |
Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/b6ef0be7-6586-4835-801d-5075c0f3cb93 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed in commit 78b814c. There were four TypeScript errors in
|
|
@copilot 17:21:37.015 @objectstack/spec:build: ✓ data/TenantDatabaseLifecycle.json |
Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/9a0b7fca-28e4-41c5-9146-e6221be809ab Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed in commit a56cc68. |
|
@copilot 合并最新代码并解决冲突 |
…HANGELOG conflict Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces first-class metadata version history tracking (history timeline, diffing, rollback) across the ObjectStack metadata system, adding both protocol-level schemas and a database-backed implementation in the packages/metadata runtime layer.
Changes:
- Adds spec contracts + Zod schemas for metadata history records, diff results, and retention policy configuration.
- Extends the metadata runtime (
DatabaseLoader,MetadataManager) with history persistence, history querying, diff generation, and rollback. - Adds cleanup utilities, REST routes, documentation, changelog entry, and an initial test suite for history behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/spec/src/system/metadata-persistence.zod.ts | Defines history/diff/retention Zod schemas and exported types. |
| packages/spec/src/contracts/metadata-service.ts | Extends IMetadataService contract with getHistory, rollback, diff. |
| packages/metadata/src/utils/metadata-history-utils.ts | Implements checksum normalization + simple diff and summary generation. |
| packages/metadata/src/utils/history-cleanup.ts | Adds retention cleanup manager for history records (age/count policies). |
| packages/metadata/src/routes/history-routes.ts | Adds REST endpoints for history, rollback, and diff operations. |
| packages/metadata/src/objects/sys-metadata-history.object.ts | Introduces sys_metadata_history system object schema + indexes. |
| packages/metadata/src/metadata-manager.ts | Implements getHistory, rollback, diff on MetadataManager. |
| packages/metadata/src/metadata-history.test.ts | Adds tests for create/update history, rollback, diff, dedupe behavior. |
| packages/metadata/src/loaders/database-loader.ts | Adds history-table provisioning, checksum computation, and history writes on save. |
| packages/metadata/src/index.ts | Re-exports new object, routes, utilities, and spec types. |
| docs/METADATA_HISTORY.md | Documents usage and architecture for history/versioning features. |
| CHANGELOG.md | Adds an “Unreleased” entry describing metadata history/versioning. |
| */ | ||
| metadata: z.record(z.string(), z.unknown()).describe('Snapshot of metadata definition at this version'), |
There was a problem hiding this comment.
MetadataHistoryRecordSchema.metadata is defined as a required object (z.record(...)), but the metadata history table stores the snapshot as a JSON string (see DatabaseLoader.createHistoryRecord) and the API can omit metadata when includeMetadata is false. This schema should reflect the actual persisted/returned shape (e.g., metadata optional and/or z.unknown() / z.union([z.string(), z.record(...)])) or split into separate “row” vs “API response” schemas.
| */ | |
| metadata: z.record(z.string(), z.unknown()).describe('Snapshot of metadata definition at this version'), | |
| * May be stored as a raw JSON string in the history table, or as a parsed object | |
| * in higher-level APIs. When `includeMetadata` is false, this field may be omitted. | |
| */ | |
| metadata: z | |
| .union([z.string(), z.record(z.string(), z.unknown())]) | |
| .optional() | |
| .describe('Snapshot of metadata definition at this version (raw JSON string or parsed object)'), |
| const historyResult = records.map((row: Record<string, unknown>) => ({ | ||
| id: row.id as string, | ||
| metadataId: row.metadata_id as string, | ||
| name: row.name as string, | ||
| type: row.type as string, | ||
| version: row.version as number, | ||
| operationType: row.operation_type as 'create' | 'update' | 'publish' | 'revert' | 'delete', | ||
| metadata: includeMetadata | ||
| ? (typeof row.metadata === 'string' ? JSON.parse(row.metadata as string) : row.metadata) | ||
| : undefined, | ||
| checksum: row.checksum as string, | ||
| previousChecksum: row.previous_checksum as string | undefined, | ||
| changeNote: row.change_note as string | undefined, | ||
| tenantId: row.tenant_id as string | undefined, | ||
| recordedBy: row.recorded_by as string | undefined, | ||
| recordedAt: row.recorded_at as string, | ||
| })); | ||
|
|
There was a problem hiding this comment.
When includeMetadata is false, this function sets metadata to undefined, but MetadataHistoryQueryResult/MetadataHistoryRecord types require metadata. This will either fail type-checking or produce results that don’t conform to the declared contract. Consider making metadata optional in the schema/type, or returning a different record shape when includeMetadata is false.
| const historyResult = records.map((row: Record<string, unknown>) => ({ | |
| id: row.id as string, | |
| metadataId: row.metadata_id as string, | |
| name: row.name as string, | |
| type: row.type as string, | |
| version: row.version as number, | |
| operationType: row.operation_type as 'create' | 'update' | 'publish' | 'revert' | 'delete', | |
| metadata: includeMetadata | |
| ? (typeof row.metadata === 'string' ? JSON.parse(row.metadata as string) : row.metadata) | |
| : undefined, | |
| checksum: row.checksum as string, | |
| previousChecksum: row.previous_checksum as string | undefined, | |
| changeNote: row.change_note as string | undefined, | |
| tenantId: row.tenant_id as string | undefined, | |
| recordedBy: row.recorded_by as string | undefined, | |
| recordedAt: row.recorded_at as string, | |
| })); | |
| const historyResult = records.map((row: Record<string, unknown>) => { | |
| const parsedMetadata = | |
| typeof row.metadata === 'string' | |
| ? JSON.parse(row.metadata as string) | |
| : (row.metadata as unknown); | |
| return { | |
| id: row.id as string, | |
| metadataId: row.metadata_id as string, | |
| name: row.name as string, | |
| type: row.type as string, | |
| version: row.version as number, | |
| operationType: row.operation_type as 'create' | 'update' | 'publish' | 'revert' | 'delete', | |
| metadata: includeMetadata ? parsedMetadata : null, | |
| checksum: row.checksum as string, | |
| previousChecksum: row.previous_checksum as string | undefined, | |
| changeNote: row.change_note as string | undefined, | |
| tenantId: row.tenant_id as string | undefined, | |
| recordedBy: row.recorded_by as string | undefined, | |
| recordedAt: row.recorded_at as string, | |
| }; | |
| }); |
| if (existing) { | ||
| // Update existing record | ||
| const version = ((existing.version as number) ?? 0) + 1; | ||
| const previousChecksum = existing.checksum as string | undefined; | ||
|
|
||
| await this.driver.update(this.tableName, existing.id as string, { | ||
| metadata: metadataJson, | ||
| version, | ||
| checksum: newChecksum, | ||
| updated_at: now, | ||
| state: 'active', | ||
| }); | ||
|
|
||
| // Create history record for update | ||
| await this.createHistoryRecord( | ||
| existing.id as string, | ||
| type, | ||
| name, | ||
| version, | ||
| data, | ||
| 'update', | ||
| previousChecksum | ||
| ); |
There was a problem hiding this comment.
On update, the metadata record version is always incremented even when newChecksum matches the existing checksum; only the history insert is skipped. This can create version gaps in history (e.g., sys_metadata.version = 2 while history only has version 1) and makes rollback/diff by version unreliable. Consider short-circuiting the update entirely when the checksum is unchanged (no version bump) or storing a history entry that matches the bumped version.
| // Create history record for update | ||
| await this.createHistoryRecord( | ||
| existing.id as string, | ||
| type, | ||
| name, | ||
| version, | ||
| data, | ||
| 'update', | ||
| previousChecksum | ||
| ); |
There was a problem hiding this comment.
The PR/docs say history writes are asynchronous and “don’t block metadata operations”, but save() awaits createHistoryRecord() (including checksum calculation + DB write). Either update the docs/PR claim, or make history persistence non-blocking (e.g., queue/background task) while still preserving ordering/consistency guarantees.
| private async ensureHistorySchema(): Promise<void> { | ||
| if (!this.trackHistory || this.historySchemaReady) return; | ||
|
|
||
| try { | ||
| await this.driver.syncSchema(this.historyTableName, { | ||
| ...SysMetadataHistoryObject, | ||
| name: this.historyTableName, | ||
| }); | ||
| this.historySchemaReady = true; | ||
| } catch { | ||
| // If syncSchema fails (e.g. table already exists), mark ready and continue | ||
| this.historySchemaReady = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
ensureHistorySchema() marks historySchemaReady = true for all errors, not just “already exists”. If schema sync fails due to permissions/connectivity, history will silently stop attempting schema creation and subsequent writes will fail. Consider logging the error and only marking ready on success (or on a known “already exists” error).
| const oldRecords = await driver.find(historyTableName, { | ||
| object: historyTableName, | ||
| where: filter, | ||
| fields: ['id'], | ||
| }); | ||
|
|
||
| for (const record of oldRecords) { | ||
| try { | ||
| await driver.delete(historyTableName, record.id as string); | ||
| deleted++; | ||
| } catch { | ||
| errors++; | ||
| } |
There was a problem hiding this comment.
Cleanup currently deletes records one-by-one after fetching them (find then delete in a loop). IDataDriver supports bulkDelete() and may support deleteMany(), which would avoid N+1 deletes and reduce round-trips. Consider using deleteMany when available (fallback to per-record deletes), and for count-based cleanup query only the IDs beyond maxVersions (using orderBy + offset + fields: ['id']) instead of loading all versions into memory.
| const oldRecords = await driver.find(historyTableName, { | |
| object: historyTableName, | |
| where: filter, | |
| fields: ['id'], | |
| }); | |
| for (const record of oldRecords) { | |
| try { | |
| await driver.delete(historyTableName, record.id as string); | |
| deleted++; | |
| } catch { | |
| errors++; | |
| } | |
| const driverAny = driver as any; | |
| // Prefer driver-level bulk operations when available to avoid N+1 deletes. | |
| if (typeof driverAny.deleteMany === 'function') { | |
| const result = await driverAny.deleteMany(historyTableName, filter); | |
| if (typeof result === 'number') { | |
| deleted += result; | |
| } | |
| } else { | |
| const oldRecords = await driver.find(historyTableName, { | |
| object: historyTableName, | |
| where: filter, | |
| fields: ['id'], | |
| }); | |
| const ids: string[] = []; | |
| for (const record of oldRecords) { | |
| if (record && typeof record.id === 'string') { | |
| ids.push(record.id); | |
| } | |
| } | |
| if (ids.length > 0) { | |
| if (typeof (driver as any).bulkDelete === 'function') { | |
| const bulkResult = await (driver as any).bulkDelete(historyTableName, ids); | |
| if (typeof bulkResult === 'number') { | |
| deleted += bulkResult; | |
| } else { | |
| // Assume all requested records were deleted if no count is returned. | |
| deleted += ids.length; | |
| } | |
| } else { | |
| for (const id of ids) { | |
| try { | |
| await driver.delete(historyTableName, id); | |
| deleted++; | |
| } catch { | |
| errors++; | |
| } | |
| } | |
| } | |
| } |
| // Note: There may be overlap between age-based and count-based cleanup | ||
| // so the total is not simply the sum | ||
| return { | ||
| recordsByAge, | ||
| recordsByCount, | ||
| total: Math.max(recordsByAge, recordsByCount), | ||
| }; |
There was a problem hiding this comment.
getCleanupStats() returns total: Math.max(recordsByAge, recordsByCount), which can significantly undercount when age-based and count-based policies apply to different records (union size can be larger than the max). Consider returning separate counts only, or returning both a lower bound (max) and an upper bound (sum) instead of a single misleading total.
| import { MetadataManager } from '../metadata-manager.js'; | ||
| import { DatabaseLoader } from '../loaders/database-loader.js'; |
There was a problem hiding this comment.
These imports look incorrect for a test located in src/: ../metadata-manager.js and ../loaders/database-loader.js resolve outside src and likely won’t exist under Vitest. Other tests in this package import as ./metadata-manager / ./loaders/database-loader (no .js extension). Update the imports to match the existing test conventions so the suite can run.
| import { MetadataManager } from '../metadata-manager.js'; | |
| import { DatabaseLoader } from '../loaders/database-loader.js'; | |
| import { MetadataManager } from './metadata-manager'; | |
| import { DatabaseLoader } from './loaders/database-loader'; |
docs/METADATA_HISTORY.md
Outdated
|
|
||
| ## Performance Considerations | ||
|
|
||
| - History records are written asynchronously and failures don't block main operations |
There was a problem hiding this comment.
This section states “History records are written asynchronously”, but the current implementation awaits history writes in DatabaseLoader.save(). Either adjust the wording to match actual behavior, or update the implementation to perform background persistence so the documentation stays accurate.
| - History records are written asynchronously and failures don't block main operations | |
| - History records are written as part of the main save operation (synchronous with metadata changes) |
CHANGELOG.md
Outdated
|
|
||
| This aligns ObjectStack with enterprise platforms like Salesforce Setup Audit Trail and | ||
| ServiceNow Update Sets. See `docs/METADATA_HISTORY.md` for detailed usage. | ||
| ([Phase 4a: Metadata Versioning & History](https://github.com/objectstack-ai/framework/issues/XXXX)) |
There was a problem hiding this comment.
The changelog entry links to issues/XXXX, which looks like a placeholder and will be a broken link if merged. Replace with the real issue/PR number or remove the link until one exists.
| ([Phase 4a: Metadata Versioning & History](https://github.com/objectstack-ai/framework/issues/XXXX)) | |
| (Phase 4a: Metadata Versioning & History) |
|
@copilot fix all |
…, tests, docs Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/b9843a05-b0ee-4a50-bf70-18097fedf0b8 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Fixed in commit 1358200. All 14 issues from the reviewer have been addressed:
|
Adds comprehensive version history tracking for metadata items with rollback, diff, and automatic retention cleanup. Aligns with enterprise platforms like Salesforce Setup Audit Trail and ServiceNow Update Sets.
Core Implementation
Schema & Data Model
MetadataHistoryRecordSchemawith version, checksum, operation type, and audit fieldssys_metadata_historysystem table with indexes for timeline queriesService Layer
IMetadataServicewithgetHistory(),rollback(), anddiff()methodsDatabaseLoaderto automatically write history on create/update operationsREST API
GET /api/v1/metadata/:type/:name/history- Query version timelinePOST /api/v1/metadata/:type/:name/rollback- Restore to specific versionGET /api/v1/metadata/:type/:name/diff- Compare versions with JSON patch operationsRetention Management
HistoryCleanupManagerwith age-based (maxAgeDays) and count-based (maxVersions) policiesUsage
Technical Details
tenantIdfilter on all queries(metadata_id, version),(metadata_id, recorded_at),(type, name)hasMoreindicatorSee
docs/METADATA_HISTORY.mdfor complete documentation.