refactor: Phase 1 PR2/7 — Extract LockManager IIFE module - #146
Merged
Conversation
Part of #129 Phase 1 (PR2/7): Extract 6 lock management methods. Changes: - Create LockManager.js.html with IIFE pattern (function(App){...})(App) - Move 6 methods from JavaScript.html App object: _getLocks, _saveLocks, acquireLock, releaseLock, releaseCurrentLock, refreshLockHeartbeat - Convert this.xxx → App.xxx in extracted methods (key difference from PR1) - Update Index.html: add LockManager.js include after UtilityFunctions.js - Update appWiringContracts.test.js: move methods to IIFE_EXTRACTED, count 41→37 - Update syncMethodsWiring.test.js: load LockManager source, update patterns Follows spec: 1147 tests pass, ESLint 0 errors, coverage unchanged. Scope gate: 5 files (new .html + JavaScript.html + Index.html + 2 wiring tests). Agend-Agent: cb-team-impl Agend-Branch: refactor/phase1-pr2-lock-manager Agend-Issued-At: 2026-06-24T05:47:40.128365+00:00
Owner
Author
✅ VERIFIEDReviewer: cb-team-reviewer Files Reviewed
SummaryPhase 1 PR2/7: Extracts 6 lock management methods from JavaScript.html into a new
|
| Method | Original this. |
Converted App. |
✅ |
|---|---|---|---|
_getLocks |
0 (uses localStorage) | 0 | ✅ |
_saveLocks |
0 (uses localStorage) | 0 | ✅ |
acquireLock |
_getLocks, tabId ×2, _saveLocks |
All → App. |
✅ |
releaseLock |
_getLocks, tabId, _saveLocks |
All → App. |
✅ |
releaseCurrentLock |
releaseLock, activeScheduleId |
All → App. |
✅ |
refreshLockHeartbeat |
isReadOnly, activeScheduleId ×3, _getLocks, _saveLocks, tabId |
All → App. |
✅ |
Load Order Verification (D2 1-hop)
Config.js.html (L468) → defines AppConfig ← used by refreshLockHeartbeat
...
JavaScript.html (L475) → defines App object
UtilityFunctions.js.html (L476) → PR1
LockManager.js.html (L477) → PR2, receives `App` via IIFE param
<script>App.init()</script> (L478)
AppConfig.ALL_SCHEDULES_ID available (Config loads at L468) ✅. App defined before LockManager loads ✅.
Stage 1 — Correctness
Zero findings:
- No conflict markers
- All 6 methods faithfully extracted (line-by-line verified against main:L742-795)
- Breadcrumb comments left in JavaScript.html for traceability
- IIFE pattern:
(function(App) { ... })(App)— correct closure - appWiringContracts: methods correctly reclassified (PRIVATE_HELPERS/NOT_EXTRACTABLE/EXTRACTED_TO_LIB → IIFE_EXTRACTED)
- Public method count 41→37 (4 public lock methods removed from JavaScript.html)
- Private helpers filter updated to check IIFE_EXTRACTED
- syncMethodsWiring:
extractMethodBodyenhanced with IIFE pattern fallback - All lock method wiring assertions updated
this\.→App\. - Lock methods now tested against
lockManagerSource(notjsHtmlSource)
Stage 2 — Adversarial
Zero findings:
refreshLockHeartbeatreferencesAppConfig.ALL_SCHEDULES_ID— verified AppConfig loaded at L468 before LockManager at L477- IIFE receives
Appas parameter but also usesAppConfigdirectly from global scope — this is consistent with existing GAS pattern (AppConfig is always global, never passed as IIFE param) - Method bodies are exact copies except
this.→App.— no logic changes, no subtle rewrites extractMethodBodyregex enhancement handles bothmethodName: function()(JS.html) andApp.methodName = function()(IIFE) patterns correctly
Evidence
- ran:
gh pr view 146 --json headRefOid→cf6e8429ba64dc75c6a6f7ff7c0af9bdea194306(SHA aligned) - ran:
grep -n 'this\.' LockManager.js.html→ NO this. references found (all 13 converted) - ran:
grep -nc 'App\.' LockManager.js.html→ 22 App. references (13 conversions + 9 method assignments) - ran:
gh api .../JavaScript.html?ref=main | sed -n 742,795p→ original source verified - ran:
gh api .../Index.html | grep -n AppConfig\|LockManager\|JavaScript\|init→ load order verified (Config L468 → JavaScript L475 → LockManager L477 → init L478) - cited: LockManager.js.html:L31 —
existingLock.tabId !== App.tabId(wasthis.tabId) - cited: LockManager.js.html:L58-62 —
refreshLockHeartbeatwithApp.isReadOnly,App.activeScheduleId,AppConfig.ALL_SCHEDULES_ID— all converted - cited: JavaScript.html:L742-744 — breadcrumb:
// _getLocks — moved to LockManager.js.html (Phase 1 PR2)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1 PR2: LockManager IIFE Extraction
Part of #129 Phase 1 (PR2/7) — Wave A: 2/3
What Changed
Extracts 6 lock management methods from JavaScript.html into a new
LockManager.js.htmlIIFE module. Unlike PR1 (pure functions), these methods access App state (tabId,activeScheduleId,isReadOnly) so allthis.xxxreferences are converted toApp.xxx.Files Changed (Scope Gate: 5 files)
LockManager.js.htmlJavaScript.htmlIndex.htmltests/unit/appWiringContracts.test.jstests/unit/syncMethodsWiring.test.jsthis→ApppatternsMethods Extracted
this→Appconversions_getLocks_saveLocksacquireLock_getLocks,_saveLocks,tabId)releaseLock_getLocks,_saveLocks,tabId)releaseCurrentLockreleaseLock,activeScheduleId)refreshLockHeartbeatisReadOnly,activeScheduleId,_getLocks,_saveLocks,tabId)Load Order
Verification
npm test: 1147/1147 tests pass (36 files)npx eslint --ext .html .: 0 errors, 8 warnings (pre-existing)this→Appconversion verified in all 6 methods